Enable Service Endpoints for Subnets

kubernetes-sigs · Sep 29, 2022 · 6363aa0 · 6363aa0
1 parent d186835
commit 6363aa0
Show file tree

Hide file tree

Showing 25 changed files with 495 additions and 39 deletions.
diff --git a/api/v1alpha3/azurecluster_conversion.go b/api/v1alpha3/azurecluster_conversion.go
@@ -89,6 +89,7 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error {
 
 	// Here we manually restore outbound security rules. Since v1alpha3 only supports ingress ("Inbound") rules, all v1alpha4/v1beta1 outbound rules are dropped when an AzureCluster
 	// is converted to v1alpha3. We loop through all security group rules. For all previously existing outbound rules we restore the full rule.
+	// Also restores ServiceEndpoints
 	for _, restoredSubnet := range restored.Spec.NetworkSpec.Subnets {
 		for i, dstSubnet := range dst.Spec.NetworkSpec.Subnets {
 			if dstSubnet.Name != restoredSubnet.Name {
@@ -104,6 +105,8 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error {
 			dst.Spec.NetworkSpec.Subnets[i].SecurityGroup.SecurityRules = append(dst.Spec.NetworkSpec.Subnets[i].SecurityGroup.SecurityRules, restoredOutboundRules...)
 			dst.Spec.NetworkSpec.Subnets[i].NatGateway = restoredSubnet.NatGateway
 
+			dst.Spec.NetworkSpec.Subnets[i].ServiceEndpoints = restoredSubnet.ServiceEndpoints
+
 			break
 		}
 	}

diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go
diff --git a/api/v1alpha4/azurecluster_conversion.go b/api/v1alpha4/azurecluster_conversion.go
@@ -70,11 +70,12 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error {
 		}
 	}
 
-	// Restore NAT Gateway IP tags.
+	// Restore NAT Gateway IP tags and ServiceEndpoints.
 	for _, restoredSubnet := range restored.Spec.NetworkSpec.Subnets {
 		for i, dstSubnet := range dst.Spec.NetworkSpec.Subnets {
 			if dstSubnet.Name == restoredSubnet.Name {
 				dst.Spec.NetworkSpec.Subnets[i].NatGateway.NatGatewayIP.IPTags = restoredSubnet.NatGateway.NatGatewayIP.IPTags
+				dst.Spec.NetworkSpec.Subnets[i].ServiceEndpoints = restoredSubnet.ServiceEndpoints
 			}
 		}
 	}
@@ -87,6 +88,7 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error {
 		if restored.Spec.BastionSpec.AzureBastion.Subnet.NatGateway.NatGatewayIP.Name == dst.Spec.BastionSpec.AzureBastion.Subnet.NatGateway.NatGatewayIP.Name {
 			dst.Spec.BastionSpec.AzureBastion.Subnet.NatGateway.NatGatewayIP.IPTags = restored.Spec.BastionSpec.AzureBastion.Subnet.NatGateway.NatGatewayIP.IPTags
 		}
+		dst.Spec.BastionSpec.AzureBastion.Subnet.ServiceEndpoints = restored.Spec.BastionSpec.AzureBastion.Subnet.ServiceEndpoints
 	}
 
 	return nil

diff --git a/api/v1alpha4/zz_generated.conversion.go b/api/v1alpha4/zz_generated.conversion.go
diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go
@@ -51,6 +51,15 @@ const (
 	// https://docs.microsoft.com/en-us/azure/virtual-network/network-security-groups-overview#security-rules
 	minRulePriority = 100
 	maxRulePriority = 4096
+	// Must start with 'Microsoft.', then an alpha character, then can include alnum.
+	serviceEndpointServiceRegexPattern = `^Microsoft\.[a-zA-Z]{1,42}[a-zA-Z0-9]{0,42}$`
+	// Must start with an alpha character and then can include alnum OR be only *.
+	serviceEndpointLocationRegexPattern = `^([a-z]{1,42}\d{0,5}|[*])$`
+)
+
+var (
+	serviceEndpointServiceRegex  = regexp.MustCompile(serviceEndpointServiceRegexPattern)
+	serviceEndpointLocationRegex = regexp.MustCompile(serviceEndpointLocationRegexPattern)
 )
 
 // validateCluster validates a cluster.
@@ -194,6 +203,10 @@ func validateSubnets(subnets Subnets, vnet VnetSpec, fldPath *field.Path) field.
 			}
 		}
 		allErrs = append(allErrs, validateSubnetCIDR(subnet.CIDRBlocks, vnet.CIDRBlocks, fldPath.Index(i).Child("cidrBlocks"))...)
