add unit tests

2021-04-30 17:55:57 -05:00 · 2021-04-30 17:55:57 -05:00 · 1dc855ce81
--- a/pkg/cluster/deploystorage_test.go
+++ b/pkg/cluster/deploystorage_test.go
@ -8,15 +8,18 @@ import (
 	"fmt"
 	"testing"

+	mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
 	mgmtfeatures "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-07-01/features"
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/Azure/go-autorest/autorest/azure"
+	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/golang/mock/gomock"
 	"github.com/sirupsen/logrus"

 	"github.com/Azure/ARO-RP/pkg/api"
 	mock_features "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/features"
 	mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env"
+	mock_subnet "github.com/Azure/ARO-RP/pkg/util/mocks/subnet"
 )

 func TestCreateAndUpdateErrors(t *testing.T) {
@ -106,3 +109,76 @@ func TestCreateAndUpdateErrors(t *testing.T) {
 		})
 	}
 }
+
+func TestSetMasterSubnetPolicies(t *testing.T) {
+	ctx := context.Background()
+
+	for _, tt := range []struct {
+		name           string
+		mocks          func(*mock_subnet.MockManager)
+		gatewayEnabled bool
+		wantErr        string
+	}{
+		{
+			name: "ok, !gatewayEnabled",
+			mocks: func(subnet *mock_subnet.MockManager) {
+				subnet.EXPECT().Get(ctx, "subnetID").Return(&mgmtnetwork.Subnet{}, nil)
+				subnet.EXPECT().CreateOrUpdate(ctx, "subnetID", &mgmtnetwork.Subnet{
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						PrivateLinkServiceNetworkPolicies: to.StringPtr("Disabled"),
+					},
+				}).Return(nil)
+			},
+		},
+		{
+			name: "ok, gatewayEnabled",
+			mocks: func(subnet *mock_subnet.MockManager) {
+				subnet.EXPECT().Get(ctx, "subnetID").Return(&mgmtnetwork.Subnet{}, nil)
+				subnet.EXPECT().CreateOrUpdate(ctx, "subnetID", &mgmtnetwork.Subnet{
+					SubnetPropertiesFormat: &mgmtnetwork.SubnetPropertiesFormat{
+						PrivateEndpointNetworkPolicies:    to.StringPtr("Disabled"),
+						PrivateLinkServiceNetworkPolicies: to.StringPtr("Disabled"),
+					},
+				}).Return(nil)
+			},
+			gatewayEnabled: true,
+		},
+		{
+			name: "error",
+			mocks: func(subnet *mock_subnet.MockManager) {
+				subnet.EXPECT().Get(ctx, "subnetID").Return(nil, fmt.Errorf("sad"))
+			},
+			wantErr: "sad",
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			subnet := mock_subnet.NewMockManager(controller)
+			tt.mocks(subnet)
+
+			m := &manager{
+				doc: &api.OpenShiftClusterDocument{
+					OpenShiftCluster: &api.OpenShiftCluster{
+						Properties: api.OpenShiftClusterProperties{
+							MasterProfile: api.MasterProfile{
+								SubnetID: "subnetID",
+							},
+							FeatureProfile: api.FeatureProfile{
+								GatewayEnabled: tt.gatewayEnabled,
+							},
+						},
+					},
+				},
+				subnet: subnet,
+			}
+
+			err := m.setMasterSubnetPolicies(ctx)
+			if err != nil && err.Error() != tt.wantErr ||
+				err == nil && tt.wantErr != "" {
+				t.Error(err)
+			}
+		})
+	}
+}
--- a/pkg/cluster/ipaddresses.go
+++ b/pkg/cluster/ipaddresses.go
@ -5,6 +5,7 @@ package cluster

 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@ -223,23 +224,29 @@ func (m *manager) ensureGatewayCreate(ctx context.Context) error {
 	// that here.
 	var linkIdentifier string
 	for _, conn := range *pls.PrivateEndpointConnections {
-		if !strings.EqualFold(*conn.PrivateEndpoint.ID, *pe.ID) ||
-			strings.EqualFold(*conn.PrivateLinkServiceConnectionState.Status, "Approved") {
+		if !strings.EqualFold(*conn.PrivateEndpoint.ID, *pe.ID) {
 			continue
 		}

-		conn.PrivateLinkServiceConnectionState.Status = to.StringPtr("Approved")
-		conn.PrivateLinkServiceConnectionState.Description = to.StringPtr("Approved")
+		linkIdentifier = *conn.LinkIdentifier

-		_, err = m.rpPrivateLinkServices.UpdatePrivateEndpointConnection(ctx, m.env.GatewayResourceGroup(), "gateway-pls-001", *conn.Name, conn)
-		if err != nil {
-			return err
+		if !strings.EqualFold(*conn.PrivateLinkServiceConnectionState.Status, "Approved") {
+			conn.PrivateLinkServiceConnectionState.Status = to.StringPtr("Approved")
+			conn.PrivateLinkServiceConnectionState.Description = to.StringPtr("Approved")
+
+			_, err = m.rpPrivateLinkServices.UpdatePrivateEndpointConnection(ctx, m.env.GatewayResourceGroup(), "gateway-pls-001", *conn.Name, conn)
+			if err != nil {
+				return err
+			}
 		}

-		linkIdentifier = *conn.LinkIdentifier
 		break
 	}

+	if linkIdentifier == "" {
+		return errors.New("private endpoint connection not found")
+	}
+
 	_, err = m.dbGateway.Create(ctx, &api.GatewayDocument{
 		ID: linkIdentifier,
 		Gateway: &api.Gateway{
--- a/pkg/cluster/ipaddresses_test.go
+++ b/pkg/cluster/ipaddresses_test.go
@ -20,6 +20,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/database/cosmosdb"
 	mock_network "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/network"
 	mock_dns "github.com/Azure/ARO-RP/pkg/util/mocks/dns"
+	mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env"
 	mock_subnet "github.com/Azure/ARO-RP/pkg/util/mocks/subnet"
 	testdatabase "github.com/Azure/ARO-RP/test/database"
 )
@ -653,3 +654,223 @@ func TestUpdateAPIIPEarly(t *testing.T) {
 		})
 	}
 }
+
+func TestEnsureGatewayCreate(t *testing.T) {
+	ctx := context.Background()
+	resourceID := "/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/resourceGroup/providers/Microsoft.RedHatOpenShift/openShiftClusters/resourceName"
+
+	for _, tt := range []struct {
+		name                     string
+		mocks                    func(*mock_env.MockInterface, *mock_network.MockPrivateEndpointsClient, *mock_network.MockPrivateLinkServicesClient)
+		fixture                  func(*testdatabase.Fixture)
+		checker                  func(*testdatabase.Checker)
+		gatewayEnabled           bool
+		gatewayPrivateEndpointIP string
+		wantErr                  string
+	}{
+		{
+			name: "noop: gateway not enabled",
+		},
+		{
+			name:                     "noop: IP set",
+			gatewayPrivateEndpointIP: "1.2.3.4",
+		},
+		{
+			name: "error: private endpoint connection not found",
+			mocks: func(env *mock_env.MockInterface, privateEndpoints *mock_network.MockPrivateEndpointsClient, rpPrivateLinkServices *mock_network.MockPrivateLinkServicesClient) {
+				env.EXPECT().GatewayResourceGroup().AnyTimes().Return("gatewayResourceGroup")
+				privateEndpoints.EXPECT().Get(ctx, "clusterResourceGroup", "infra-pe", "networkInterfaces").Return(mgmtnetwork.PrivateEndpoint{
+					PrivateEndpointProperties: &mgmtnetwork.PrivateEndpointProperties{
+						NetworkInterfaces: &[]mgmtnetwork.Interface{
+							{
+								InterfacePropertiesFormat: &mgmtnetwork.InterfacePropertiesFormat{
+									IPConfigurations: &[]mgmtnetwork.InterfaceIPConfiguration{
+										{
+											InterfaceIPConfigurationPropertiesFormat: &mgmtnetwork.InterfaceIPConfigurationPropertiesFormat{
+												PrivateIPAddress: to.StringPtr("1.2.3.4"),
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+					ID: to.StringPtr("peID"),
+				}, nil)
+				rpPrivateLinkServices.EXPECT().Get(ctx, "gatewayResourceGroup", "gateway-pls-001", "").Return(mgmtnetwork.PrivateLinkService{
+					PrivateLinkServiceProperties: &mgmtnetwork.PrivateLinkServiceProperties{
+						PrivateEndpointConnections: &[]mgmtnetwork.PrivateEndpointConnection{},
+					},
+				}, nil)
+			},
+			gatewayEnabled: true,
+			wantErr:        "private endpoint connection not found",
+		},
+		{
+			name: "ok",
+			mocks: func(env *mock_env.MockInterface, privateEndpoints *mock_network.MockPrivateEndpointsClient, rpPrivateLinkServices *mock_network.MockPrivateLinkServicesClient) {
+				env.EXPECT().GatewayResourceGroup().AnyTimes().Return("gatewayResourceGroup")
+				privateEndpoints.EXPECT().Get(ctx, "clusterResourceGroup", "infra-pe", "networkInterfaces").Return(mgmtnetwork.PrivateEndpoint{
+					PrivateEndpointProperties: &mgmtnetwork.PrivateEndpointProperties{
+						NetworkInterfaces: &[]mgmtnetwork.Interface{
+							{
+								InterfacePropertiesFormat: &mgmtnetwork.InterfacePropertiesFormat{
+									IPConfigurations: &[]mgmtnetwork.InterfaceIPConfiguration{
+										{
+											InterfaceIPConfigurationPropertiesFormat: &mgmtnetwork.InterfaceIPConfigurationPropertiesFormat{
+												PrivateIPAddress: to.StringPtr("1.2.3.4"),
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+					ID: to.StringPtr("peID"),
+				}, nil)
+				rpPrivateLinkServices.EXPECT().Get(ctx, "gatewayResourceGroup", "gateway-pls-001", "").Return(mgmtnetwork.PrivateLinkService{
+					PrivateLinkServiceProperties: &mgmtnetwork.PrivateLinkServiceProperties{
+						PrivateEndpointConnections: &[]mgmtnetwork.PrivateEndpointConnection{
+							{
+								PrivateEndpointConnectionProperties: &mgmtnetwork.PrivateEndpointConnectionProperties{
+									PrivateEndpoint: &mgmtnetwork.PrivateEndpoint{
+										ID: to.StringPtr("otherPeID"),
+									},
+								},
+							},
+							{
+								PrivateEndpointConnectionProperties: &mgmtnetwork.PrivateEndpointConnectionProperties{
+									PrivateEndpoint: &mgmtnetwork.PrivateEndpoint{
+										ID: to.StringPtr("peID"),
+									},
+									PrivateLinkServiceConnectionState: &mgmtnetwork.PrivateLinkServiceConnectionState{
+										Status: to.StringPtr(""),
+									},
+									LinkIdentifier: to.StringPtr("1234"),
+								},
+								Name: to.StringPtr("conn"),
+							},
+						},
+					},
+				}, nil)
+				rpPrivateLinkServices.EXPECT().UpdatePrivateEndpointConnection(ctx, "gatewayResourceGroup", "gateway-pls-001", "conn", mgmtnetwork.PrivateEndpointConnection{
+					PrivateEndpointConnectionProperties: &mgmtnetwork.PrivateEndpointConnectionProperties{
+						PrivateEndpoint: &mgmtnetwork.PrivateEndpoint{
+							ID: to.StringPtr("peID"),
+						},
+						PrivateLinkServiceConnectionState: &mgmtnetwork.PrivateLinkServiceConnectionState{
+							Status:      to.StringPtr("Approved"),
+							Description: to.StringPtr("Approved"),
+						},
+						LinkIdentifier: to.StringPtr("1234"),
+					},
+					Name: to.StringPtr("conn"),
+				}).Return(mgmtnetwork.PrivateEndpointConnection{}, nil)
+			},
+			fixture: func(f *testdatabase.Fixture) {
+				f.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{
+					Key: strings.ToLower(resourceID),
+					OpenShiftCluster: &api.OpenShiftCluster{
+						ID: resourceID,
+					},
+				})
+			},
+			checker: func(c *testdatabase.Checker) {
+				c.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{
+					Key: strings.ToLower(resourceID),
+					OpenShiftCluster: &api.OpenShiftCluster{
+						ID: resourceID,
+						Properties: api.OpenShiftClusterProperties{
+							NetworkProfile: api.NetworkProfile{
+								GatewayPrivateEndpointIP: "1.2.3.4",
+								GatewayPrivateLinkID:     "1234",
+							},
+						},
+					},
+				})
+				c.AddGatewayDocuments(&api.GatewayDocument{
+					ID: "1234",
+					Gateway: &api.Gateway{
+						ID:                              resourceID,
+						StorageSuffix:                   "storageSuffix",
+						ImageRegistryStorageAccountName: "imageRegistryStorageAccountName",
+					},
+				})
+			},
+			gatewayEnabled: true,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			env := mock_env.NewMockInterface(controller)
+			privateEndpoints := mock_network.NewMockPrivateEndpointsClient(controller)
+			rpPrivateLinkServices := mock_network.NewMockPrivateLinkServicesClient(controller)
+
+			dbOpenShiftClusters, clientOpenShiftClusters := testdatabase.NewFakeOpenShiftClusters()
+			dbGateway, clientGateway := testdatabase.NewFakeGateway()
+
+			f := testdatabase.NewFixture().WithOpenShiftClusters(dbOpenShiftClusters).WithGateway(dbGateway)
+			if tt.mocks != nil {
+				tt.mocks(env, privateEndpoints, rpPrivateLinkServices)
+			}
+			if tt.fixture != nil {
+				tt.fixture(f)
+			}
+			err := f.Create()
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			m := &manager{
+				env:       env,
+				db:        dbOpenShiftClusters,
+				dbGateway: dbGateway,
+				doc: &api.OpenShiftClusterDocument{
+					Key: strings.ToLower(resourceID),
+					OpenShiftCluster: &api.OpenShiftCluster{
+						ID: resourceID,
+						Properties: api.OpenShiftClusterProperties{
+							ClusterProfile: api.ClusterProfile{
+								ResourceGroupID: "/clusterResourceGroup",
+							},
+							NetworkProfile: api.NetworkProfile{
+								GatewayPrivateEndpointIP: tt.gatewayPrivateEndpointIP,
+							},
+							FeatureProfile: api.FeatureProfile{
+								GatewayEnabled: tt.gatewayEnabled,
+							},
+							StorageSuffix:                   "storageSuffix",
+							ImageRegistryStorageAccountName: "imageRegistryStorageAccountName",
+							InfraID:                         "infra",
+						},
+					},
+				},
+				privateEndpoints:      privateEndpoints,
+				rpPrivateLinkServices: rpPrivateLinkServices,
+			}
+
+			err = m.ensureGatewayCreate(ctx)
+			if err != nil && err.Error() != tt.wantErr ||
+				err == nil && tt.wantErr != "" {
+				t.Fatal(err)
+			}
+
+			c := testdatabase.NewChecker()
+			if tt.checker != nil {
+				tt.checker(c)
+			}
+
+			errs := c.CheckOpenShiftClusters(clientOpenShiftClusters)
+			for _, err := range errs {
+				t.Error(err)
+			}
+
+			errs = c.CheckGateways(clientGateway)
+			for _, err := range errs {
+				t.Error(err)
+			}
+		})
+	}
+}
--- a/pkg/database/openshiftclusters.go
+++ b/pkg/database/openshiftclusters.go
@ -84,14 +84,14 @@ func NewOpenShiftClusters(ctx context.Context, isLocalDevelopmentMode bool, dbc
 	}

 	documentClient := cosmosdb.NewOpenShiftClusterDocumentClient(collc, collOpenShiftClusters)
-	return NewOpenShiftClustersWithProvidedClient(documentClient, collc), nil
+	return NewOpenShiftClustersWithProvidedClient(documentClient, collc, uuid.Must(uuid.NewV4()).String()), nil
 }

-func NewOpenShiftClustersWithProvidedClient(client cosmosdb.OpenShiftClusterDocumentClient, collectionClient cosmosdb.CollectionClient) OpenShiftClusters {
+func NewOpenShiftClustersWithProvidedClient(client cosmosdb.OpenShiftClusterDocumentClient, collectionClient cosmosdb.CollectionClient, uuid string) OpenShiftClusters {
 	return &openShiftClusters{
 		c:     client,
 		collc: collectionClient,
-		uuid:  uuid.Must(uuid.NewV4()).String(),
+		uuid:  uuid,
 	}
 }

--- a/pkg/database/subscriptions.go
+++ b/pkg/database/subscriptions.go
@ -78,13 +78,13 @@ func NewSubscriptions(ctx context.Context, isLocalDevelopmentMode bool, dbc cosm
 	}

 	documentClient := cosmosdb.NewSubscriptionDocumentClient(collc, collSubscriptions)
-	return NewSubscriptionsWithProvidedClient(documentClient), nil
+	return NewSubscriptionsWithProvidedClient(documentClient, uuid.Must(uuid.NewV4()).String()), nil
 }

-func NewSubscriptionsWithProvidedClient(client cosmosdb.SubscriptionDocumentClient) Subscriptions {
+func NewSubscriptionsWithProvidedClient(client cosmosdb.SubscriptionDocumentClient, uuid string) Subscriptions {
 	return &subscriptions{
 		c:    client,
-		uuid: uuid.Must(uuid.NewV4()).String(),
+		uuid: uuid,
 	}
 }

--- a/test/database/check.go
+++ b/test/database/check.go
@ -22,6 +22,7 @@ type Checker struct {
 	billingDocuments          []*api.BillingDocument
 	asyncOperationDocuments   []*api.AsyncOperationDocument
 	portalDocuments           []*api.PortalDocument
+	gatewayDocuments          []*api.GatewayDocument
 }

 func NewChecker() *Checker {
@ -83,6 +84,17 @@ func (f *Checker) AddPortalDocuments(docs ...*api.PortalDocument) {
 	}
 }

+func (f *Checker) AddGatewayDocuments(docs ...*api.GatewayDocument) {
+	for _, doc := range docs {
+		docCopy, err := deepCopy(doc)
+		if err != nil {
+			panic(err)
+		}
+
+		f.gatewayDocuments = append(f.gatewayDocuments, docCopy.(*api.GatewayDocument))
+	}
+}
+
 func (f *Checker) CheckOpenShiftClusters(openShiftClusters *cosmosdb.FakeOpenShiftClusterDocumentClient) (errs []error) {
 	ctx := context.Background()

@ -194,3 +206,23 @@ func (f *Checker) CheckPortals(portals *cosmosdb.FakePortalDocumentClient) (errs

 	return errs
 }
+
+func (f *Checker) CheckGateways(gateways *cosmosdb.FakeGatewayDocumentClient) (errs []error) {
+	ctx := context.Background()
+
+	all, err := gateways.ListAll(ctx, nil)
+	if err != nil {
+		return []error{err}
+	}
+
+	if len(f.gatewayDocuments) != 0 && len(all.GatewayDocuments) == len(f.gatewayDocuments) {
+		diff := deep.Equal(all.GatewayDocuments, f.gatewayDocuments)
+		for _, i := range diff {
+			errs = append(errs, errors.New(i))
+		}
+	} else if len(all.GatewayDocuments) != 0 || len(f.gatewayDocuments) != 0 {
+		errs = append(errs, fmt.Errorf("gateways length different, %d vs %d", len(all.GatewayDocuments), len(f.gatewayDocuments)))
+	}
+
+	return errs
+}
--- a/test/database/fixtures.go
+++ b/test/database/fixtures.go
@ -18,12 +18,14 @@ type Fixture struct {
 	billingDocuments          []*api.BillingDocument
 	asyncOperationDocuments   []*api.AsyncOperationDocument
 	portalDocuments           []*api.PortalDocument
+	gatewayDocuments          []*api.GatewayDocument

 	openShiftClustersDatabase database.OpenShiftClusters
 	billingDatabase           database.Billing
 	subscriptionsDatabase     database.Subscriptions
 	asyncOperationsDatabase   database.AsyncOperations
 	portalDatabase            database.Portal
+	gatewayDatabase           database.Gateway
 }

 func NewFixture() *Fixture {
@ -55,6 +57,11 @@ func (f *Fixture) WithPortal(db database.Portal) *Fixture {
 	return f
 }

+func (f *Fixture) WithGateway(db database.Gateway) *Fixture {
+	f.gatewayDatabase = db
+	return f
+}
+
 func (f *Fixture) AddOpenShiftClusterDocuments(docs ...*api.OpenShiftClusterDocument) {
 	for _, doc := range docs {
 		docCopy, err := deepCopy(doc)
@ -110,6 +117,17 @@ func (f *Fixture) AddPortalDocuments(docs ...*api.PortalDocument) {
 	}
 }

+func (f *Fixture) AddGatewayDocuments(docs ...*api.GatewayDocument) {
+	for _, doc := range docs {
+		docCopy, err := deepCopy(doc)
+		if err != nil {
+			panic(err)
+		}
+
+		f.gatewayDocuments = append(f.gatewayDocuments, docCopy.(*api.GatewayDocument))
+	}
+}
+
 func (f *Fixture) Create() error {
 	ctx := context.Background()

@ -151,5 +169,12 @@ func (f *Fixture) Create() error {
 		}
 	}

+	for _, i := range f.gatewayDocuments {
+		_, err := f.gatewayDatabase.Create(ctx, i)
+		if err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
--- a/test/database/inmemory.go
+++ b/test/database/inmemory.go
@ -24,14 +24,14 @@ func NewFakeOpenShiftClusters() (db database.OpenShiftClusters, client *cosmosdb
 	coll := &fakeCollectionClient{}
 	client = cosmosdb.NewFakeOpenShiftClusterDocumentClient(jsonHandle)
 	injectOpenShiftClusters(client)
-	db = database.NewOpenShiftClustersWithProvidedClient(client, coll)
+	db = database.NewOpenShiftClustersWithProvidedClient(client, coll, "")
 	return db, client
 }

 func NewFakeSubscriptions() (db database.Subscriptions, client *cosmosdb.FakeSubscriptionDocumentClient) {
 	client = cosmosdb.NewFakeSubscriptionDocumentClient(jsonHandle)
 	injectSubscriptions(client)
-	db = database.NewSubscriptionsWithProvidedClient(client)
+	db = database.NewSubscriptionsWithProvidedClient(client, "")
 	return db, client
 }

@ -53,3 +53,9 @@ func NewFakePortal() (db database.Portal, client *cosmosdb.FakePortalDocumentCli
 	db = database.NewPortalWithProvidedClient(client)
 	return db, client
 }
+
+func NewFakeGateway() (db database.Gateway, client *cosmosdb.FakeGatewayDocumentClient) {
+	client = cosmosdb.NewFakeGatewayDocumentClient(jsonHandle)
+	db = database.NewGatewayWithProvidedClient(client)
+	return db, client
+}