Merge pull request #1099 from Azure/mock-cosmos-db

Mock DBClient with GoMock

.bingo/Variables.mk

@@ -35,3 +35,9 @@ $(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
 	@echo "(re)installing $(GOBIN)/golangci-lint-v1.61.0"
 	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.61.0 "github.com/golangci/golangci-lint/cmd/golangci-lint"
 
+MOCKGEN := $(GOBIN)/mockgen-v0.5.0
+$(MOCKGEN): $(BINGO_DIR)/mockgen.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/mockgen-v0.5.0"
+	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=mockgen.mod -o=$(GOBIN)/mockgen-v0.5.0 "go.uber.org/mock/mockgen"
+

.bingo/mockgen.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.23.3
+
+require go.uber.org/mock v0.5.0 // mockgen

.bingo/mockgen.sum

@@ -0,0 +1,8 @@
+go.uber.org/mock v0.5.0 h1:KAMbZvZPyBPWgD14IrIQ38QCyjwpvVVV6K/bHl1IwQU=
+go.uber.org/mock v0.5.0/go.mod h1:ge71pBPLYDk7QIi1LupWxdAykm7KIEFchiOqd6z7qMM=
+golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0=
+golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
+golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
+golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/tools v0.22.0 h1:gqSGLZqv+AI9lIQzniJ0nZDRG5GBPsSi+DRNHWNz6yA=
+golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c=

.bingo/variables.env

@@ -14,3 +14,5 @@ GOIMPORTS="${GOBIN}/goimports-v0.26.0"
 
 GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.61.0"
 
+MOCKGEN="${GOBIN}/mockgen-v0.5.0"
+

.github/workflows/ci-go.yml

@@ -40,6 +40,16 @@ jobs:
           go-version-file: 'go.work'
           check-latest: true
 
+      - name: 'Regenerate mocks'
+        run: |
+          make mocks
+          if [[ ! -z "$(git status --short)" ]]
+          then
+            echo "there are some modified files"
+            git status
+            exit 1
+          fi
+
       - name: 'Test'
         run: JOB_ID=${{ github.job }} PRINCIPAL_ID=${{ secrets.GHA_PRINCIPAL_ID }} make test
 

Makefile

@@ -18,6 +18,14 @@ test:
 	go list -f '{{.Dir}}/...' -m |RUN_TEMPLATIZE_E2E=true xargs go test -timeout 1200s -tags=$(GOTAGS) -cover
 .PHONY: test
 
+mocks: install-tools
+	MOCKGEN=${MOCKGEN} go generate ./internal/mocks
+.PHONY: mocks
+
+install-tools: $(BINGO)
+	$(BINGO) get
+.PHONY: install-tools
+
 # There is currently no convenient way to run golangci-lint against a whole Go workspace
 # https://github.com/golang/go/issues/50745
 MODULES := $(shell go list -f '{{.Dir}}/...' -m | xargs)

backend/Dockerfile

@@ -1,5 +1,5 @@
 # Builder image installs tools needed to build aro-hcp-backend
-FROM --platform=linux/amd64 mcr.microsoft.com/oss/go/microsoft/golang:1.23-fips-cbl-mariner2.0@sha256:e6951a34cc0cbdc62b5110c4301dfe9c8daf8ee89c1b8616082a6e0b89cd820f as builder
+FROM --platform=linux/amd64 mcr.microsoft.com/oss/go/microsoft/golang:1.23-fips-cbl-mariner2.0@sha256:97d76864911dfbaf6b3387b6e0583abe5af74d7b744773c4facbaf02389e654f as builder
 WORKDIR /app
 ADD archive.tar.gz .
 # https://github.com/microsoft/go/tree/microsoft/main/eng/doc/fips#build-option-to-require-fips-mode

backend/operations_scanner_test.go

@@ -5,16 +5,17 @@ package main
 
 import (
 	"context"
-	"errors"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
+	"go.uber.org/mock/gomock"
 
 	"github.com/Azure/ARO-HCP/internal/api/arm"
 	"github.com/Azure/ARO-HCP/internal/database"
+	"github.com/Azure/ARO-HCP/internal/mocks"
 	"github.com/Azure/ARO-HCP/internal/ocm"
 )
 
@@ -60,6 +61,8 @@ func TestDeleteOperationCompleted(t *testing.T) {
 			var request *http.Request
 
 			ctx := context.Background()
+			ctrl := gomock.NewController(t)
+			mockDBClient := mocks.NewMockDBClient(ctrl)
 
 			resourceID, err := arm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/testCluster")
 			if err != nil {
@@ -74,20 +77,26 @@ func TestDeleteOperationCompleted(t *testing.T) {
 			defer server.Close()
 
 			scanner := &OperationsScanner{
-				dbClient:           database.NewCache(),
+				dbClient:           mockDBClient,
 				notificationClient: server.Client(),
 			}
 
 			operationDoc := database.NewOperationDocument(database.OperationRequestDelete, resourceID, internalID)
 			operationDoc.NotificationURI = server.URL
 			operationDoc.Status = tt.operationStatus
 
-			_ = scanner.dbClient.CreateOperationDoc(ctx, operationDoc)
+			var resourceDocDeleted bool
 
-			if tt.resourceDocPresent {
-				resourceDoc := database.NewResourceDocument(resourceID)
-				_ = scanner.dbClient.CreateResourceDoc(ctx, resourceDoc)
-			}
+			mockDBClient.EXPECT().
+				DeleteResourceDoc(gomock.Any(), resourceID).
+				Do(func(ctx context.Context, resourceID *arm.ResourceID) {
+					resourceDocDeleted = tt.resourceDocPresent
+				})
+			mockDBClient.EXPECT().
+				UpdateOperationDoc(gomock.Any(), operationDoc.ID, gomock.Any()).
+				DoAndReturn(func(ctx context.Context, operationID string, callback func(*database.OperationDocument) bool) (bool, error) {
+					return callback(operationDoc), nil
+				})
 
 			err = scanner.deleteOperationCompleted(ctx, slog.Default(), operationDoc)
 
@@ -103,18 +112,11 @@ func TestDeleteOperationCompleted(t *testing.T) {
 				t.Errorf("Got unexpected error: %v", err)
 			}
 
-			if err == nil && tt.resourceDocPresent {
-				_, getErr := scanner.dbClient.GetResourceDoc(ctx, resourceID)
-				if !errors.Is(getErr, database.ErrNotFound) {
-					t.Error("Expected resource document to be deleted")
-				}
+			if err == nil && tt.resourceDocPresent && !resourceDocDeleted {
+				t.Error("Expected resource document to be deleted")
 			}
 
 			if err == nil && tt.expectAsyncNotification {
-				operationDoc, getErr := scanner.dbClient.GetOperationDoc(ctx, operationDoc.ID)
-				if getErr != nil {
-					t.Fatal(getErr)
-				}
 				if operationDoc.Status != arm.ProvisioningStateSucceeded {
 					t.Errorf("Expected updated operation status to be %s but got %s",
 						arm.ProvisioningStateSucceeded,
@@ -207,6 +209,8 @@ func TestUpdateOperationStatus(t *testing.T) {
 			var request *http.Request
 
 			ctx := context.Background()
+			ctrl := gomock.NewController(t)
+			mockDBClient := mocks.NewMockDBClient(ctrl)
 
 			resourceID, err := arm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/testCluster")
 			if err != nil {
@@ -221,27 +225,41 @@ func TestUpdateOperationStatus(t *testing.T) {
 			defer server.Close()
 
 			scanner := &OperationsScanner{
-				dbClient:           database.NewCache(),
+				dbClient:           mockDBClient,
 				notificationClient: server.Client(),
 			}
 
 			operationDoc := database.NewOperationDocument(database.OperationRequestCreate, resourceID, internalID)
 			operationDoc.NotificationURI = server.URL
 			operationDoc.Status = tt.currentOperationStatus
 
-			_ = scanner.dbClient.CreateOperationDoc(ctx, operationDoc)
+			var resourceDoc *database.ResourceDocument
 
 			if tt.resourceDocPresent {
-				resourceDoc := database.NewResourceDocument(resourceID)
+				resourceDoc = database.NewResourceDocument(resourceID)
 				if tt.resourceMatchOperationID {
 					resourceDoc.ActiveOperationID = operationDoc.ID
 				} else {
 					resourceDoc.ActiveOperationID = "another operation"
 				}
 				resourceDoc.ProvisioningState = tt.resourceProvisioningState
-				_ = scanner.dbClient.CreateResourceDoc(ctx, resourceDoc)
 			}
 
+			mockDBClient.EXPECT().
+				UpdateOperationDoc(gomock.Any(), operationDoc.ID, gomock.Any()).
+				DoAndReturn(func(ctx context.Context, operationID string, callback func(*database.OperationDocument) bool) (bool, error) {
+					return callback(operationDoc), nil
+				})
+			mockDBClient.EXPECT().
+				UpdateResourceDoc(gomock.Any(), resourceID, gomock.Any()).
+				DoAndReturn(func(ctx context.Context, resourceID *arm.ResourceID, callback func(*database.ResourceDocument) bool) (bool, error) {
+					if resourceDoc != nil {
+						return callback(resourceDoc), nil
+					} else {
+						return false, database.ErrNotFound
+					}
+				})
+
 			err = scanner.updateOperationStatus(ctx, slog.Default(), operationDoc, tt.updatedOperationStatus, nil)
 
 			if request == nil && tt.expectAsyncNotification {
@@ -257,10 +275,6 @@ func TestUpdateOperationStatus(t *testing.T) {
 			}
 
 			if err == nil && tt.expectAsyncNotification {
-				operationDoc, getErr := scanner.dbClient.GetOperationDoc(ctx, operationDoc.ID)
-				if getErr != nil {
-					t.Fatal(getErr)
-				}
 				if operationDoc.Status != tt.updatedOperationStatus {
 					t.Errorf("Expected updated operation status to be %s but got %s",
 						tt.updatedOperationStatus,
@@ -269,10 +283,6 @@ func TestUpdateOperationStatus(t *testing.T) {
 			}
 
 			if err == nil && tt.resourceDocPresent {
-				resourceDoc, getErr := scanner.dbClient.GetResourceDoc(ctx, resourceID)
-				if getErr != nil {
-					t.Fatal(getErr)
-				}
 				if resourceDoc.ActiveOperationID == "" && !tt.expectResourceOperationIDCleared {
 					t.Error("Resource's active operation ID is unexpectedly empty")
 				} else if resourceDoc.ActiveOperationID != "" && tt.expectResourceOperationIDCleared {

frontend/Dockerfile

@@ -1,5 +1,5 @@
 # Base and builder image will need to be replaced by Fips compliant one
-FROM --platform=linux/amd64 mcr.microsoft.com/oss/go/microsoft/golang:1.23-fips-cbl-mariner2.0@sha256:e6951a34cc0cbdc62b5110c4301dfe9c8daf8ee89c1b8616082a6e0b89cd820f as builder
+FROM --platform=linux/amd64 mcr.microsoft.com/oss/go/microsoft/golang:1.23-fips-cbl-mariner2.0@sha256:97d76864911dfbaf6b3387b6e0583abe5af74d7b744773c4facbaf02389e654f as builder
 
 WORKDIR /app
 ADD archive.tar.gz .

frontend/Makefile

@@ -19,7 +19,7 @@ info:
 	@echo "COMMIT: ${COMMIT}"
 
 run:
-	./aro-hcp-frontend --use-cache --location ${LOCATION} \
+	./aro-hcp-frontend --location ${LOCATION} \
 		--clusters-service-url http://localhost:8000 \
 		--cluster-service-provision-shard 1 \
 		--cluster-service-noop-provision \

frontend/README.md

@@ -5,11 +5,6 @@
 make frontend
 ```
 
-## Run the frontend binary locally (requires a local running CS to fully function)
-```
-make run
-```
-
 ## Build the frontend container
 ```bash
 # Note: for testing changes, please use your own registry

frontend/cmd/cmd.go

@@ -36,7 +36,6 @@ type FrontendOpts struct {
 	metricsPort int
 	port        int
 
-	useCache   bool
 	cosmosName string
 	cosmosURL  string
 }
@@ -61,7 +60,6 @@ func NewRootCmd() *cobra.Command {
 		},
 	}
 
-	rootCmd.Flags().BoolVar(&opts.useCache, "use-cache", false, "leverage a local cache instead of reaching out to a database")
 	rootCmd.Flags().StringVar(&opts.cosmosName, "cosmos-name", os.Getenv("DB_NAME"), "Cosmos database name")
 	rootCmd.Flags().StringVar(&opts.cosmosURL, "cosmos-url", os.Getenv("DB_URL"), "Cosmos database url")
 	rootCmd.Flags().StringVar(&opts.location, "location", os.Getenv("LOCATION"), "Azure location")
@@ -74,8 +72,6 @@ func NewRootCmd() *cobra.Command {
 	rootCmd.Flags().BoolVar(&opts.clusterServiceNoopProvision, "cluster-service-noop-provision", false, "Skip cluster service provisioning steps for development purposes")
 	rootCmd.Flags().BoolVar(&opts.clusterServiceNoopDeprovision, "cluster-service-noop-deprovision", false, "Skip cluster service deprovisioning steps for development purposes")
 
-	rootCmd.MarkFlagsMutuallyExclusive("use-cache", "cosmos-name")
-	rootCmd.MarkFlagsMutuallyExclusive("use-cache", "cosmos-url")
 	rootCmd.MarkFlagsRequiredTogether("cosmos-name", "cosmos-url")
 
 	return rootCmd
@@ -89,40 +85,36 @@ func (opts *FrontendOpts) Run() error {
 	prometheusEmitter := frontend.NewPrometheusEmitter(prometheus.DefaultRegisterer)
 
 	// Configure database configuration and client
-	dbClient := database.NewCache()
-	if !opts.useCache {
-		var err error
 
-		azcoreClientOptions := azcore.ClientOptions{
-			// FIXME Cloud should be determined by other means.
-			Cloud: cloud.AzurePublic,
-		}
+	azcoreClientOptions := azcore.ClientOptions{
+		// FIXME Cloud should be determined by other means.
+		Cloud: cloud.AzurePublic,
+	}
 
-		credential, err := azidentity.NewDefaultAzureCredential(
-			&azidentity.DefaultAzureCredentialOptions{
-				ClientOptions: azcoreClientOptions,
-			})
-		if err != nil {
-			return err
-		}
+	credential, err := azidentity.NewDefaultAzureCredential(
+		&azidentity.DefaultAzureCredentialOptions{
+			ClientOptions: azcoreClientOptions,
+		})
+	if err != nil {
+		return err
+	}
 
-		cosmosClient, err := azcosmos.NewClient(opts.cosmosURL, credential,
-			&azcosmos.ClientOptions{
-				ClientOptions: azcoreClientOptions,
-			})
-		if err != nil {
-			return err
-		}
+	cosmosClient, err := azcosmos.NewClient(opts.cosmosURL, credential,
+		&azcosmos.ClientOptions{
+			ClientOptions: azcoreClientOptions,
+		})
+	if err != nil {
+		return err
+	}
 
-		cosmosDatabaseClient, err := cosmosClient.NewDatabase(opts.cosmosName)
-		if err != nil {
-			return err
-		}
+	cosmosDatabaseClient, err := cosmosClient.NewDatabase(opts.cosmosName)
+	if err != nil {
+		return err
+	}
 
-		dbClient, err = database.NewCosmosDBClient(context.Background(), cosmosDatabaseClient)
-		if err != nil {
-			return fmt.Errorf("creating the database client failed: %v", err)
-		}
+	dbClient, err := database.NewCosmosDBClient(context.Background(), cosmosDatabaseClient)
+	if err != nil {
+		return fmt.Errorf("creating the database client failed: %v", err)
 	}
 
 	listener, err := net.Listen("tcp4", fmt.Sprintf(":%d", opts.port))