Switch to using the secondary key (#3427)

* Switch to using the secondary key * Documentation update * Log the name of the key used * Pass log arg through * Fix import ordering * Shorten line
Azure · Mar 21, 2024 · 07672dd · 07672dd
1 parent c9b7d81
commit 07672dd
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 11 deletions.
diff --git a/cmd/aro/dbtoken.go b/cmd/aro/dbtoken.go
@@ -65,7 +65,8 @@ func dbtoken(ctx context.Context, log *logrus.Entry) error {
 	clientOptions := &policy.ClientOptions{
 		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
 	}
-	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
+	logrusEntry := log.WithField("component", "database")
+	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/aro/monitor.go b/cmd/aro/monitor.go
@@ -91,7 +91,8 @@ func monitor(ctx context.Context, log *logrus.Entry) error {
 	clientOptions := &policy.ClientOptions{
 		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
 	}
-	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
+	logrusEntry := log.WithField("component", "database")
+	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/aro/portal.go b/cmd/aro/portal.go
@@ -101,7 +101,8 @@ func portal(ctx context.Context, log *logrus.Entry, audit *logrus.Entry) error {
 	clientOptions := &policy.ClientOptions{
 		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
 	}
-	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
+	logrusEntry := log.WithField("component", "database")
+	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/aro/rp.go b/cmd/aro/rp.go
@@ -113,7 +113,8 @@ func rp(ctx context.Context, log, audit *logrus.Entry) error {
 	clientOptions := &policy.ClientOptions{
 		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
 	}
-	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
+	logrusEntry := log.WithField("component", "database")
+	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/aro/update_ocp_versions.go b/cmd/aro/update_ocp_versions.go
@@ -192,7 +192,8 @@ func getVersionsDatabase(ctx context.Context, log *logrus.Entry) (database.OpenS
 	clientOptions := &policy.ClientOptions{
 		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
 	}
-	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
+	logrusEntry := log.WithField("component", "database")
+	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
 	if err != nil {
 		return nil, err
 	}

diff --git a/docs/dbtoken-service.md b/docs/dbtoken-service.md
@@ -8,12 +8,13 @@ In brief, there are three options:
 
 1. use r/w or r/o primary keys, which grant access to the whole database account
 2. implement a service which transforms (1) into scoped resource tokens
-3. a third AAD RBAC-based model is in preview.
+3. a third AAD RBAC-based model with Entra Ids.
 
 Currently, the RP, monitoring and portal service share the same security
 boundary (the RP VM) and use option 1.  The dbtoken service, which also runs on
-the RP VM, is our implementation of option 2.  As and when option 3 goes GA, it
-may be possible to retire the dbtoken service.
+the RP VM, is our implementation of option 2.  Option 3 is now in GA and there are plans
+[here](https://issues.redhat.com/browse/ARO-5512) to implement it and replace option 1
+and 2.
 
 The purpose of the dbtoken service at its implementation time is to enable the
 gateway component (which handles end-user traffic) to access the service Cosmos

diff --git a/hack/db/db.go b/hack/db/db.go
@@ -68,7 +68,8 @@ func run(ctx context.Context, log *logrus.Entry) error {
 	clientOptions := &policy.ClientOptions{
 		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
 	}
-	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, tokenCredential, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
+	logrusEntry := log.WithField("component", "database")
+	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, tokenCredential, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/database/database.go b/pkg/database/database.go
@@ -55,7 +55,7 @@ func NewDatabaseClient(log *logrus.Entry, _env env.Core, authorizer cosmosdb.Aut
 	return cosmosdb.NewDatabaseClient(log, c, h, databaseAccountName+"."+_env.Environment().CosmosDBDNSSuffix, authorizer), nil
 }
 
-func NewMasterKeyAuthorizer(ctx context.Context, token azcore.TokenCredential, clientOptions *policy.ClientOptions, subscriptionID, resourceGroup, databaseAccountName string) (cosmosdb.Authorizer, error) {
+func NewMasterKeyAuthorizer(ctx context.Context, log *logrus.Entry, token azcore.TokenCredential, clientOptions *policy.ClientOptions, subscriptionID, resourceGroup, databaseAccountName string) (cosmosdb.Authorizer, error) {
 	databaseaccounts, err := armcosmos.NewDatabaseAccountsClient(subscriptionID, token, clientOptions)
 	if err != nil {
 		return nil, err
@@ -68,7 +68,13 @@ func NewMasterKeyAuthorizer(ctx context.Context, token azcore.TokenCredential, c
 		return nil, err
 	}
 
-	return cosmosdb.NewMasterKeyAuthorizer(*keys.PrimaryMasterKey)
+	return cosmosdb.NewMasterKeyAuthorizer(getDatabaseKey(keys, log))
+}
+
+func getDatabaseKey(keys sdkcosmos.DatabaseAccountsClientListKeysResponse, log *logrus.Entry) string {
+	keyName := "SecondaryMasterKey"
+	log.Infof("Using %s to authenticate with CosmosDB", keyName)
+	return *keys.SecondaryMasterKey
 }
 
 func NewJSONHandle(aead encryption.AEAD) (*codec.JsonHandle, error) {

diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go
@@ -0,0 +1,63 @@
+package database
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"testing"
+
+	sdkcosmos "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cosmos/armcosmos/v2"
+	"github.com/onsi/gomega"
+	"github.com/onsi/gomega/types"
+	"github.com/sirupsen/logrus"
+
+	testlog "github.com/Azure/ARO-RP/test/util/log"
+)
+
+func TestGetDatabaseKey(t *testing.T) {
+	primaryMasterKeyName := "PrimaryMasterKey"
+	primaryReadOnlyMasterKeyName := "PrimaryReadonlyMasterKey"
+	secondaryMasterKeyName := "SecondaryMasterKey"
+	secondaryReadOnlyMasterKeyName := "SecondaryReadonlyMasterKey"
+	for _, tt := range []struct {
+		name        string
+		wantData    string
+		wantEntries []map[string]types.GomegaMatcher
+	}{
+		{
+			name:     "Use correct CosmosDB Key",
+			wantData: secondaryMasterKeyName,
+			wantEntries: []map[string]types.GomegaMatcher{
+				{
+					"level": gomega.Equal(logrus.InfoLevel),
+					"msg":   gomega.ContainSubstring(secondaryMasterKeyName),
+				},
+			},
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			h, log := testlog.New()
+
+			keys := sdkcosmos.DatabaseAccountsClientListKeysResponse{
+				DatabaseAccountListKeysResult: sdkcosmos.DatabaseAccountListKeysResult{
+					PrimaryMasterKey:           &primaryMasterKeyName,
+					PrimaryReadonlyMasterKey:   &primaryReadOnlyMasterKeyName,
+					SecondaryMasterKey:         &secondaryMasterKeyName,
+					SecondaryReadonlyMasterKey: &secondaryReadOnlyMasterKeyName,
+				},
+			}
+
+			result := getDatabaseKey(keys, log)
+			t.Log(result)
+
+			if result != tt.wantData {
+				t.Errorf("Expected %s, got %s", tt.wantData, result)
+			}
+
+			err := testlog.AssertLoggingOutput(h, tt.wantEntries)
+			if err != nil {
+				t.Error(err)
+			}
+		})
+	}
+}