diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index fa6536ab088..89138da931e 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -1057,13 +1057,17 @@ func (s *ClusterScope) SetAnnotation(key, value string) { // PrivateEndpointSpecs returns the private endpoint specs. func (s *ClusterScope) PrivateEndpointSpecs() []azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint] { - var privateEndpointSpecs []azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint] subnetsList := s.AzureCluster.Spec.NetworkSpec.Subnets - + numberOfSubnets := len(subnetsList) if s.IsAzureBastionEnabled() { subnetsList = append(subnetsList, s.AzureCluster.Spec.BastionSpec.AzureBastion.Subnet) + numberOfSubnets++ } + // privateEndpointSpecs will be an empty list if no private endpoints were found. + // We pre-allocate the list to avoid unnecessary allocations during append. + privateEndpointSpecs := make([]azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint], 0, numberOfSubnets) + for _, subnet := range subnetsList { for _, privateEndpoint := range subnet.PrivateEndpoints { privateEndpointSpec := &privateendpoints.PrivateEndpointSpec{ diff --git a/azure/scope/cluster_test.go b/azure/scope/cluster_test.go index 7227135f9d2..9a6993e1752 100644 --- a/azure/scope/cluster_test.go +++ b/azure/scope/cluster_test.go @@ -3447,7 +3447,7 @@ func TestPrivateEndpointSpecs(t *testing.T) { want []azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint] }{ { - name: "returns nil if no subnets are specified", + name: "returns empty private endpoints list if no subnets are specified", clusterScope: ClusterScope{ AzureCluster: &infrav1.AzureCluster{ Spec: infrav1.AzureClusterSpec{ @@ -3458,10 +3458,10 @@ func TestPrivateEndpointSpecs(t *testing.T) { }, cache: &ClusterCache{}, }, - want: nil, + want: make([]azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint], 0), }, { - name: "returns nil if no private endpoints are specified", + name: "returns empty private endpoints list if no private endpoints are specified", clusterScope: ClusterScope{ AzureCluster: &infrav1.AzureCluster{ Spec: infrav1.AzureClusterSpec{ @@ -3478,7 +3478,7 @@ func TestPrivateEndpointSpecs(t *testing.T) { }, cache: &ClusterCache{}, }, - want: nil, + want: make([]azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint], 0), }, { name: "returns list of private endpoint specs if private endpoints are specified", @@ -3610,6 +3610,7 @@ func TestPrivateEndpointSpecs(t *testing.T) { }, }, }, + AdditionalTags: make(infrav1.Tags, 0), }, &privateendpoints.PrivateEndpointSpec{ Name: "my-private-endpoint-2", @@ -3637,6 +3638,7 @@ func TestPrivateEndpointSpecs(t *testing.T) { }, }, }, + AdditionalTags: make(infrav1.Tags, 0), }, &privateendpoints.PrivateEndpointSpec{ Name: "my-private-endpoint-3", @@ -3664,6 +3666,7 @@ func TestPrivateEndpointSpecs(t *testing.T) { }, }, }, + AdditionalTags: make(infrav1.Tags, 0), }, }, }, diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 9407c29b66b..06fcf438492 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -809,11 +809,12 @@ func (s *ManagedControlPlaneScope) AvailabilityStatusFilter(cond *clusterv1.Cond // PrivateEndpointSpecs returns the private endpoint specs. func (s *ManagedControlPlaneScope) PrivateEndpointSpecs() []azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint] { - privateEndpointSpecs := make([]azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint], len(s.ControlPlane.Spec.VirtualNetwork.Subnet.PrivateEndpoints)) + privateEndpointSpecs := make([]azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint], 0, len(s.ControlPlane.Spec.VirtualNetwork.Subnet.PrivateEndpoints)) for _, privateEndpoint := range s.ControlPlane.Spec.VirtualNetwork.Subnet.PrivateEndpoints { privateEndpointSpec := &privateendpoints.PrivateEndpointSpec{ Name: privateEndpoint.Name, + Namespace: s.Cluster.Namespace, ResourceGroup: s.VNetSpec().ResourceGroupName(), Location: privateEndpoint.Location, CustomNetworkInterfaceName: privateEndpoint.CustomNetworkInterfaceName, @@ -839,7 +840,6 @@ func (s *ManagedControlPlaneScope) PrivateEndpointSpecs() []azure.ASOResourceSpe } privateEndpointSpec.PrivateLinkServiceConnections = append(privateEndpointSpec.PrivateLinkServiceConnections, pl) } - privateEndpointSpecs = append(privateEndpointSpecs, privateEndpointSpec) } diff --git a/azure/scope/managedcontrolplane_test.go b/azure/scope/managedcontrolplane_test.go index 48f5668a3b6..8e386e93ad3 100644 --- a/azure/scope/managedcontrolplane_test.go +++ b/azure/scope/managedcontrolplane_test.go @@ -18,8 +18,10 @@ package scope import ( "context" + "reflect" "testing" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" "github.com/Azure/go-autorest/autorest" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,6 +31,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/agentpools" "sigs.k8s.io/cluster-api-provider-azure/azure/services/managedclusters" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/privateendpoints" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -1038,3 +1041,169 @@ func TestAreLocalAccountsDisabled(t *testing.T) { }) } } + +func TestManagedControlPlaneScope_PrivateEndpointSpecs(t *testing.T) { + scheme := runtime.NewScheme() + _ = expv1.AddToScheme(scheme) + _ = infrav1.AddToScheme(scheme) + + cases := []struct { + Name string + Input ManagedControlPlaneScopeParams + Expected []azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint] + Err string + }{ + { + Name: "returns empty private endpoints list if no subnets are specified", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{}, + }, + }, + }, + Expected: make([]azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint], 0), + }, + { + Name: "returns empty private endpoints list if no private endpoints are specified", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + Subnet: infrav1.ManagedControlPlaneSubnet{ + PrivateEndpoints: infrav1.PrivateEndpoints{}, + }, + }, + }, + }, + }, + Expected: make([]azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint], 0), + }, + { + Name: "returns list of private endpoint specs if private endpoints are specified", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "dummy-ns", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "dummy-ns", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000001", + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + ResourceGroup: "dummy-rg", + Name: "vnet1", + Subnet: infrav1.ManagedControlPlaneSubnet{ + Name: "subnet1", + PrivateEndpoints: infrav1.PrivateEndpoints{ + { + Name: "my-private-endpoint", + Location: "westus2", + PrivateLinkServiceConnections: []infrav1.PrivateLinkServiceConnection{ + { + Name: "my-pls-connection", + PrivateLinkServiceID: "my-pls-id", + GroupIDs: []string{ + "my-group-id-1", + }, + RequestMessage: "my-request-message", + }, + }, + CustomNetworkInterfaceName: "my-custom-nic", + PrivateIPAddresses: []string{ + "IP1", + "IP2", + }, + ApplicationSecurityGroups: []string{ + "ASG1", + "ASG2", + }, + ManualApproval: true, + }, + }, + }, + }, + }, + }, + }, + Expected: []azure.ASOResourceSpecGetter[*asonetworkv1.PrivateEndpoint]{ + &privateendpoints.PrivateEndpointSpec{ + Name: "my-private-endpoint", + Namespace: "dummy-ns", + ResourceGroup: "dummy-rg", + Location: "westus2", + CustomNetworkInterfaceName: "my-custom-nic", + PrivateIPAddresses: []string{ + "IP1", + "IP2", + }, + SubnetID: "/subscriptions/00000000-0000-0000-0000-000000000001/resourceGroups/dummy-rg/providers/Microsoft.Network/virtualNetworks/vnet1/subnets/subnet1", + ApplicationSecurityGroups: []string{ + "ASG1", + "ASG2", + }, + ClusterName: "my-cluster", + PrivateLinkServiceConnections: []privateendpoints.PrivateLinkServiceConnection{ + { + Name: "my-pls-connection", + RequestMessage: "my-request-message", + PrivateLinkServiceID: "my-pls-id", + GroupIDs: []string{ + "my-group-id-1", + }, + }, + }, + ManualApproval: true, + AdditionalTags: make(infrav1.Tags, 0), + }, + }, + }, + } + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(c.Input.ControlPlane).Build() + c.Input.Client = fakeClient + s, err := NewManagedControlPlaneScope(context.TODO(), c.Input) + g.Expect(err).To(Succeed()) + if got := s.PrivateEndpointSpecs(); !reflect.DeepEqual(got, c.Expected) { + t.Errorf("PrivateEndpointSpecs() = %s, want %s", specArrayToString(got), specArrayToString(c.Expected)) + } + }) + } +} diff --git a/azure/services/privateendpoints/spec.go b/azure/services/privateendpoints/spec.go index b7d14bf54a7..ca686af6c38 100644 --- a/azure/services/privateendpoints/spec.go +++ b/azure/services/privateendpoints/spec.go @@ -89,7 +89,9 @@ func (s *PrivateEndpointSpec) Parameters(ctx context.Context, existingPrivateEnd privateEndpoint.Spec.AzureName = s.Name // CustomNetworkInterfaceName - privateEndpoint.Spec.CustomNetworkInterfaceName = ptr.To(s.CustomNetworkInterfaceName) + if s.CustomNetworkInterfaceName != "" { + privateEndpoint.Spec.CustomNetworkInterfaceName = ptr.To(s.CustomNetworkInterfaceName) + } // IPConfigurations if len(s.PrivateIPAddresses) > 0 { @@ -105,7 +107,9 @@ func (s *PrivateEndpointSpec) Parameters(ctx context.Context, existingPrivateEnd } // Location - privateEndpoint.Spec.Location = ptr.To(s.Location) + if s.Location != "" { + privateEndpoint.Spec.Location = ptr.To(s.Location) + } // ManualPrivateLinkServiceConnections and PrivateLinkServiceConnections if len(s.PrivateLinkServiceConnections) > 0 { diff --git a/azure/services/privateendpoints/spec_test.go b/azure/services/privateendpoints/spec_test.go index 2a2ffb097dc..bc897a23887 100644 --- a/azure/services/privateendpoints/spec_test.go +++ b/azure/services/privateendpoints/spec_test.go @@ -58,8 +58,7 @@ func TestParameters(t *testing.T) { { name: "Creating a new PrivateEndpoint", spec: &PrivateEndpointSpec{ - Name: privateEndpoint1.Name, - + Name: privateEndpoint1.Name, ResourceGroup: "test-group", ClusterName: "my-cluster", ApplicationSecurityGroups: privateEndpoint1.ApplicationSecurityGroups, @@ -81,10 +80,7 @@ func TestParameters(t *testing.T) { ARMID: "asg1", }, }}, - AzureName: "test-private-endpoint1", - CustomNetworkInterfaceName: ptr.To(""), - IpConfigurations: []asonetworkv1.PrivateEndpointIPConfiguration{}, - Location: ptr.To(""), + AzureName: "test-private-endpoint1", PrivateLinkServiceConnections: []asonetworkv1.PrivateLinkServiceConnection{{ Name: ptr.To(privateEndpoint1.PrivateLinkServiceConnections[0].Name), PrivateLinkServiceReference: &genruntime.ResourceReference{ @@ -107,11 +103,12 @@ func TestParameters(t *testing.T) { }, }, { - name: "User updates an existing private endpoint's Extended Location", + name: "User updates an existing private endpoint's Extended Location and Location", spec: &PrivateEndpointSpec{ Name: privateEndpoint1.Name, Namespace: "test-namespace", ResourceGroup: "test-group", + Location: "test-location", ClusterName: "my-cluster", ApplicationSecurityGroups: nil, PrivateLinkServiceConnections: []PrivateLinkServiceConnection{{ @@ -133,10 +130,7 @@ func TestParameters(t *testing.T) { ARMID: "asg1", }, }}, - AzureName: "test-private-endpoint1", - CustomNetworkInterfaceName: ptr.To(""), - IpConfigurations: []asonetworkv1.PrivateEndpointIPConfiguration{}, - Location: ptr.To(""), + AzureName: "test-private-endpoint1", PrivateLinkServiceConnections: []asonetworkv1.PrivateLinkServiceConnection{{ Name: ptr.To(privateEndpoint1.PrivateLinkServiceConnections[0].Name), PrivateLinkServiceReference: &genruntime.ResourceReference{ @@ -180,10 +174,8 @@ func TestParameters(t *testing.T) { ARMID: "asg1", }, }}, - AzureName: "test-private-endpoint1", - CustomNetworkInterfaceName: ptr.To(""), - IpConfigurations: []asonetworkv1.PrivateEndpointIPConfiguration{}, - Location: ptr.To(""), + AzureName: "test-private-endpoint1", + Location: ptr.To("test-location"), PrivateLinkServiceConnections: []asonetworkv1.PrivateLinkServiceConnection{{ Name: ptr.To(privateEndpoint1.PrivateLinkServiceConnections[0].Name), PrivateLinkServiceReference: &genruntime.ResourceReference{ @@ -219,10 +211,11 @@ func TestParameters(t *testing.T) { { name: "PrivateEndpoint doesn't exist. Creating a new one with manual approval: true", spec: &PrivateEndpointSpec{ - Name: privateEndpoint1Manual.Name, - ResourceGroup: "test-group", - ClusterName: "my-cluster", - ApplicationSecurityGroups: privateEndpoint1Manual.ApplicationSecurityGroups, + Name: privateEndpoint1Manual.Name, + ResourceGroup: "test-group", + ClusterName: "my-cluster", + CustomNetworkInterfaceName: "test-interface-name", + ApplicationSecurityGroups: privateEndpoint1Manual.ApplicationSecurityGroups, PrivateLinkServiceConnections: []PrivateLinkServiceConnection{{ Name: privateEndpoint1Manual.PrivateLinkServiceConnections[0].Name, GroupIDs: privateEndpoint1Manual.PrivateLinkServiceConnections[0].GroupIDs, @@ -234,7 +227,7 @@ func TestParameters(t *testing.T) { }, existing: nil, expect: func(g *WithT, result asonetworkv1.PrivateEndpoint) { - g.Expect(result).To(BeNil()) + g.Expect(result).NotTo(BeNil()) g.Expect(result).To(Equal(asonetworkv1.PrivateEndpoint{ Spec: asonetworkv1.PrivateEndpoint_Spec{ ApplicationSecurityGroups: []asonetworkv1.ApplicationSecurityGroupSpec_PrivateEndpoint_SubResourceEmbedded{{ @@ -243,16 +236,14 @@ func TestParameters(t *testing.T) { }, }}, AzureName: "test-private-endpoint-manual", - CustomNetworkInterfaceName: ptr.To(""), - IpConfigurations: []asonetworkv1.PrivateEndpointIPConfiguration{}, - Location: ptr.To(""), + CustomNetworkInterfaceName: ptr.To("test-interface-name"), ManualPrivateLinkServiceConnections: []asonetworkv1.PrivateLinkServiceConnection{{ - Name: ptr.To(privateEndpoint1.PrivateLinkServiceConnections[0].Name), + Name: ptr.To(privateEndpoint1Manual.PrivateLinkServiceConnections[0].Name), PrivateLinkServiceReference: &genruntime.ResourceReference{ - ARMID: privateEndpoint1.PrivateLinkServiceConnections[0].PrivateLinkServiceID, + ARMID: privateEndpoint1Manual.PrivateLinkServiceConnections[0].PrivateLinkServiceID, }, - GroupIds: privateEndpoint1.PrivateLinkServiceConnections[0].GroupIDs, - RequestMessage: ptr.To(privateEndpoint1.PrivateLinkServiceConnections[0].RequestMessage), + GroupIds: privateEndpoint1Manual.PrivateLinkServiceConnections[0].GroupIDs, + RequestMessage: ptr.To(privateEndpoint1Manual.PrivateLinkServiceConnections[0].RequestMessage), }}, Owner: &genruntime.KnownResourceReference{ Name: "test-group", @@ -263,10 +254,6 @@ func TestParameters(t *testing.T) { }, }, Tags: map[string]string{"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned", "Name": "test-private-endpoint-manual"}, - ExtendedLocation: &asonetworkv1.ExtendedLocation{ - Name: ptr.To("dummy-name"), - Type: ptr.To(asonetworkv1.ExtendedLocationType_EdgeZone), - }, }, })) },