Merge pull request #3521 from pacevedom/USHIFT-3473

USHIFT-3473: Add validations for network ipv4/ipv6 stacks

Author: openshift-merge-bot[bot]

etcd/vendor/github.com/openshift/microshift/pkg/config/config.go

@@ -19,6 +19,7 @@ import (
 
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
+	netutils "k8s.io/utils/net"
 	"k8s.io/utils/ptr"
 
 	"github.com/apparentlymart/go-cidr/cidr"
@@ -301,6 +302,24 @@ func (c *Config) updateComputedValues() error {
 }
 
 func (c *Config) validate() error {
+	if !isValidIPAddress(c.ApiServer.AdvertiseAddress) {
+		return fmt.Errorf("error validating apiServer.advertiseAddress (%q)", c.ApiServer.AdvertiseAddress)
+	}
+	if c.ApiServer.SkipInterface {
+		err := checkAdvertiseAddressConfigured(c.ApiServer.AdvertiseAddress)
+		if err != nil {
+			return err
+		}
+	}
+
+	if !isValidIPAddress(c.Node.NodeIP) {
+		return fmt.Errorf("error validating node.nodeIP (%q)", c.Node.NodeIP)
+	}
+
+	if err := validateNetworkStack(c); err != nil {
+		return fmt.Errorf("error validating networks: %w", err)
+	}
+
 	//nolint:nestif // extracting the nested ifs will just increase the complexity of the if expressions as validation expands
 	if len(c.ApiServer.SubjectAltNames) > 0 {
 		// Any entry in SubjectAltNames will be included in the external access certificates.
@@ -353,13 +372,6 @@ func (c *Config) validate() error {
 		)
 	}
 
-	if c.ApiServer.SkipInterface {
-		err := checkAdvertiseAddressConfigured(c.ApiServer.AdvertiseAddress)
-		if err != nil {
-			return err
-		}
-	}
-
 	switch c.Ingress.Status {
 	case StatusManaged, StatusRemoved:
 	default:
@@ -588,3 +600,39 @@ func validateAuditLogConfig(cfg AuditLog) error {
 	}
 	return errors.Join(errs...) // Join returns nil if len(errs) == 0
 }
+
+func validateNetworkStack(cfg *Config) error {
+	if len(cfg.Network.ClusterNetwork) != len(cfg.Network.ServiceNetwork) {
+		return fmt.Errorf("network.serviceNetwork and network.clusterNetwork have different cardinality")
+	}
+	if len(cfg.Network.ServiceNetwork) > 2 {
+		return fmt.Errorf("network.serviceNetwork can not have more than 2 entries")
+	}
+	ipv4Entries := 0
+	ipv6Entries := 0
+	for i := 0; i < len(cfg.Network.ClusterNetwork); i++ {
+		_, _, err := net.ParseCIDR(cfg.Network.ServiceNetwork[i])
+		if err != nil {
+			return fmt.Errorf("invalid format in network.ServiceNetwork[%d]: %w", i, err)
+		}
+		_, _, err = net.ParseCIDR(cfg.Network.ClusterNetwork[i])
+		if err != nil {
+			return fmt.Errorf("invalid format in network.ClusterNetwork[%d]: %w", i, err)
+		}
+		if netutils.IPFamilyOfCIDRString(cfg.Network.ServiceNetwork[i]) != netutils.IPFamilyOfCIDRString(cfg.Network.ClusterNetwork[i]) {
+			return fmt.Errorf("mismatched IP families in network.ServiceNetwork[%d] and network.ClusterNetwork[%d]", i, i)
+		}
+		if netutils.IPFamilyOfCIDRString(cfg.Network.ServiceNetwork[i]) == netutils.IPv4 {
+			ipv4Entries++
+		} else {
+			ipv6Entries++
+		}
+	}
+	if ipv4Entries > 1 || ipv6Entries > 1 {
+		return fmt.Errorf("invalid number of entries of the same IP family in network.serviceNetwork and network.clusterNetwork")
+	}
+	if netutils.IPFamilyOfString(cfg.ApiServer.AdvertiseAddress) != netutils.IPFamilyOfCIDRString(cfg.Network.ServiceNetwork[0]) {
+		return fmt.Errorf("invalid IP family in apiServer.AdvertiseAddress: does not match first network.ServiceNetwork IP family")
+	}
+	return nil
+}

etcd/vendor/github.com/openshift/microshift/pkg/config/network.go

@@ -529,6 +529,84 @@ func TestValidate(t *testing.T) {
 			}(),
 			expectErr: false,
 		},
+		{
+			name: "network-too-many-entries",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Network.ServiceNetwork = []string{"1.2.3.4/24", "::1/128", "5.6.7.8/24"}
+				c.Network.ClusterNetwork = []string{"9.10.11.12/24", "::2/128", "13.14.15.16/24"}
+				c.ApiServer.AdvertiseAddress = "17.18.19.20"
+				return c
+			}(),
+			expectErr: true,
+		},
+		{
+			name: "network-same-ip-family-ipv4",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Network.ServiceNetwork = []string{"21.22.23.24/24", "25.26.27.28/24"}
+				c.Network.ClusterNetwork = []string{"29.30.31.32/24", "33.34.35.36/24"}
+				c.ApiServer.AdvertiseAddress = "37.38.39.40"
+				return c
+			}(),
+			expectErr: true,
+		},
+		{
+			name: "network-same-ip-family-ipv6",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Network.ServiceNetwork = []string{"fd01::/64", "fd02::/64"}
+				c.Network.ClusterNetwork = []string{"fd03::/64", "fd04::/64"}
+				c.ApiServer.AdvertiseAddress = "fd01::1"
+				return c
+			}(),
+			expectErr: true,
+		},
+		{
+			name: "network-bad-format-ipv4",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Network.ServiceNetwork = []string{"1.2.3.300/24"}
+				c.Network.ClusterNetwork = []string{"300.1.2.3/24"}
+				c.ApiServer.AdvertiseAddress = "8.8.8.8"
+				return c
+			}(),
+			expectErr: true,
+		},
+		{
+			name: "network-bad-format-ipv6",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Network.ServiceNetwork = []string{"fd01:::/64"}
+				c.Network.ClusterNetwork = []string{"fd05::/64"}
+				c.ApiServer.AdvertiseAddress = "fd01::2"
+				return c
+			}(),
+			expectErr: true,
+		},
+		{
+			name: "network-different-ip-family",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Network.ServiceNetwork = []string{"fd05::/64"}
+				c.Network.ClusterNetwork = []string{"4.3.2.1/24"}
+				c.ApiServer.AdvertiseAddress = "fd01::3"
+				return c
+			}(),
+			expectErr: true,
+		},
+
+		{
+			name: "network-different-ip-family-advertise-address",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Network.ServiceNetwork = []string{"fd06::/64"}
+				c.Network.ClusterNetwork = []string{"fd07::/64"}
+				c.ApiServer.AdvertiseAddress = "10.20.30.40"
+				return c
+			}(),
+			expectErr: true,
+		},
 	}
 	for _, tt := range ttests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/config/config.go

@@ -58,3 +58,8 @@ func getClusterDNS(serviceCIDR string) (string, error) {
 
 	return dnsClusterIP.String(), nil
 }
+
+func isValidIPAddress(ipAddress string) bool {
+	ip := net.ParseIP(ipAddress)
+	return ip != nil
+}