Clean up some duplicated code in cmd/ (#3648)

* move some repeated code into pkg/util/service/ * cleanups in cmd/aro * update_ocp_versions does not need AEAD * cache the authorisers rather than recreating them * env mock updates * move stuff around from review
Azure · Jul 17, 2024 · 81f22cb · 81f22cb
1 parent 94cd8d7
commit 81f22cb
Show file tree

Hide file tree

Showing 15 changed files with 170 additions and 201 deletions.
diff --git a/cmd/aro/const.go b/cmd/aro/const.go
@@ -4,9 +4,6 @@ package main
 // Licensed under the Apache License 2.0.
 
 const (
-	envDatabaseName                     = "DATABASE_NAME"
-	envDatabaseAccountName              = "DATABASE_ACCOUNT_NAME"
-	envKeyVaultPrefix                   = "KEYVAULT_PREFIX"
 	envOpenShiftVersions                = "OPENSHIFT_VERSIONS"
 	envInstallerImageDigests            = "INSTALLER_IMAGE_DIGESTS"
 	envPlatformWorkloadIdentityRoleSets = "PLATFORM_WORKLOAD_IDENTITY_ROLE_SETS"

diff --git a/cmd/aro/gateway.go b/cmd/aro/gateway.go
@@ -5,7 +5,6 @@ package main
 
 import (
 	"context"
-	"fmt"
 	"os"
 	"os/signal"
 	"syscall"
@@ -35,28 +34,12 @@ func gateway(ctx context.Context, log *logrus.Entry) error {
 
 	go g.Run()
 
-	if err := env.ValidateVars(envDatabaseAccountName); err != nil {
-		return err
-	}
-
-	msiToken, err := _env.NewMSITokenCredential()
-	if err != nil {
-		return err
-	}
-	logrusEntry := log.WithField("component", "database")
-
-	dbAccountName := os.Getenv(envDatabaseAccountName)
-	scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
-	dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
-	if err != nil {
-		return err
-	}
-	dbc, err := database.NewDatabaseClient(logrusEntry, _env, dbAuthorizer, m, nil, dbAccountName)
+	dbc, err := database.NewDatabaseClientFromEnv(ctx, _env, log, m, nil)
 	if err != nil {
 		return err
 	}
 
-	dbName, err := DBName(_env.IsLocalDevelopmentMode())
+	dbName, err := env.DBName(_env)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/aro/main.go b/cmd/aro/main.go
@@ -12,7 +12,6 @@ import (
 	"os"
 	"strings"
 
-	"github.com/Azure/ARO-RP/pkg/env"
 	utillog "github.com/Azure/ARO-RP/pkg/util/log"
 	_ "github.com/Azure/ARO-RP/pkg/util/scheme"
 	"github.com/Azure/ARO-RP/pkg/util/version"
@@ -98,15 +97,3 @@ func checkMinArgs(required int) {
 		os.Exit(2)
 	}
 }
-
-func DBName(isLocalDevelopmentMode bool) (string, error) {
-	if !isLocalDevelopmentMode {
-		return "ARO", nil
-	}
-
-	if err := env.ValidateVars(envDatabaseName); err != nil {
-		return "", fmt.Errorf("%v (development mode)", err.Error())
-	}
-
-	return os.Getenv(envDatabaseName), nil
-}
diff --git a/cmd/aro/monitor.go b/cmd/aro/monitor.go
@@ -5,7 +5,6 @@ package main
 
 import (
 	"context"
-	"fmt"
 	"os"
 
 	"github.com/Azure/go-autorest/tracing"
@@ -22,7 +21,6 @@ import (
 	pkgmonitor "github.com/Azure/ARO-RP/pkg/monitor"
 	"github.com/Azure/ARO-RP/pkg/proxy"
 	"github.com/Azure/ARO-RP/pkg/util/encryption"
-	"github.com/Azure/ARO-RP/pkg/util/keyvault"
 )
 
 func monitor(ctx context.Context, log *logrus.Entry) error {
@@ -60,48 +58,17 @@ func monitor(ctx context.Context, log *logrus.Entry) error {
 
 	clusterm := statsd.New(ctx, log.WithField("component", "metrics"), _env, os.Getenv("CLUSTER_MDM_ACCOUNT"), os.Getenv("CLUSTER_MDM_NAMESPACE"), os.Getenv("MDM_STATSD_SOCKET"))
 
-	msiToken, err := _env.NewMSITokenCredential()
+	aead, err := encryption.NewAEADWithCore(ctx, _env, env.EncryptionSecretV2Name, env.EncryptionSecretName)
 	if err != nil {
 		return err
 	}
 
-	msiKVAuthorizer, err := _env.NewMSIAuthorizer(_env.Environment().KeyVaultScope)
+	dbc, err := database.NewDatabaseClientFromEnv(ctx, _env, log, &noop.Noop{}, aead)
 	if err != nil {
 		return err
 	}
 
-	if err := env.ValidateVars(envKeyVaultPrefix); err != nil {
-		return err
-	}
-	keyVaultPrefix := os.Getenv(envKeyVaultPrefix)
-	// TODO: should not be using the service keyvault here
-	serviceKeyvaultURI := keyvault.URI(_env, env.ServiceKeyvaultSuffix, keyVaultPrefix)
-	serviceKeyvault := keyvault.NewManager(msiKVAuthorizer, serviceKeyvaultURI)
-
-	aead, err := encryption.NewMulti(ctx, serviceKeyvault, env.EncryptionSecretV2Name, env.EncryptionSecretName)
-	if err != nil {
-		return err
-	}
-
-	if err := env.ValidateVars(envDatabaseAccountName); err != nil {
-		return err
-	}
-
-	dbAccountName := os.Getenv(envDatabaseAccountName)
-
-	logrusEntry := log.WithField("component", "database")
-	scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
-	dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
-	if err != nil {
-		return err
-	}
-
-	dbc, err := database.NewDatabaseClient(log.WithField("component", "database"), _env, dbAuthorizer, &noop.Noop{}, aead, dbAccountName)
-	if err != nil {
-		return err
-	}
-
-	dbName, err := DBName(_env.IsLocalDevelopmentMode())
+	dbName, err := env.DBName(_env)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/aro/portal.go b/cmd/aro/portal.go
@@ -6,7 +6,6 @@ package main
 import (
 	"context"
 	"crypto/x509"
-	"fmt"
 	"net"
 	"os"
 	"strings"
@@ -61,16 +60,6 @@ func portal(ctx context.Context, log *logrus.Entry, audit *logrus.Entry) error {
 		return err
 	}
 
-	msiToken, err := _env.NewMSITokenCredential()
-	if err != nil {
-		return err
-	}
-
-	msiKVAuthorizer, err := _env.NewMSIAuthorizer(_env.Environment().KeyVaultScope)
-	if err != nil {
-		return err
-	}
-
 	m := statsd.New(ctx, log.WithField("component", "portal"), _env, os.Getenv("MDM_ACCOUNT"), os.Getenv("MDM_NAMESPACE"), os.Getenv("MDM_STATSD_SOCKET"))
 
 	g, err := golang.NewMetrics(log.WithField("component", "portal"), m)
@@ -80,52 +69,37 @@ func portal(ctx context.Context, log *logrus.Entry, audit *logrus.Entry) error {
 
 	go g.Run()
 
-	if err := env.ValidateVars(envKeyVaultPrefix); err != nil {
-		return err
-	}
-	keyVaultPrefix := os.Getenv(envKeyVaultPrefix)
-	// TODO: should not be using the service keyvault here
-	serviceKeyvaultURI := keyvault.URI(_env, env.ServiceKeyvaultSuffix, keyVaultPrefix)
-	serviceKeyvault := keyvault.NewManager(msiKVAuthorizer, serviceKeyvaultURI)
-
-	aead, err := encryption.NewMulti(ctx, serviceKeyvault, env.EncryptionSecretV2Name, env.EncryptionSecretName)
+	aead, err := encryption.NewAEADWithCore(ctx, _env, env.EncryptionSecretV2Name, env.EncryptionSecretName)
 	if err != nil {
 		return err
 	}
 
-	if err := env.ValidateVars(envDatabaseAccountName); err != nil {
-		return err
-	}
-
-	dbAccountName := os.Getenv(envDatabaseAccountName)
-
-	logrusEntry := log.WithField("component", "database")
-	scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
-	dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
+	dbc, err := database.NewDatabaseClientFromEnv(ctx, _env, log, m, aead)
 	if err != nil {
 		return err
 	}
 
-	dbc, err := database.NewDatabaseClient(log.WithField("component", "database"), _env, dbAuthorizer, m, aead, dbAccountName)
+	dbName, err := env.DBName(_env)
 	if err != nil {
 		return err
 	}
 
-	dbName, err := DBName(_env.IsLocalDevelopmentMode())
+	dbOpenShiftClusters, err := database.NewOpenShiftClusters(ctx, dbc, dbName)
 	if err != nil {
 		return err
 	}
 
-	dbOpenShiftClusters, err := database.NewOpenShiftClusters(ctx, dbc, dbName)
+	dbPortal, err := database.NewPortal(ctx, dbc, dbName)
 	if err != nil {
 		return err
 	}
 
-	dbPortal, err := database.NewPortal(ctx, dbc, dbName)
+	msiKVAuthorizer, err := _env.NewMSIAuthorizer(_env.Environment().KeyVaultScope)
 	if err != nil {
 		return err
 	}
 
+	keyVaultPrefix := os.Getenv(encryption.KeyVaultPrefix)
 	portalKeyvaultURI := keyvault.URI(_env, env.PortalKeyvaultSuffix, keyVaultPrefix)
 	portalKeyvault := keyvault.NewManager(msiKVAuthorizer, portalKeyvaultURI)
 

diff --git a/cmd/aro/rp.go b/cmd/aro/rp.go
@@ -100,34 +100,17 @@ func rp(ctx context.Context, log, audit *logrus.Entry) error {
 
 	clusterm := statsd.New(ctx, log.WithField("component", "metrics"), _env, os.Getenv("CLUSTER_MDM_ACCOUNT"), os.Getenv("CLUSTER_MDM_NAMESPACE"), os.Getenv("MDM_STATSD_SOCKET"))
 
-	msiToken, err := _env.NewMSITokenCredential()
+	aead, err := encryption.NewAEADWithCore(ctx, _env, env.EncryptionSecretV2Name, env.EncryptionSecretName)
 	if err != nil {
 		return err
 	}
 
-	aead, err := encryption.NewMulti(ctx, _env.ServiceKeyvault(), env.EncryptionSecretV2Name, env.EncryptionSecretName)
+	dbc, err := database.NewDatabaseClientFromEnv(ctx, _env, log, metrics, aead)
 	if err != nil {
 		return err
 	}
 
-	if err := env.ValidateVars(envDatabaseAccountName); err != nil {
-		return err
-	}
-	dbAccountName := os.Getenv(envDatabaseAccountName)
-
-	logrusEntry := log.WithField("component", "database")
-	scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
-	dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
-	if err != nil {
-		return err
-	}
-
-	dbc, err := database.NewDatabaseClient(log.WithField("component", "database"), _env, dbAuthorizer, metrics, aead, dbAccountName)
-	if err != nil {
-		return err
-	}
-
-	dbName, err := DBName(env.IsLocalDevelopmentMode())
+	dbName, err := env.DBName(_env)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/aro/update_ocp_versions.go b/cmd/aro/update_ocp_versions.go
@@ -10,15 +10,12 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy"
 	"github.com/sirupsen/logrus"
 
 	"github.com/Azure/ARO-RP/pkg/api"
 	"github.com/Azure/ARO-RP/pkg/database"
 	"github.com/Azure/ARO-RP/pkg/env"
 	"github.com/Azure/ARO-RP/pkg/metrics/statsd"
-	"github.com/Azure/ARO-RP/pkg/util/encryption"
-	"github.com/Azure/ARO-RP/pkg/util/keyvault"
 	"github.com/Azure/ARO-RP/pkg/util/version"
 )
 
@@ -160,53 +157,18 @@ func getVersionsDatabase(ctx context.Context, log *logrus.Entry) (database.OpenS
 		}
 	}
 
-	msiToken, err := _env.NewMSITokenCredential()
-	if err != nil {
-		return nil, fmt.Errorf("MSI Authorizer failed with: %s", err.Error())
-	}
-
-	msiKVAuthorizer, err := _env.NewMSIAuthorizer(_env.Environment().KeyVaultScope)
-	if err != nil {
-		return nil, fmt.Errorf("MSI KeyVault Authorizer failed with: %s", err.Error())
-	}
-
 	m := statsd.New(ctx, log.WithField("component", "update-ocp-versions"), _env, os.Getenv("MDM_ACCOUNT"), os.Getenv("MDM_NAMESPACE"), os.Getenv("MDM_STATSD_SOCKET"))
 
-	if err := env.ValidateVars(envKeyVaultPrefix); err != nil {
-		return nil, err
-	}
-	keyVaultPrefix := os.Getenv(envKeyVaultPrefix)
-	serviceKeyvaultURI := keyvault.URI(_env, env.ServiceKeyvaultSuffix, keyVaultPrefix)
-	serviceKeyvault := keyvault.NewManager(msiKVAuthorizer, serviceKeyvaultURI)
-
-	aead, err := encryption.NewMulti(ctx, serviceKeyvault, env.EncryptionSecretV2Name, env.EncryptionSecretName)
+	dbc, err := database.NewDatabaseClientFromEnv(ctx, _env, log, m, nil)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed creating database client: %w", err)
 	}
 
-	if err := env.ValidateVars(envDatabaseAccountName); err != nil {
-		return nil, err
-	}
-
-	dbAccountName := os.Getenv(envDatabaseAccountName)
-	clientOptions := &policy.ClientOptions{
-		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
-	}
-	logrusEntry := log.WithField("component", "database")
-	dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
-	if err != nil {
-		return nil, err
-	}
-
-	dbc, err := database.NewDatabaseClient(log.WithField("component", "database"), _env, dbAuthorizer, m, aead, dbAccountName)
+	dbName, err := env.DBName(_env)
 	if err != nil {
 		return nil, err
 	}
 
-	dbName, err := DBName(_env.IsLocalDevelopmentMode())
-	if err != nil {
-		return nil, err
-	}
 	dbOpenShiftVersions, err := database.NewOpenShiftVersions(ctx, dbc, dbName)
 	if err != nil {
 		return nil, err

diff --git a/cmd/aro/update_role_sets.go b/cmd/aro/update_role_sets.go
@@ -16,7 +16,6 @@ import (
 	"github.com/Azure/ARO-RP/pkg/env"
 	"github.com/Azure/ARO-RP/pkg/metrics/statsd"
 	"github.com/Azure/ARO-RP/pkg/util/encryption"
-	"github.com/Azure/ARO-RP/pkg/util/keyvault"
 )
 
 func getRoleSetsFromEnv() ([]api.PlatformWorkloadIdentityRoleSetProperties, error) {
@@ -38,26 +37,23 @@ func getPlatformWorkloadIdentityRoleSetDatabase(ctx context.Context, log *logrus
 		return nil, fmt.Errorf("MSI Authorizer failed with: %s", err.Error())
 	}
 
-	msiKVAuthorizer, err := _env.NewMSIAuthorizer(_env.Environment().KeyVaultScope)
-	if err != nil {
-		return nil, fmt.Errorf("MSI KeyVault Authorizer failed with: %s", err.Error())
-	}
-
 	m := statsd.New(ctx, log.WithField("component", "update-role-sets"), _env, os.Getenv("MDM_ACCOUNT"), os.Getenv("MDM_NAMESPACE"), os.Getenv("MDM_STATSD_SOCKET"))
 
-	keyVaultPrefix := os.Getenv(envKeyVaultPrefix)
-	serviceKeyvaultURI := keyvault.URI(_env, env.ServiceKeyvaultSuffix, keyVaultPrefix)
-	serviceKeyvault := keyvault.NewManager(msiKVAuthorizer, serviceKeyvaultURI)
+	aead, err := encryption.NewAEADWithCore(ctx, _env, env.EncryptionSecretV2Name, env.EncryptionSecretName)
+	if err != nil {
+		return nil, err
+	}
 
-	aead, err := encryption.NewMulti(ctx, serviceKeyvault, env.EncryptionSecretV2Name, env.EncryptionSecretName)
+	dbName, err := env.DBName(_env)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := env.ValidateVars(envDatabaseAccountName); err != nil {
+	dbAccountName, err := env.DBAccountName()
+	if err != nil {
 		return nil, err
 	}
-	dbAccountName := os.Getenv(envDatabaseAccountName)
+
 	clientOptions := &policy.ClientOptions{
 		ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
 	}
@@ -73,11 +69,6 @@ func getPlatformWorkloadIdentityRoleSetDatabase(ctx context.Context, log *logrus
 		return nil, err
 	}
 
-	dbName, err := DBName(_env.IsLocalDevelopmentMode())
-	if err != nil {
-		return nil, err
-	}
-
 	return database.NewPlatformWorkloadIdentityRoleSets(ctx, dbc, dbName)
 }