add upgrade action to fix NSG on private clusters

pkg/install/fixnsg.go

@@ -0,0 +1,58 @@
+package install
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+
+	mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-07-01/network"
+
+	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/util/stringutils"
+	"github.com/Azure/ARO-RP/pkg/util/subnet"
+)
+
+func (i *Installer) fixNSG(ctx context.Context) error {
+	if i.doc.OpenShiftCluster.Properties.APIServerProfile.Visibility == api.VisibilityPublic {
+		return nil
+	}
+
+	infraID := i.doc.OpenShiftCluster.Properties.InfraID
+	if infraID == "" {
+		infraID = "aro"
+	}
+
+	resourceGroup := stringutils.LastTokenByte(i.doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/')
+
+	nsg, err := i.securitygroups.Get(ctx, resourceGroup, infraID+subnet.NSGControlPlaneSuffix, "")
+	if err != nil {
+		return err
+	}
+
+	if nsg.SecurityGroupPropertiesFormat == nil ||
+		nsg.SecurityRules == nil {
+		return nil
+	}
+
+	rules := make([]mgmtnetwork.SecurityRule, 0, len(*nsg.SecurityRules))
+
+	for _, rule := range *nsg.SecurityGroupPropertiesFormat.SecurityRules {
+		if rule.SecurityRulePropertiesFormat != nil &&
+			rule.Protocol == mgmtnetwork.SecurityRuleProtocolTCP &&
+			rule.DestinationPortRange != nil &&
+			*rule.DestinationPortRange == "6443" {
+			continue
+		}
+
+		rules = append(rules, rule)
+	}
+
+	if len(rules) == len(*nsg.SecurityRules) {
+		return nil
+	}
+
+	nsg.SecurityRules = &rules
+
+	return i.securitygroups.CreateOrUpdateAndWait(ctx, resourceGroup, infraID+subnet.NSGControlPlaneSuffix, nsg)
+}

pkg/install/fixnsg_test.go

@@ -0,0 +1,105 @@
+package install
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-07-01/network"
+	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/golang/mock/gomock"
+
+	"github.com/Azure/ARO-RP/pkg/api"
+	mock_network "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/network"
+)
+
+func TestFixNSG(t *testing.T) {
+	ctx := context.Background()
+	subscriptionID := "af848f0a-dbe3-449f-9ccd-6f23ac6ef9f1"
+
+	tests := []struct {
+		name       string
+		infraID    string
+		visibility api.Visibility
+		mocks      func(*mock_network.MockSecurityGroupsClient)
+		wantErr    string
+	}{
+		{
+			name:       "private/good",
+			infraID:    "test",
+			visibility: api.VisibilityPrivate,
+			mocks: func(nsgc *mock_network.MockSecurityGroupsClient) {
+				nsgc.EXPECT().Get(gomock.Any(), "test-cluster", "test-controlplane-nsg", "").Return(
+					mgmtnetwork.SecurityGroup{}, nil)
+			},
+		},
+		{
+			name:       "private/needs fix",
+			infraID:    "test",
+			visibility: api.VisibilityPrivate,
+			mocks: func(nsgc *mock_network.MockSecurityGroupsClient) {
+				nsgc.EXPECT().Get(gomock.Any(), "test-cluster", "test-controlplane-nsg", "").Return(
+					mgmtnetwork.SecurityGroup{
+						SecurityGroupPropertiesFormat: &mgmtnetwork.SecurityGroupPropertiesFormat{
+							SecurityRules: &[]mgmtnetwork.SecurityRule{
+								{
+									SecurityRulePropertiesFormat: &mgmtnetwork.SecurityRulePropertiesFormat{
+										Protocol:             mgmtnetwork.SecurityRuleProtocolTCP,
+										DestinationPortRange: to.StringPtr("6443"),
+									},
+								},
+							},
+						},
+					}, nil)
+
+				nsgc.EXPECT().CreateOrUpdateAndWait(gomock.Any(), "test-cluster", "test-controlplane-nsg",
+					mgmtnetwork.SecurityGroup{
+						SecurityGroupPropertiesFormat: &mgmtnetwork.SecurityGroupPropertiesFormat{
+							SecurityRules: &[]mgmtnetwork.SecurityRule{},
+						},
+					}).Return(nil)
+			},
+		},
+		{
+			name:       "public/good",
+			visibility: api.VisibilityPublic,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			securitygroupsClient := mock_network.NewMockSecurityGroupsClient(controller)
+			if tt.mocks != nil {
+				tt.mocks(securitygroupsClient)
+			}
+
+			i := &Installer{
+				securitygroups: securitygroupsClient,
+				doc: &api.OpenShiftClusterDocument{
+					OpenShiftCluster: &api.OpenShiftCluster{
+						Properties: api.OpenShiftClusterProperties{
+							InfraID: tt.infraID,
+							ClusterProfile: api.ClusterProfile{
+								ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", subscriptionID),
+							},
+							APIServerProfile: api.APIServerProfile{
+								Visibility: tt.visibility,
+							},
+						},
+					},
+				},
+			}
+
+			err := i.fixNSG(ctx)
+			if err != nil && err.Error() != tt.wantErr ||
+				err == nil && tt.wantErr != "" {
+				t.Error(err)
+			}
+		})
+	}
+}

pkg/install/install.go

@@ -68,6 +68,7 @@ type Installer struct {
 	interfaces        network.InterfacesClient
 	publicipaddresses network.PublicIPAddressesClient
 	loadbalancers     network.LoadBalancersClient
+	securitygroups    network.SecurityGroupsClient
 	deployments       features.DeploymentsClient
 	groups            features.ResourceGroupsClient
 	accounts          storage.AccountsClient
@@ -136,6 +137,7 @@ func NewInstaller(ctx context.Context, log *logrus.Entry, _env env.Interface, db
 		interfaces:        network.NewInterfacesClient(r.SubscriptionID, fpAuthorizer),
 		publicipaddresses: network.NewPublicIPAddressesClient(r.SubscriptionID, fpAuthorizer),
 		loadbalancers:     network.NewLoadBalancersClient(r.SubscriptionID, fpAuthorizer),
+		securitygroups:    network.NewSecurityGroupsClient(r.SubscriptionID, fpAuthorizer),
 		deployments:       features.NewDeploymentsClient(r.SubscriptionID, fpAuthorizer),
 		groups:            features.NewResourceGroupsClient(r.SubscriptionID, fpAuthorizer),
 		accounts:          storage.NewAccountsClient(r.SubscriptionID, fpAuthorizer),
@@ -154,6 +156,7 @@ func (i *Installer) AdminUpgrade(ctx context.Context) error {
 		condition{i.apiServersReady, 30 * time.Minute},
 		action(i.ensureBillingRecord), // belt and braces
 		action(i.fixLBProbes),
+		action(i.fixNSG),
 		action(i.fixPullSecret),
 		action(i.ensureGenevaLogging),
 		action(i.ensureIfReload),