Merge pull request #2213 from AldoFusterTurpin/refactor/simplify_enableServiceEndpoints

Refactor/simplify enable service endpoints
2022-08-09 11:53:16 +10:00 · 2022-08-09 11:53:16 +10:00 · 6fbcd522a2
--- a/pkg/cluster/enableendpoints.go
+++ b/pkg/cluster/enableendpoints.go
@ -0,0 +1,34 @@
+package cluster
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"fmt"
+)
+
+// enableServiceEndpoints should enable service endpoints on
+// subnets for storage account access
+func (m *manager) enableServiceEndpoints(ctx context.Context) error {
+	subnetIds, err := m.getSubnetIds()
+	if err != nil {
+		return err
+	}
+
+	return m.subnet.CreateOrUpdateFromIds(ctx, subnetIds)
+}
+
+func (m *manager) getSubnetIds() ([]string, error) {
+	subnets := []string{
+		m.doc.OpenShiftCluster.Properties.MasterProfile.SubnetID,
+	}
+
+	for _, wp := range m.doc.OpenShiftCluster.Properties.WorkerProfiles {
+		if len(wp.SubnetID) == 0 {
+			return nil, fmt.Errorf("WorkerProfile '%s' has no SubnetID; check that the corresponding MachineSet is valid", wp.Name)
+		}
+		subnets = append(subnets, wp.SubnetID)
+	}
+	return subnets, nil
+}
--- a/pkg/cluster/enableendpoints_test.go
+++ b/pkg/cluster/enableendpoints_test.go
@ -0,0 +1,106 @@
+package cluster
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+
+	"github.com/Azure/ARO-RP/pkg/api"
+	mock_subnet "github.com/Azure/ARO-RP/pkg/util/mocks/subnet"
+)
+
+var (
+	subscriptionId    = "0000000-0000-0000-0000-000000000000"
+	vnetResourceGroup = "vnet-rg"
+	vnetName          = "vnet"
+	subnetNameWorker  = "worker"
+	subnetNameMaster  = "master"
+	subnetIdWorker    = "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameWorker
+	subnetIdMaster    = "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameMaster
+)
+
+func TestEnableServiceEndpointsShouldCall_SubnetManager_CreateOrUpdateFromIds_AndReturnNoError(t *testing.T) {
+	oc := &api.OpenShiftCluster{
+		Properties: api.OpenShiftClusterProperties{
+			MasterProfile: api.MasterProfile{SubnetID: subnetIdMaster},
+			WorkerProfiles: []api.WorkerProfile{
+				{SubnetID: subnetIdWorker},
+			},
+		},
+	}
+
+	subnetIds := []string{oc.Properties.MasterProfile.SubnetID, oc.Properties.WorkerProfiles[0].SubnetID}
+
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	ctx := context.Background()
+
+	subnetManagerMock := mock_subnet.NewMockManager(controller)
+
+	subnetManagerMock.
+		EXPECT().
+		CreateOrUpdateFromIds(ctx, subnetIds).
+		Return(nil)
+
+	m := &manager{
+		subnet: subnetManagerMock,
+		doc: &api.OpenShiftClusterDocument{
+			OpenShiftCluster: oc,
+		},
+	}
+
+	err := m.enableServiceEndpoints(ctx)
+	expectedError := ""
+
+	if (err != nil && err.Error() != expectedError) || (err == nil && expectedError != "") {
+		t.Fatalf("expected error '%v', but got '%v'", expectedError, err)
+	}
+}
+
+func TestEnableServiceEndpointsShouldReturnWorkerProfileHasNoSubnetIdErrorAndShouldNotCall_SubnetManager_CreateOrUpdateFromIds(t *testing.T) {
+	oc := &api.OpenShiftCluster{
+		Properties: api.OpenShiftClusterProperties{
+			MasterProfile: api.MasterProfile{
+				SubnetID: subnetIdMaster,
+			},
+			WorkerProfiles: []api.WorkerProfile{
+				{
+					Name:     "profile_name",
+					SubnetID: "",
+				},
+			},
+		},
+	}
+
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	ctx := context.Background()
+
+	subnetManagerMock := mock_subnet.NewMockManager(controller)
+
+	subnetManagerMock.
+		EXPECT().
+		CreateOrUpdateFromIds(gomock.Any(), gomock.Any()).
+		Times(0).
+		Return(nil)
+
+	m := &manager{
+		subnet: subnetManagerMock,
+		doc: &api.OpenShiftClusterDocument{
+			OpenShiftCluster: oc,
+		},
+	}
+
+	err := m.enableServiceEndpoints(ctx)
+	expectedError := "WorkerProfile 'profile_name' has no SubnetID; check that the corresponding MachineSet is valid"
+
+	if (err != nil && err.Error() != expectedError) || (err == nil && expectedError != "") {
+		t.Fatalf("expected error '%v', but got '%v'", expectedError, err)
+	}
+}
--- a/pkg/cluster/storageaccount_test.go
+++ b/pkg/cluster/storageaccount_test.go
@ -1,138 +0,0 @@
-package cluster
-
-// Copyright (c) Microsoft Corporation.
-// Licensed under the Apache License 2.0.
-
-import (
-	"context"
-	"testing"
-
-	mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
-	"github.com/Azure/go-autorest/autorest/to"
-	"github.com/golang/mock/gomock"
-
-	"github.com/Azure/ARO-RP/pkg/api"
-	mock_subnet "github.com/Azure/ARO-RP/pkg/util/mocks/subnet"
-)
-
-var (
-	subscriptionId    = "0000000-0000-0000-0000-000000000000"
-	vnetResourceGroup = "vnet-rg"
-	vnetName          = "vnet"
-	subnetNameWorker  = "worker"
-	subnetNameMaster  = "master"
-)
-
-func getValidSubnet(endpoints bool, state *mgmtnetwork.ProvisioningState) *mgmtnetwork.Subnet {
-	s := &mgmtnetwork.Subnet{
-		SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{},
-	}
-	if endpoints {
-		s.SubnetPropertiesFormat = &mgmtnetwork.SubnetPropertiesFormat{
-			ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
-				{
-					Service:   to.StringPtr("Microsoft.ContainerRegistry"),
-					Locations: &[]string{"*"},
-				},
-				{
-					Service:   to.StringPtr("Microsoft.Storage"),
-					Locations: &[]string{"*"},
-				},
-			},
-		}
-		if state != nil {
-			for i := range *s.SubnetPropertiesFormat.ServiceEndpoints {
-				(*s.SubnetPropertiesFormat.ServiceEndpoints)[i].ProvisioningState = *state
-			}
-		}
-	}
-	return s
-}
-
-func TestEnableServiceEndpoints(t *testing.T) {
-	ctx := context.Background()
-
-	type test struct {
-		name string
-		oc   *api.OpenShiftCluster
-		mock func(subnetMock *mock_subnet.MockManager, tt test)
-	}
-
-	for _, tt := range []test{
-		{
-			name: "nothing to do",
-			oc: &api.OpenShiftCluster{
-				Properties: api.OpenShiftClusterProperties{
-					MasterProfile: api.MasterProfile{
-						SubnetID: "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameMaster,
-					},
-					WorkerProfiles: []api.WorkerProfile{
-						{
-							SubnetID: "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameWorker,
-						},
-					},
-				},
-			},
-			mock: func(subnetClient *mock_subnet.MockManager, tt test) {
-				subnets := []string{
-					tt.oc.Properties.MasterProfile.SubnetID,
-				}
-				for _, wp := range tt.oc.Properties.WorkerProfiles {
-					subnets = append(subnets, wp.SubnetID)
-				}
-
-				for _, subnetId := range subnets {
-					state := mgmtnetwork.Succeeded
-					subnetClient.EXPECT().Get(gomock.Any(), subnetId).Return(getValidSubnet(true, &state), nil)
-				}
-			},
-		},
-		{
-			name: "enable endpoints",
-			oc: &api.OpenShiftCluster{
-				Properties: api.OpenShiftClusterProperties{
-					MasterProfile: api.MasterProfile{
-						SubnetID: "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameMaster,
-					},
-					WorkerProfiles: []api.WorkerProfile{
-						{
-							SubnetID: "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameWorker,
-						},
-					},
-				},
-			},
-			mock: func(subnetClient *mock_subnet.MockManager, tt test) {
-				subnets := []string{
-					tt.oc.Properties.MasterProfile.SubnetID,
-				}
-				for _, wp := range tt.oc.Properties.WorkerProfiles {
-					subnets = append(subnets, wp.SubnetID)
-				}
-
-				for _, subnetId := range subnets {
-					subnetClient.EXPECT().Get(gomock.Any(), subnetId).Return(getValidSubnet(false, nil), nil)
-					subnetClient.EXPECT().CreateOrUpdate(gomock.Any(), subnetId, getValidSubnet(true, nil)).Return(nil)
-				}
-			},
-		},
-	} {
-		t.Run(tt.name, func(t *testing.T) {
-			controller := gomock.NewController(t)
-			defer controller.Finish()
-
-			subnetClient := mock_subnet.NewMockManager(controller)
-
-			tt.mock(subnetClient, tt)
-
-			m := &manager{
-				subnet: subnetClient,
-				doc: &api.OpenShiftClusterDocument{
-					OpenShiftCluster: tt.oc,
-				},
-			}
-
-			// we don't test errors as all of them would be out of our control
-			_ = m.enableServiceEndpoints(ctx)
-		})
-	}
-}
--- a/pkg/cluster/storageaccounts.go
+++ b/pkg/cluster/storageaccounts.go
@ -5,11 +5,7 @@ package cluster

 import (
 	"context"
-	"fmt"
-	"strings"

-	mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
-	"github.com/Azure/go-autorest/autorest/to"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

 	"github.com/Azure/ARO-RP/pkg/api"
@ -17,59 +13,6 @@ import (
 	"github.com/Azure/ARO-RP/pkg/util/stringutils"
 )

-// enableServiceEndpoints should enable service endpoints on
-// subnets for storage account access
-func (m *manager) enableServiceEndpoints(ctx context.Context) error {
-	subnets := []string{
-		m.doc.OpenShiftCluster.Properties.MasterProfile.SubnetID,
-	}
-
-	for _, wp := range m.doc.OpenShiftCluster.Properties.WorkerProfiles {
-		if len(wp.SubnetID) > 0 {
-			subnets = append(subnets, wp.SubnetID)
-		} else {
-			return fmt.Errorf("WorkerProfile '%s' has no SubnetID; check that the corresponding MachineSet is valid", wp.Name)
-		}
-	}
-
-	for _, subnetId := range subnets {
-		subnet, err := m.subnet.Get(ctx, subnetId)
-		if err != nil {
-			return err
-		}
-
-		var changed bool
-		for _, endpoint := range api.SubnetsEndpoints {
-			var found bool
-			if subnet != nil && subnet.ServiceEndpoints != nil {
-				for _, se := range *subnet.ServiceEndpoints {
-					if strings.EqualFold(*se.Service, endpoint) &&
-						se.ProvisioningState == mgmtnetwork.Succeeded {
-						found = true
-					}
-				}
-			}
-			if !found {
-				if subnet.ServiceEndpoints == nil {
-					subnet.ServiceEndpoints = &[]mgmtnetwork.ServiceEndpointPropertiesFormat{}
-				}
-				*subnet.ServiceEndpoints = append(*subnet.ServiceEndpoints, mgmtnetwork.ServiceEndpointPropertiesFormat{
-					Service:   to.StringPtr(endpoint),
-					Locations: &[]string{"*"},
-				})
-				changed = true
-			}
-		}
-		if changed {
-			err := m.subnet.CreateOrUpdate(ctx, subnetId, subnet)
-			if err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
 // migrateStorageAccounts redeploys storage accounts with firewall rules preventing external access
 // The encryption flag is set to false/disabled for legacy storage accounts.
 func (m *manager) migrateStorageAccounts(ctx context.Context) error {
--- a/pkg/util/mocks/subnet/subnet.go
+++ b/pkg/util/mocks/subnet/subnet.go
@ -51,6 +51,20 @@ func (mr *MockManagerMockRecorder) CreateOrUpdate(arg0, arg1, arg2 interface{})
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*MockManager)(nil).CreateOrUpdate), arg0, arg1, arg2)
 }

+// CreateOrUpdateFromIds mocks base method.
+func (m *MockManager) CreateOrUpdateFromIds(arg0 context.Context, arg1 []string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "CreateOrUpdateFromIds", arg0, arg1)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// CreateOrUpdateFromIds indicates an expected call of CreateOrUpdateFromIds.
+func (mr *MockManagerMockRecorder) CreateOrUpdateFromIds(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateFromIds", reflect.TypeOf((*MockManager)(nil).CreateOrUpdateFromIds), arg0, arg1)
+}
+
 // Get mocks base method.
 func (m *MockManager) Get(arg0 context.Context, arg1 string) (*network.Subnet, error) {
 	m.ctrl.T.Helper()
@ -66,6 +80,21 @@ func (mr *MockManagerMockRecorder) Get(arg0, arg1 interface{}) *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockManager)(nil).Get), arg0, arg1)
 }

