Merge pull request #118 from tylerschultz/require-gateway-ipv4-pool

Add validation for IP family and gateways
kubernetes-sigs · Apr 28, 2023 · d274a18 · d274a18
2 parents 2899ac6 + 4e964ec
commit d274a18
Show file tree

Hide file tree

Showing 2 changed files with 115 additions and 6 deletions.
diff --git a/internal/webhooks/inclusterippool.go b/internal/webhooks/inclusterippool.go
@@ -191,17 +191,29 @@ func validateAddresses(newPool types.GenericInClusterPool) field.ErrorList {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "prefix"), newPool.PoolSpec().Prefix, "a valid prefix is required when using addresses"))
 	}
 
+	var hasIPv4Addr, hasIPv6Addr bool
+	for _, address := range newPool.PoolSpec().Addresses {
+		ipSet, err := poolutil.AddressToIPSet(address)
+		if err != nil {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "addresses"), address, "provided address is not a valid IP, range, nor CIDR"))
+			continue
+		}
+		from := ipSet.Ranges()[0].From()
+		hasIPv4Addr = hasIPv4Addr || from.Is4()
+		hasIPv6Addr = hasIPv6Addr || from.Is6()
+	}
+	if hasIPv4Addr && hasIPv6Addr {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "addresses"), newPool.PoolSpec().Addresses, "provided addresses are of mixed IP families"))
+	}
+
 	if newPool.PoolSpec().Gateway != "" {
-		_, err := netip.ParseAddr(newPool.PoolSpec().Gateway)
+		gateway, err := netip.ParseAddr(newPool.PoolSpec().Gateway)
 		if err != nil {
 			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "gateway"), newPool.PoolSpec().Gateway, err.Error()))
 		}
-	}
 
-	for _, address := range newPool.PoolSpec().Addresses {
-		if !poolutil.AddressStrParses(address) {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "addresses"), address, "provided address is not a valid IP, range, nor CIDR"))
-			continue
+		if gateway.Is6() && hasIPv4Addr || gateway.Is4() && hasIPv6Addr {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "gateway"), newPool.PoolSpec().Gateway, "provided gateway and addresses are of mixed IP families"))
 		}
 	}
 

diff --git a/internal/webhooks/inclusterippool_test.go b/internal/webhooks/inclusterippool_test.go
@@ -67,6 +67,65 @@ func TestInClusterIPPoolDefaulting(t *testing.T) {
 				Prefix:  28,
 			},
 		},
+		{
+			name: "addresses with prefix and no gateway",
+			spec: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"10.0.0.25",
+					"10.0.0.26",
+					"10.0.0.27",
+				},
+				Prefix: 28,
+			},
+			expect: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"10.0.0.25",
+					"10.0.0.26",
+					"10.0.0.27",
+				},
+				Prefix: 28,
+			},
+		},
+		{
+			name: "IPv6 addresses with gateway and prefix",
+			spec: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"fe01::2",
+					"fe01::3-fe01::5",
+					"fe01::1:1/126",
+				},
+				Gateway: "fe01::1",
+				Prefix:  28,
+			},
+			expect: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"fe01::2",
+					"fe01::3-fe01::5",
+					"fe01::1:1/126",
+				},
+				Gateway: "fe01::1",
+				Prefix:  28,
+			},
+		},
+		{
+			name: "IPv6 addresses and prefix with no gateway",
+			spec: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"fe01::2",
+					"fe01::3-fe01::5",
+					"fe01::1:1/126",
+				},
+				Prefix: 28,
+			},
+			expect: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"fe01::2",
+					"fe01::3-fe01::5",
+					"fe01::1:1/126",
+				},
+				Prefix: 28,
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -308,6 +367,44 @@ func TestInvalidScenarios(t *testing.T) {
 			},
 			expectedError: "provided address belongs to a different subnet than others",
 		},
+		{
+			testcase: "Addresses are IPv4 and Gateway is IPv6",
+			spec: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"10.0.1.0",
+					"10.0.0.2-10.0.0.250",
+				},
+				Prefix:  24,
+				Gateway: "fd00::1",
+			},
+			expectedError: "provided gateway and addresses are of mixed IP families",
+		},
+		{
+			testcase: "Addresses are IPv6 and Gateway is IPv4",
+			spec: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"fd00::1",
+					"fd00::100-fd00::200",
+				},
+				Prefix:  24,
+				Gateway: "10.0.0.1",
+			},
+			expectedError: "provided gateway and addresses are of mixed IP families",
+		},
+		{
+			testcase: "Addresses is using mismatched IP families",
+			spec: v1alpha1.InClusterIPPoolSpec{
+				Addresses: []string{
+					"fd00::1",
+					"fd00::100-fd00::200",
+					"10.0.1.0",
+					"10.0.0.2-10.0.0.250",
+				},
+				Prefix:  24,
+				Gateway: "10.0.0.1",
+			},
+			expectedError: "provided addresses are of mixed IP families",
+		},
 	}
 	for _, tt := range tests {
 		namespacedPool := &v1alpha1.InClusterIPPool{Spec: tt.spec}