Properly surfaces errors when attempting to parse a subnet-mask-suffixed

address from NetworkInfo.
orbital-transfer-survey · May 15, 2020 · d832a3a · d832a3a
1 parent 2158c26
commit d832a3a
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 26 deletions.
diff --git a/apiserver/common/networkingcommon/types.go b/apiserver/common/networkingcommon/types.go
@@ -187,25 +187,17 @@ func NetworkInterfacesToStateArgs(ifaces corenetwork.InterfaceInfos) (
 			devicesArgs = append(devicesArgs, args)
 		}
 
-		if iface.CIDR == "" || iface.PrimaryAddress().Value == "" {
-			logger.Tracef(
-				"skipping empty CIDR %q and/or Address %q of %q",
-				iface.CIDR, iface.PrimaryAddress(), iface.InterfaceName,
-			)
-			continue
-		}
-		_, ipNet, err := net.ParseCIDR(iface.CIDR)
+		cidrAddress, err := iface.CIDRAddress()
 		if err != nil {
-			logger.Warningf("FIXME: ignoring unexpected CIDR format %q: %v", iface.CIDR, err)
+			logger.Warningf("ignoring unexpected address/CIDR format: %q/%q, %v",
+				iface.PrimaryAddress(), iface.CIDR, err)
 			continue
 		}
-		ipAddr := net.ParseIP(iface.PrimaryAddress().Value)
-		if ipAddr == nil {
-			logger.Warningf("FIXME: ignoring unexpected Address format %q", iface.PrimaryAddress().Value)
+		if cidrAddress == "" {
+			logger.Tracef("skipping empty CIDR %q and/or Address %q of %q",
+				iface.CIDR, iface.PrimaryAddress(), iface.InterfaceName)
 			continue
 		}
-		ipNet.IP = ipAddr
-		cidrAddress := ipNet.String()
 
 		var derivedConfigMethod corenetwork.AddressConfigMethod
 		switch method := corenetwork.AddressConfigMethod(iface.ConfigType); method {

diff --git a/cloudconfig/cloudinit/network_ubuntu.go b/cloudconfig/cloudinit/network_ubuntu.go
@@ -34,7 +34,10 @@ func GenerateENITemplate(interfaces corenetwork.InterfaceInfos) (string, error)
 	}
 	logger.Debugf("generating /e/n/i template from %#v", interfaces)
 
-	prepared := PrepareNetworkConfigFromInterfaces(interfaces)
+	prepared, err := PrepareNetworkConfigFromInterfaces(interfaces)
+	if err != nil {
+		return "", errors.Trace(err)
+	}
 
 	var output bytes.Buffer
 	gateway4Handled := false
@@ -147,7 +150,11 @@ func GenerateNetplan(interfaces corenetwork.InterfaceInfos) (string, error) {
 	netPlan.Network.Version = 2
 	for _, info := range interfaces {
 		var iface netplan.Ethernet
-		if cidr := info.CIDRAddress(); cidr != "" {
+		cidr, err := info.CIDRAddress()
+		if err != nil {
+			return "", errors.Trace(err)
+		}
+		if cidr != "" {
 			iface.Addresses = append(iface.Addresses, cidr)
 		} else if info.ConfigType == corenetwork.ConfigDHCP {
 			t := true
@@ -210,7 +217,7 @@ type PreparedConfig struct {
 // PrepareNetworkConfigFromInterfaces collects the necessary information to
 // render a persistent network config from the given slice of
 // network.InterfaceInfo. The result always includes the loopback interface.
-func PrepareNetworkConfigFromInterfaces(interfaces corenetwork.InterfaceInfos) *PreparedConfig {
+func PrepareNetworkConfigFromInterfaces(interfaces corenetwork.InterfaceInfos) (*PreparedConfig, error) {
 	dnsServers := set.NewStrings()
 	dnsSearchDomains := set.NewStrings()
 	gateway4Address := ""
@@ -247,7 +254,11 @@ func PrepareNetworkConfigFromInterfaces(interfaces corenetwork.InterfaceInfos) *
 			autoStarted.Add(ifaceName)
 		}
 
-		if cidr := info.CIDRAddress(); cidr != "" {
+		cidr, err := info.CIDRAddress()
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
+		if cidr != "" {
 			nameToAddress[ifaceName] = cidr
 		} else if info.ConfigType == corenetwork.ConfigDHCP {
 			nameToAddress[ifaceName] = string(corenetwork.ConfigDHCP)
@@ -290,7 +301,7 @@ func PrepareNetworkConfigFromInterfaces(interfaces corenetwork.InterfaceInfos) *
 	}
 
 	logger.Debugf("prepared network config for rendering: %+v", prepared)
-	return prepared
+	return prepared, nil
 }
 
 // AddNetworkConfig adds configuration scripts for specified interfaces

diff --git a/core/network/address.go b/core/network/address.go
@@ -139,6 +139,11 @@ func (a MachineAddress) String() string {
 	return prefix + a.Value
 }
 
+// IP returns the net.IP representation of this address.
+func (a MachineAddress) IP() net.IP {
+	return net.ParseIP(a.Value)
+}
+
 // sortOrder calculates the "weight" of the address when sorting:
 // - public IPs first;
 // - hostnames after that, but "localhost" will be last if present;

diff --git a/core/network/nic.go b/core/network/nic.go
@@ -162,7 +162,7 @@ type InterfaceInfo struct {
 	DNSServers ProviderAddresses
 
 	// MTU is the Maximum Transmission Unit controlling the maximum size of the
-	// protocol packats that the interface can pass through. It is only used
+	// protocol packets that the interface can pass through. It is only used
 	// when > 0.
 	MTU int
 
@@ -211,21 +211,21 @@ func (i *InterfaceInfo) IsVLAN() bool {
 }
 
 // CIDRAddress returns Address.Value combined with CIDR mask.
-func (i *InterfaceInfo) CIDRAddress() string {
+func (i *InterfaceInfo) CIDRAddress() (string, error) {
 	primaryAddr := i.PrimaryAddress()
 	if i.CIDR == "" || primaryAddr.Value == "" {
-		return ""
+		return "", nil
 	}
 	_, ipNet, err := net.ParseCIDR(i.CIDR)
 	if err != nil {
-		return errors.Trace(err).Error()
+		return "", errors.Trace(err)
 	}
-	ip := net.ParseIP(primaryAddr.Value)
+	ip := primaryAddr.IP()
 	if ip == nil {
-		return errors.Errorf("cannot parse IP address %q", primaryAddr.Value).Error()
+		return "", errors.Errorf("cannot parse IP address %q", primaryAddr.Value)
 	}
 	ipNet.IP = ip
-	return ipNet.String()
+	return ipNet.String(), nil
 }
 
 // PrimaryAddress returns the primary address for the interface.