+
+		if len(subnet.ServiceEndpoints) > 0 {
+			allErrs = append(allErrs, validateServiceEndpoints(subnet.ServiceEndpoints, fldPath.Index(i).Child("serviceEndpoints"))...)
+		}
 	}
 	for k, v := range requiredSubnetRoles {
 		if !v {
@@ -548,3 +561,53 @@ func validateClassSpecForControlPlaneOutboundLB(lb *LoadBalancerClassSpec, apise
 
 	return allErrs
 }
+
+func validateServiceEndpoints(serviceEndpoints []ServiceEndpointSpec, fldPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	serviceEndpointsServices := make(map[string]bool, len(serviceEndpoints))
+	for i, se := range serviceEndpoints {
+		if se.Service == "" {
+			allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("service"), "service is required for all service endpoints"))
+		} else {
+			if err := validateServiceEndpointServiceName(se.Service, fldPath.Index(i).Child("service")); err != nil {
+				allErrs = append(allErrs, err)
+			}
+			if _, ok := serviceEndpointsServices[se.Service]; ok {
+				allErrs = append(allErrs, field.Duplicate(fldPath.Index(i).Child("service"), se.Service))
+			}
+			serviceEndpointsServices[se.Service] = true
+		}
+
+		if len(se.Locations) == 0 {
+			allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("locations"), "locations are required for all service endpoints"))
+		} else {
+			serviceEndpointsLocations := make(map[string]bool, len(se.Locations))
+			for j, locationName := range se.Locations {
+				if err := validateServiceEndpointLocationName(locationName, fldPath.Index(i).Child("locations").Index(j)); err != nil {
+					allErrs = append(allErrs, err)
+				}
+				if _, ok := serviceEndpointsLocations[locationName]; ok {
+					allErrs = append(allErrs, field.Duplicate(fldPath.Index(i).Child("locations").Index(j), locationName))
+				}
+				serviceEndpointsLocations[locationName] = true
+			}
+		}
+	}
+
+	return allErrs
+}
+
+func validateServiceEndpointServiceName(serviceName string, fldPath *field.Path) *field.Error {
+	if success := serviceEndpointServiceRegex.MatchString(serviceName); !success {
+		return field.Invalid(fldPath, serviceName, fmt.Sprintf("service name of endpoint service doesn't match regex %s", serviceEndpointServiceRegexPattern))
+	}
+	return nil
+}
+
+func validateServiceEndpointLocationName(location string, fldPath *field.Path) *field.Error {
+	if success := serviceEndpointLocationRegex.MatchString(location); !success {
+		return field.Invalid(fldPath, location, fmt.Sprintf("location doesn't match regex %s", serviceEndpointLocationRegexPattern))
+	}
+	return nil
+}
diff --git a/api/v1beta1/azurecluster_validation_test.go b/api/v1beta1/azurecluster_validation_test.go
@@ -1378,3 +1378,130 @@ func createValidAPIServerInternalLB() LoadBalancerSpec {
 		},
 	}
 }