+// GetAll mocks base method.
+func (m *MockManager) GetAll(arg0 context.Context, arg1 []string) ([]*network.Subnet, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetAll", arg0, arg1)
+	ret0, _ := ret[0].([]*network.Subnet)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// GetAll indicates an expected call of GetAll.
+func (mr *MockManagerMockRecorder) GetAll(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAll", reflect.TypeOf((*MockManager)(nil).GetAll), arg0, arg1)
+}
+
 // GetHighestFreeIP mocks base method.
 func (m *MockManager) GetHighestFreeIP(arg0 context.Context, arg1 string) (string, error) {
 	m.ctrl.T.Helper()
--- a/pkg/util/subnet/addendpoints.go
+++ b/pkg/util/subnet/addendpoints.go
@ -0,0 +1,71 @@
+package subnet
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+import (
+	"strings"
+
+	mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	"github.com/Azure/go-autorest/autorest/to"
+)
+
+// addEndpointsToSubnets adds the endpoints (that either are missing in subnets
+// or aren't in succeded state in subnets) to the subnets and returns those updated subnets.
+// This method does not talk to any external dependecies to remain pure bussiness logic.
+// The result of this function should be passed to a subnet manager to update the subnets
+// in Azure.
+func addEndpointsToSubnets(endpoints []string, subnets []*mgmtnetwork.Subnet) (subnetsToBeUpdated []*mgmtnetwork.Subnet) {
+	for _, subnet := range subnets {
+		if subnetChanged := addEndpointsToSubnet(endpoints, subnet); subnetChanged {
+			subnetsToBeUpdated = append(subnetsToBeUpdated, subnet)
+		}
+	}
+
+	return subnetsToBeUpdated
+}
+
+// addEndpointsToSubnet adds the endpoints (that either are missing in subnet
+// or aren't in succeded state in the subnet) to the subnet and returns the updated subnet
+func addEndpointsToSubnet(endpoints []string, subnet *mgmtnetwork.Subnet) (subnetChanged bool) {
+	for _, endpoint := range endpoints {
+		endpointFound, serviceEndpointPtr := subnetContainsEndpoint(subnet, endpoint)
+
+		if !endpointFound || serviceEndpointPtr.ProvisioningState != mgmtnetwork.Succeeded {
+			addEndpointToSubnet(endpoint, subnet)
+			subnetChanged = true
+		}
+	}
+
+	return subnetChanged
+}
+
+// subnetContainsEndpoint returns false and nil if subnet does not contain the endpoint.
+// If the subnet does contain the endpoint, true and a pointer to the service endpoint
+// is returned to be able to do additional checks and perform actions accordingly.
+func subnetContainsEndpoint(subnet *mgmtnetwork.Subnet, endpoint string) (endpointFound bool, serviceEndpointPtr *mgmtnetwork.ServiceEndpointPropertiesFormat) {
+	if subnet == nil || subnet.ServiceEndpoints == nil {
+		return false, nil
+	}
+
+	for _, serviceEndpoint := range *subnet.ServiceEndpoints {
+		if endpointFound = strings.EqualFold(*serviceEndpoint.Service, endpoint); endpointFound {
+			return true, &serviceEndpoint
+		}
+	}
+
+	return false, nil
+}
+
+// addEndpointToSubnet appends the endpoint to the slice of ServiceEndpoints of the subnet.
+func addEndpointToSubnet(endpoint string, subnet *mgmtnetwork.Subnet) {
+	if subnet.ServiceEndpoints == nil {
+		subnet.ServiceEndpoints = &[]mgmtnetwork.ServiceEndpointPropertiesFormat{}
+	}
+
+	serviceEndpoint := mgmtnetwork.ServiceEndpointPropertiesFormat{
+		Service:   to.StringPtr(endpoint),
+		Locations: &[]string{"*"},
+	}
+
+	*subnet.ServiceEndpoints = append(*subnet.ServiceEndpoints, serviceEndpoint)
+}
--- a/pkg/util/subnet/addendpoints_test.go
+++ b/pkg/util/subnet/addendpoints_test.go
@ -0,0 +1,239 @@
+package subnet
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"reflect"
+	"testing"
+
+	mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	"github.com/Azure/go-autorest/autorest/to"
+)
+
+func TestAddEndpointsToSubnets(t *testing.T) {
+	var (
+		subscriptionId    = "0000000-0000-0000-0000-000000000000"
+		vnetResourceGroup = "vnet-rg"
+		vnetName          = "vnet"
+		subnetNameWorker  = "worker"
+		subnetNameMaster  = "master"
+		subnetIdWorker    = "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameWorker
+		subnetIdMaster    = "/subscriptions/" + subscriptionId + "/resourceGroups/" + vnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + vnetName + "/subnets/" + subnetNameMaster
+	)
+
+	type testData struct {
+		name            string
+		subnets         []*mgmtnetwork.Subnet
+		newEndpoints    []string
+		expectedSubnets []*mgmtnetwork.Subnet
+	}
+
+	tt := []testData{
+		{
+			name:            "addEndpointsToSubnets should return nil as subnets is nil",
+			subnets:         nil,
+			newEndpoints:    []string{"Microsoft.ContainerRegistry", "Microsoft.Storage"},
+			expectedSubnets: nil,
+		},
+		{
+			name:            "addEndpointsToSubnets should return nil as subnets is an empty slice",
+			subnets:         []*mgmtnetwork.Subnet{},
+			newEndpoints:    []string{"Microsoft.ContainerRegistry", "Microsoft.Storage"},
+			expectedSubnets: nil,
+		},
+		{
+			name: "addEndpointsToSubnets should return nil as all subnets contain all new endpoints and those are in succeeded state",
+			subnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:           to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Succeeded,
+							},
+							{
+								Service:           to.StringPtr("Microsoft.Storage"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Succeeded,
+							},
+						},
+					},
+				},
+				{
+					ID: to.StringPtr(subnetIdWorker),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:           to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Succeeded,
+							},
+							{
+								Service:           to.StringPtr("Microsoft.Storage"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Succeeded,
+							},
+						},
+					},
+				},
+			},
+			newEndpoints:    []string{"Microsoft.ContainerRegistry", "Microsoft.Storage"},
+			expectedSubnets: nil,
+		},
+		{
+			name: "addEndpointsToSubnets should return a new updated Subnet because the original subnet's service endpoints is empty",
+			subnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{},
+					},
+				},
+			},
+			newEndpoints: []string{"Microsoft.ContainerRegistry", "Microsoft.Storage"},
+			expectedSubnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:   to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations: &[]string{"*"},
+							},
+							{
+								Service:   to.StringPtr("Microsoft.Storage"),
+								Locations: &[]string{"*"},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "addEndpointsToSubnets should return a new updated Subnet because the original subnet's service endpoints is nil",
+			subnets: []*mgmtnetwork.Subnet{
+				{
+					ID:                     to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{},
+				},
+			},
+			newEndpoints: []string{"Microsoft.ContainerRegistry", "Microsoft.Storage"},
+			expectedSubnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:   to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations: &[]string{"*"},
+							},
+							{
+								Service:   to.StringPtr("Microsoft.Storage"),
+								Locations: &[]string{"*"},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "addEndpointsToSubnets should return an updated Subnet (with 4 endpoints: 2 previous in failed state + 2 new) as subnet contains all new endpoints but those are not in succeeded state. ",
+			subnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:           to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Failed,
+							},
+							{
+								Service:           to.StringPtr("Microsoft.Storage"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Failed,
+							},
+						},
+					},
+				},
+			},
+			newEndpoints: []string{"Microsoft.ContainerRegistry", "Microsoft.Storage"},
+			expectedSubnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:           to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Failed,
+							},
+							{
+								Service:           to.StringPtr("Microsoft.Storage"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Failed,
+							},
+							{
+								Service:   to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations: &[]string{"*"},
+							},
+							{
+								Service:   to.StringPtr("Microsoft.Storage"),
+								Locations: &[]string{"*"},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "addEndpointsToSubnets should return an updated Subnet (with 2 endpoints: 1 previous was already in succeeded state + 1 new (it was missing))",
+			subnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:           to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Succeeded,
+							},
+						},
+					},
+				},
+			},
+			newEndpoints: []string{"Microsoft.ContainerRegistry", "Microsoft.Storage"},
+			expectedSubnets: []*mgmtnetwork.Subnet{
+				{
+					ID: to.StringPtr(subnetIdMaster),
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						ServiceEndpoints: &[]mgmtnetwork.ServiceEndpointPropertiesFormat{
+							{
+								Service:           to.StringPtr("Microsoft.ContainerRegistry"),
+								Locations:         &[]string{"*"},
+								ProvisioningState: mgmtnetwork.Succeeded,
+							},
+							{
+								Service:   to.StringPtr("Microsoft.Storage"),
+								Locations: &[]string{"*"},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			subnetsToBeUpdated := addEndpointsToSubnets(tc.newEndpoints, tc.subnets)
+
+			if !reflect.DeepEqual(tc.expectedSubnets, subnetsToBeUpdated) {
+				t.Fatalf("expected subnets is different than subnetsToBeUpdated. Expected %v, but got %v", tc.expectedSubnets, subnetsToBeUpdated)
+			}
+		})
+	}
+}
--- a/pkg/util/subnet/subnet.go
+++ b/pkg/util/subnet/subnet.go
@ -26,9 +26,12 @@ type Subnet struct {

 type Manager interface {
 	Get(ctx context.Context, subnetID string) (*mgmtnetwork.Subnet, error)
+	GetAll(ctx context.Context, subnetIds []string) ([]*mgmtnetwork.Subnet, error)
 	GetHighestFreeIP(ctx context.Context, subnetID string) (string, error)
 	CreateOrUpdate(ctx context.Context, subnetID string, subnet *mgmtnetwork.Subnet) error
+	CreateOrUpdateFromIds(ctx context.Context, subnetIds []string) error
 }
+
 type manager struct {
 	subnets         network.SubnetsClient
 	virtualNetworks network.VirtualNetworksClient
@ -121,6 +124,44 @@ func (m *manager) CreateOrUpdate(ctx context.Context, subnetID string, subnet *m
 	return m.subnets.CreateOrUpdateAndWait(ctx, r.ResourceGroup, r.ResourceName, subnetName, *subnet)
 }

+func (m *manager) GetAll(ctx context.Context, subnetIds []string) ([]*mgmtnetwork.Subnet, error) {
+	if len(subnetIds) == 0 {
+		return nil, nil
+	}
+
+	subnets := make([]*mgmtnetwork.Subnet, len(subnetIds))
+
+	for i, subnetId := range subnetIds {
+		subnet, err := m.Get(ctx, subnetId)
+		if err != nil {
+			return nil, err
+		}
+
+		subnets[i] = subnet
+	}
+	return subnets, nil
+}
+
+func (m *manager) CreateOrUpdateFromIds(ctx context.Context, subnetIds []string) error {
+	subnets, err := m.GetAll(ctx, subnetIds)
+	if err != nil {
+		return err
+	}
+
+	subnetsToBeUpdated := addEndpointsToSubnets(api.SubnetsEndpoints, subnets)
+
+	return m.createOrUpdateSubnets(ctx, subnetsToBeUpdated)
+}
+
+func (m *manager) createOrUpdateSubnets(ctx context.Context, subnets []*mgmtnetwork.Subnet) error {
+	for _, subnet := range subnets {
+		if err := m.CreateOrUpdate(ctx, *subnet.ID, subnet); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Split splits the given subnetID into a vnetID and subnetName
 func Split(subnetID string) (string, string, error) {
 	parts := strings.Split(subnetID, "/")