Fixed lp:1397376 - api-endpoints behaves as expected

agent/bootstrap_test.go

@@ -135,9 +135,7 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
 	apiHostPorts, err := st.APIHostPorts()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(apiHostPorts, jc.DeepEquals, [][]network.HostPort{
-		network.AddressesWithPort(
-			network.NewAddresses("zeroonetwothree", "0.1.2.3"),
-			1234),
+		network.NewHostPorts(1234, "zeroonetwothree", "0.1.2.3"),
 	})
 
 	// Check that the state serving info is initialised correctly.

api/machiner/machiner_test.go

@@ -134,17 +134,24 @@ func (s *machinerSuite) TestSetMachineAddresses(c *gc.C) {
 	addr := s.machine.Addresses()
 	c.Assert(addr, gc.HasLen, 0)
 
-	addresses := []network.Address{
-		network.NewAddress("127.0.0.1", network.ScopeUnknown),
-		network.NewAddress("10.0.0.1", network.ScopeUnknown),
-		network.NewAddress("8.8.8.8", network.ScopeUnknown),
-	}
-	err = machine.SetMachineAddresses(addresses)
+	setAddresses := network.NewAddresses(
+		"8.8.8.8",
+		"127.0.0.1",
+		"10.0.0.1",
+	)
+	// Before setting, the addresses are sorted to put public on top,
+	// cloud-local next, machine-local last.
+	expectAddresses := network.NewAddresses(
+		"8.8.8.8",
+		"10.0.0.1",
+		"127.0.0.1",
+	)
+	err = machine.SetMachineAddresses(setAddresses)
 	c.Assert(err, jc.ErrorIsNil)
 
 	err = s.machine.Refresh()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(s.machine.MachineAddresses(), gc.DeepEquals, addresses)
+	c.Assert(s.machine.MachineAddresses(), jc.DeepEquals, expectAddresses)
 }
 
 func (s *machinerSuite) TestWatch(c *gc.C) {

api/rsyslog/rsyslog_test.go

@@ -65,7 +65,7 @@ func (s *rsyslogSuite) TestWatchForRsyslogChanges(c *gc.C) {
 	wc.AssertOneChange()
 
 	// change the API HostPorts
-	newHostPorts := network.AddressesWithPort(network.NewAddresses("127.0.0.1"), 6541)
+	newHostPorts := network.NewHostPorts(6541, "127.0.0.1")
 	err = s.State.SetAPIHostPorts([][]network.HostPort{newHostPorts})
 	c.Assert(err, jc.ErrorIsNil)
 

apiserver/client/export_test.go

@@ -14,8 +14,8 @@ var MachineJobFromParams = machineJobFromParams
 
 // Filtering exports
 var (
-	MatchPorts  = matchPorts
-	MatchSubnet = matchSubnet
+	MatchPortRanges = matchPortRanges
+	MatchSubnet     = matchSubnet
 )
 
 // Status exports

apiserver/client/filtering.go

@@ -163,11 +163,11 @@ func unitMatchSubnet(u *state.Unit, patterns []string) (bool, bool, error) {
 }
 
 func unitMatchPort(u *state.Unit, patterns []string) (bool, bool, error) {
-	ports, err := u.OpenedPorts()
+	portRanges, err := u.OpenedPorts()
 	if err != nil {
 		return false, false, err
 	}
-	return matchPorts(patterns, network.PortRangesToPorts(ports)...)
+	return matchPortRanges(patterns, portRanges...)
 }
 
 func buildServiceMatcherShims(s *state.Service, patterns ...string) (shims []closurePredicate, _ error) {
@@ -267,8 +267,8 @@ func buildUnitMatcherShims(u *state.Unit, patterns []string) []closurePredicate
 	}
 }
 
-func matchPorts(patterns []string, ports ...network.Port) (bool, bool, error) {
-	for _, p := range ports {
+func matchPortRanges(patterns []string, portRanges ...network.PortRange) (bool, bool, error) {
+	for _, p := range portRanges {
 		for _, patt := range patterns {
 			if strings.HasPrefix(p.String(), patt) {
 				return true, true, nil

apiserver/client/filtering_test.go

@@ -16,14 +16,19 @@ type filteringUnitTests struct {
 
 var _ = gc.Suite(&filteringUnitTests{})
 
-func (f *filteringUnitTests) TestMatchPorts(c *gc.C) {
+func (f *filteringUnitTests) TestMatchPortRanges(c *gc.C) {
 
-	match, ok, err := client.MatchPorts([]string{"80/tcp"}, network.Port{"tcp", 80})
+	match, ok, err := client.MatchPortRanges([]string{"80/tcp"}, network.PortRange{80, 80, "tcp"})
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(ok, jc.IsTrue)
 	c.Check(match, jc.IsTrue)
 
-	match, ok, err = client.MatchPorts([]string{"90/tcp"}, network.Port{"tcp", 80})
+	match, ok, err = client.MatchPortRanges([]string{"80-90/tcp"}, network.PortRange{80, 90, "tcp"})
+	c.Check(err, jc.ErrorIsNil)
+	c.Check(ok, jc.IsTrue)
+	c.Check(match, jc.IsTrue)
+
+	match, ok, err = client.MatchPortRanges([]string{"90/tcp"}, network.PortRange{80, 90, "tcp"})
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(ok, jc.IsTrue)
 	c.Check(match, jc.IsFalse)

apiserver/common/tools.go

@@ -362,6 +362,9 @@ func (t *toolsURLGetter) ToolsURL(v version.Binary) (string, error) {
 	// is the upgrader, and that is cloud-local.
 	hostPorts := apiHostPorts[rand.Int()%len(apiHostPorts)]
 	apiAddress := network.SelectInternalHostPort(hostPorts, false)
+	if apiAddress == "" {
+		return "", errors.Errorf("no suitable API server address to pick from %v", hostPorts)
+	}
 	serverRoot := fmt.Sprintf("https://%s/environment/%s", apiAddress, t.envUUID)
 	return ToolsURL(serverRoot, v), nil
 }

apiserver/common/tools_test.go

@@ -269,10 +269,7 @@ func (s *toolsSuite) TestToolsURLGetterAPIHostPortsError(c *gc.C) {
 func (s *toolsSuite) TestToolsURLGetter(c *gc.C) {
 	g := common.NewToolsURLGetter("my-uuid", mockAPIHostPortsGetter{
 		hostPorts: [][]network.HostPort{
-			network.AddressesWithPort(
-				network.NewAddresses("0.1.2.3"),
-				1234,
-			),
+			network.NewHostPorts(1234, "0.1.2.3"),
 		},
 	})
 	url, err := g.ToolsURL(version.Current)

apiserver/params/internal.go

@@ -69,6 +69,11 @@ type PortsResults struct {
 // of network.Port or an error.
 type PortsResult struct {
 	Error *Error
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// []string instead in order to break the dependency on the
+	// network package, as this potentially introduces hard to catch
+	// and debug wire-format changes in the protocol when the type
+	// changes!
 	Ports []network.Port
 }
 
@@ -84,7 +89,11 @@ type MachinePorts struct {
 type MachinePortRange struct {
 	UnitTag     string
 	RelationTag string
-	PortRange   network.PortRange
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// string instead in order to break the dependency on the network
+	// package, as this potentially introduces hard to catch and debug
+	// wire-format changes in the protocol when the type changes!
+	PortRange network.PortRange
 }
 
 // MachinePortsParams holds the arguments for making a
@@ -419,7 +428,12 @@ type RequestedNetworksResults struct {
 // MachineNetworkInfoResult holds network info for a single machine.
 type MachineNetworkInfoResult struct {
 	Error *Error
-	Info  []network.Info
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// []NetworkInfo (locally defined) instead in order to break the
+	// dependency on the network package, as this potentially
+	// introduces hard to catch and debug wire-format changes in the
+	// protocol when the type changes!
+	Info []network.Info
 }
 
 // MachineNetworkInfoResults holds network info for multiple machines.
@@ -458,7 +472,12 @@ type StatusResults struct {
 
 // MachineAddresses holds an machine tag and addresses.
 type MachineAddresses struct {
-	Tag       string
+	Tag string
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// []string instead in order to break the dependency on the
+	// network package, as this potentially introduces hard to catch
+	// and debug wire-format changes in the protocol when the type
+	// changes!
 	Addresses []network.Address
 }
 

apiserver/params/params.go

@@ -149,7 +149,12 @@ type AddMachineParams struct {
 	InstanceId              instance.Id
 	Nonce                   string
 	HardwareCharacteristics instance.HardwareCharacteristics
-	Addrs                   []network.Address
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// []string instead in order to break the dependency on the
+	// network package, as this potentially introduces hard to catch
+	// and debug wire-format changes in the protocol when the type
+	// changes!
+	Addrs []network.Address
 }
 
 // AddMachines holds the parameters for making the
@@ -573,7 +578,13 @@ type RsyslogConfigResult struct {
 	// Port is only used by state servers as the port to listen on.
 	// Clients should use HostPorts for the rsyslog addresses to forward
 	// logs to.
-	Port      int
+	Port int
+
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// []string instead in order to break the dependency on the
+	// network package, as this potentially introduces hard to catch
+	// and debug wire-format changes in the protocol when the type
+	// changes!
 	HostPorts []network.HostPort
 }
 
@@ -599,6 +610,11 @@ type DistributionGroupResults struct {
 // call. Each element in the top level slice holds
 // the addresses for one API server.
 type APIHostPortsResult struct {
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// [][]string instead in order to break the dependency on the
+	// network package, as this potentially introduces hard to catch
+	// and debug wire-format changes in the protocol when the type
+	// changes!
 	Servers [][]network.HostPort
 }
 
@@ -611,6 +627,11 @@ type FacadeVersions struct {
 
 // LoginResult holds the result of a Login call.
 type LoginResult struct {
+	// TODO(dimitern): Add explicit JSON serialization tags and use
+	// [][]string instead in order to break the dependency on the
+	// network package, as this potentially introduces hard to catch
+	// and debug wire-format changes in the protocol when the type
+	// changes!
 	Servers        [][]network.HostPort
 	EnvironTag     string
 	LastConnection *time.Time
@@ -637,6 +658,10 @@ type AuthUserInfo struct {
 
 // LoginRequestV1 holds the result of an Admin v1 Login call.
 type LoginResultV1 struct {
+	// TODO(dimitern): Use [][]string instead in order to break the
+	// dependency on the network package, as this potentially
+	// introduces hard to catch and debug wire-format changes in the
+	// protocol when the type changes!
 	Servers [][]network.HostPort `json:"servers"`
 
 	// EnvironTag is the tag for the environment that is being connected to.

apiserver/params/params_test.go

@@ -169,6 +169,128 @@ func (s *MarshalSuite) TestDeltaMarshalJSONUnknownEntity(c *gc.C) {
 	c.Check(err, gc.ErrorMatches, `Unexpected entity name "qwan"`)
 }
 
+// TODO(dimitern): apiserver/params should not depend on the network
+// package: network types used as fields in request/response structs
+// should be replaced with equivalent string, []string, or [][]string
+// types, so the wire-format of the API protocol will remain stable,
+// even if a network type changes its serialization format.
+//
+// This test ensures the following network package types used as
+// fields are still properly serialized and deserialized:
+//
+// params.PortsResult.Ports                []network.Port
+// params.MachinePortRange.PortRange       network.PortRange
+// params.MachineAddresses.Addresses       []network.Address
+// params.AddMachineParams.Addrs           []network.Address
+// params.RsyslogConfigResult.HostPorts    []network.HostPort
+// params.APIHostPortsResult.Servers       [][]network.HostPort
+// params.LoginResult.Servers              [][]network.HostPort
+// params.LoginResultV1.Servers            [][]network.HostPort
+func (s *MarshalSuite) TestNetworkEntities(c *gc.C) {
+
+	allAddressesCombos := []network.Address{
+		{Value: "foo0", Type: "bar0", NetworkName: "baz0", Scope: "none0"},
+		{Type: "bar1", NetworkName: "baz1", Scope: "none1"},
+		{Value: "foo2", NetworkName: "baz2", Scope: "none2"},
+		{Value: "foo3", Type: "bar3", Scope: "none3"},
+		{Value: "foo4", Type: "bar4", NetworkName: "baz4"},
+		{Value: "foo5", Type: "bar5"},
+		{Value: "foo6"},
+		{},
+	}
+	allHostPortCombos := network.AddressesWithPort(allAddressesCombos, 1234)
+	allHostPortCombos = append(allHostPortCombos,
+		network.AddressesWithPort(allAddressesCombos, 0)...,
+	)
+	allServerHostPorts := [][]network.HostPort{
+		allHostPortCombos,
+		network.AddressesWithPort(allAddressesCombos, 0),
+		network.AddressesWithPort(allAddressesCombos, 1234),
+		{},
+	}
+
+	for i, test := range []struct {
+		about string
+		input interface{}
+	}{{
+		about: "params.PortResult.Ports",
+		input: []params.PortsResult{{
+			Ports: []network.Port{
+				{Protocol: "foo", Number: 42},
+				{Protocol: "bar"},
+				{Number: 99},
+				{},
+			},
+		}, {},
+		},
+	}, {
+		about: "params.MachinePortRange.PortRange",
+		input: []params.MachinePortRange{{
+			UnitTag:     "foo",
+			RelationTag: "bar",
+			PortRange:   network.PortRange{FromPort: 42, ToPort: 69, Protocol: "baz"},
+		}, {
+			PortRange: network.PortRange{ToPort: 69, Protocol: "foo"},
+		}, {
+			PortRange: network.PortRange{FromPort: 42, Protocol: "bar"},
+		}, {
+			PortRange: network.PortRange{Protocol: "baz"},
+		}, {
+			PortRange: network.PortRange{FromPort: 42, ToPort: 69},
+		}, {
+			PortRange: network.PortRange{},
+		}, {},
+		},
+	}, {
+		about: "params.MachineAddresses.Addresses",
+		input: []params.MachineAddresses{{
+			Tag:       "foo",
+			Addresses: allAddressesCombos,
+		}, {},
+		},
+	}, {
+		about: "params.AddMachineParams.Addrs",
+		input: []params.AddMachineParams{{
+			Series:    "foo",
+			ParentId:  "bar",
+			Placement: nil,
+			Addrs:     allAddressesCombos,
+		}, {},
+		},
+	}, {
+		about: "params.RsyslogConfigResult.HostPorts",
+		input: []params.RsyslogConfigResult{{
+			CACert:    "fake",
+			HostPorts: allHostPortCombos,
+		}, {},
+		},
+	}, {
+		about: "params.APIHostPortsResult.Servers",
+		input: []params.APIHostPortsResult{{
+			Servers: allServerHostPorts,
+		}, {},
+		},
+	}, {
+		about: "params.LoginResult.Servers",
+		input: []params.LoginResult{{
+			Servers: allServerHostPorts,
+		}, {},
+		},
+	}, {
+		about: "params.LoginResultV1.Servers",
+		input: []params.LoginResultV1{{
+			Servers: allServerHostPorts,
+		}, {},
+		},
+	}} {
+		c.Logf("\ntest %d: %s", i, test.about)
+		output, err := json.Marshal(test.input)
+		c.Assert(err, jc.ErrorIsNil)
+		c.Logf("\nJSON output:\n%v", string(output))
+		c.Assert(string(output), jc.JSONEquals, test.input)
+	}
+}
+
 type ErrorResultsSuite struct{}
 
 var _ = gc.Suite(&ErrorResultsSuite{})