+
+func TestValidateServiceEndpoints(t *testing.T) {
+	g := NewWithT(t)
+
+	tests := []struct {
+		name             string
+		serviceEndpoints ServiceEndpoints
+		wantErr          bool
+		expectedErr      field.Error
+	}{
+		{
+			name: "valid service endpoint",
+			serviceEndpoints: []ServiceEndpointSpec{{
+				Service:   "Microsoft.Foo",
+				Locations: []string{"*", "eastus2"},
+			}},
+			wantErr: false,
+		},
+		{
+			name: "invalid service endpoint name doesn't start with Microsoft",
+			serviceEndpoints: []ServiceEndpointSpec{{
+				Service:   "Foo",
+				Locations: []string{"*"},
+			}},
+			wantErr: true,
+			expectedErr: field.Error{
+				Type:     "FieldValueInvalid",
+				Field:    "subnets[0].serviceEndpoints[0].service",
+				BadValue: "Foo",
+				Detail:   "service name of endpoint service doesn't match regex ^Microsoft\\.[a-zA-Z]{1,42}[a-zA-Z0-9]{0,42}$",
+			},
+		},
+		{
+			name: "invalid service endpoint name contains invalid characters",
+			serviceEndpoints: []ServiceEndpointSpec{{
+				Service:   "Microsoft.Foo",
+				Locations: []string{"*"},
+			}, {
+				Service:   "Microsoft.Foo-Bar",
+				Locations: []string{"*"},
+			}},
+			wantErr: true,
+			expectedErr: field.Error{
+				Type:     "FieldValueInvalid",
+				Field:    "subnets[0].serviceEndpoints[1].service",
+				BadValue: "Microsoft.Foo-Bar",
+				Detail:   "service name of endpoint service doesn't match regex ^Microsoft\\.[a-zA-Z]{1,42}[a-zA-Z0-9]{0,42}$",
+			},
+		},
+		{
+			name: "invalid service endpoint location contains invalid characters",
+			serviceEndpoints: []ServiceEndpointSpec{{
+				Service:   "Microsoft.Foo",
+				Locations: []string{"*"},
+			}, {
+				Service:   "Microsoft.Bar",
+				Locations: []string{"foo", "foo-bar"},
+			}},
+			wantErr: true,
+			expectedErr: field.Error{
+				Type:     "FieldValueInvalid",
+				Field:    "subnets[0].serviceEndpoints[1].locations[1]",
+				BadValue: "foo-bar",
+				Detail:   "location doesn't match regex ^([a-z]{1,42}\\d{0,5}|[*])$",
+			},
+		},
+	}
+	for _, testCase := range tests {
+		t.Run(testCase.name, func(t *testing.T) {
+			err := validateServiceEndpoints(testCase.serviceEndpoints, field.NewPath("subnets[0].serviceEndpoints"))
+			if testCase.wantErr {
+				// Searches for expected error in list of thrown errors
+				g.Expect(err).To(ContainElement(MatchError(testCase.expectedErr.Error())))
+			} else {
+				g.Expect(err).To(BeEmpty())
+			}
+		})
+	}
+}
+
+func TestServiceEndpointsLackRequiredFieldService(t *testing.T) {
+	g := NewWithT(t)
+
+	type test struct {
+		name             string
+		serviceEndpoints ServiceEndpoints
+	}
+
+	testCase := test{
+		name: "service endpoint missing service name",
+		serviceEndpoints: []ServiceEndpointSpec{{
+			Locations: []string{"*"},
+		}},
+	}
+
+	t.Run(testCase.name, func(t *testing.T) {
+		errs := validateServiceEndpoints(testCase.serviceEndpoints, field.NewPath("subnets[0].serviceEndpoints"))
+		g.Expect(errs).To(HaveLen(1))
+		g.Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
+		g.Expect(errs[0].Field).To(Equal("subnets[0].serviceEndpoints[0].service"))
+		g.Expect(errs[0].Error()).To(ContainSubstring("service is required for all service endpoints"))
+	})
+}
+
+func TestServiceEndpointsLackRequiredFieldLocations(t *testing.T) {
+	g := NewWithT(t)
+
+	type test struct {
+		name             string
+		serviceEndpoints ServiceEndpoints
+	}
+
+	testCase := test{
+		name: "service endpoint missing locations",
+		serviceEndpoints: []ServiceEndpointSpec{{
+			Service: "Microsoft.Foo",
+		}},
+	}
+
+	t.Run(testCase.name, func(t *testing.T) {
+		errs := validateServiceEndpoints(testCase.serviceEndpoints, field.NewPath("subnets[0].serviceEndpoints"))
+		g.Expect(errs).To(HaveLen(1))
+		g.Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
+		g.Expect(errs[0].Field).To(Equal("subnets[0].serviceEndpoints[0].locations"))
+		g.Expect(errs[0].Error()).To(ContainSubstring("locations are required for all service endpoints"))
+	})
+}
diff --git a/api/v1beta1/azurecluster_webhook.go b/api/v1beta1/azurecluster_webhook.go
@@ -128,6 +128,41 @@ func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
 		)
 	}
 
+	oldSubnetMap := make(map[string]SubnetSpec, len(old.Spec.NetworkSpec.Subnets))
+	oldSubnetIndex := make(map[string]int, len(old.Spec.NetworkSpec.Subnets))
+	for i, subnet := range old.Spec.NetworkSpec.Subnets {
+		oldSubnetMap[subnet.Name] = subnet
+		oldSubnetIndex[subnet.Name] = i
+	}
+	for i, subnet := range c.Spec.NetworkSpec.Subnets {
+		if oldSubnet, ok := oldSubnetMap[subnet.Name]; ok {
+			if !reflect.DeepEqual(subnet.CIDRBlocks, oldSubnet.CIDRBlocks) {
+				allErrs = append(allErrs,
+					field.Invalid(field.NewPath("spec", "networkSpec", "subnets").Index(oldSubnetIndex[subnet.Name]).Child("CIDRBlocks"),
+						c.Spec.NetworkSpec.Subnets[i].CIDRBlocks, "field is immutable"),
+				)
+			}
+			if subnet.RouteTable.Name != oldSubnet.RouteTable.Name {
+				allErrs = append(allErrs,
+					field.Invalid(field.NewPath("spec", "networkSpec", "subnets").Index(oldSubnetIndex[subnet.Name]).Child("RouteTable").Child("Name"),
+						c.Spec.NetworkSpec.Subnets[i].RouteTable.Name, "field is immutable"),
+				)
+			}
+			if subnet.NatGateway.Name != oldSubnet.NatGateway.Name {
+				allErrs = append(allErrs,
+					field.Invalid(field.NewPath("spec", "networkSpec", "subnets").Index(oldSubnetIndex[subnet.Name]).Child("NatGateway").Child("Name"),
+						c.Spec.NetworkSpec.Subnets[i].NatGateway.Name, "field is immutable"),
+				)
+			}
+			if subnet.SecurityGroup.Name != oldSubnet.SecurityGroup.Name {
+				allErrs = append(allErrs,
+					field.Invalid(field.NewPath("spec", "networkSpec", "subnets").Index(oldSubnetIndex[subnet.Name]).Child("SecurityGroup").Child("Name"),
+						c.Spec.NetworkSpec.Subnets[i].SecurityGroup.Name, "field is immutable"),
+				)
+			}
+		}
+	}
+
 	if len(allErrs) == 0 {
 		return c.validateCluster(old)
 	}

diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go
@@ -137,6 +137,9 @@ func (v *VnetSpec) IsManaged(clusterName string) bool {
 // Subnets is a slice of Subnet.
 type Subnets []SubnetSpec
 
+// ServiceEndpoints is a slice of string.
+type ServiceEndpoints []ServiceEndpointSpec
+
 // SecurityGroup defines an Azure security group.
 type SecurityGroup struct {
 	// ID is the Azure resource ID of the security group.
@@ -589,9 +592,20 @@ type SubnetSpec struct {
 	// +optional
 	NatGateway NatGateway `json:"natGateway,omitempty"`
 
+	// ServiceEndpoints is a slice of Virtual Network service endpoints to enable for the subnets.
+	// +optional
+	ServiceEndpoints ServiceEndpoints `json:"serviceEndpoints,omitempty"`
+
 	SubnetClassSpec `json:",inline"`
 }
 
+// ServiceEndpointSpec configures an Azure Service Endpoint.
+type ServiceEndpointSpec struct {
+	Service string `json:"service"`
+
+	Locations []string `json:"locations"`
+}
+
 // GetControlPlaneSubnet returns the cluster control plane subnet.
 func (n *NetworkSpec) GetControlPlaneSubnet() (SubnetSpec, error) {
 	for _, sn := range n.Subnets {