From 33256f1096ac9fa61d9dea9b0bf3d6679d6c0d42 Mon Sep 17 00:00:00 2001 From: Matthew Christopher Date: Fri, 12 Feb 2021 17:34:40 -0800 Subject: [PATCH 1/5] Fix v1 secret naming - Fix issue where namespace was mistakenly included in v1 secret naming key generation. Some resources are not expected to have namespace prefix in certain KeyVault scenarios. --- controllers/secret_naming_version_test.go | 186 +++++++++++++++--- .../azuresqlmanageduser.go | 4 +- .../azuresqluser/azuresqluser_reconcile.go | 6 +- pkg/secrets/keyvault/client.go | 8 +- 4 files changed, 169 insertions(+), 35 deletions(-) diff --git a/controllers/secret_naming_version_test.go b/controllers/secret_naming_version_test.go index 46e14ce321d..cf830f16938 100644 --- a/controllers/secret_naming_version_test.go +++ b/controllers/secret_naming_version_test.go @@ -138,20 +138,36 @@ func TestStorageAccount_SecretNamedCorrectly(t *testing.T) { } func assertSQLServerAdminSecretCreated(ctx context.Context, t *testing.T, sqlServerInstance *v1beta1.AzureSqlServer) { - secret := &v1.Secret{} - assert.Eventually(t, func() bool { - var expectedServerSecretName string - if tc.secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 { - expectedServerSecretName = sqlServerInstance.Name - } else { - expectedServerSecretName = fmt.Sprintf("%s-%s", "azuresqlserver", sqlServerInstance.Name) - } - err := tc.k8sClient.Get(ctx, types.NamespacedName{Namespace: sqlServerInstance.Namespace, Name: expectedServerSecretName}, secret) - if err != nil { - return false - } - return secret.Name == expectedServerSecretName && secret.Namespace == sqlServerInstance.Namespace - }, tc.timeoutFast, tc.retry, "wait for server to have secret") + if len(sqlServerInstance.Spec.KeyVaultToStoreSecrets) == 0 { + secret := &v1.Secret{} + assert.Eventually(t, func() bool { + var expectedServerSecretName string + if tc.secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 { + expectedServerSecretName = sqlServerInstance.Name + } else { + expectedServerSecretName = fmt.Sprintf("%s-%s", "azuresqlserver", sqlServerInstance.Name) + } + err := tc.k8sClient.Get(ctx, types.NamespacedName{Namespace: sqlServerInstance.Namespace, Name: expectedServerSecretName}, secret) + if err != nil { + return false + } + return secret.Name == expectedServerSecretName && secret.Namespace == sqlServerInstance.Namespace + }, tc.timeoutFast, tc.retry, "wait for server to have secret") + } else { + // Check that the user's secret is in the keyvault + keyVaultSecretClient := kvsecrets.New(sqlServerInstance.Spec.KeyVaultToStoreSecrets, config.GlobalCredentials(), config.SecretNamingVersion()) + + assert.Eventually(t, func() bool { + expectedSecretName := makeSQLServerKeyVaultSecretName(sqlServerInstance) + result, err := keyVaultSecretClient.KeyVaultClient.GetSecret(ctx, kvsecrets.GetVaultsURL(sqlServerInstance.Spec.KeyVaultToStoreSecrets), expectedSecretName, "") + if err != nil { + return false + } + secrets := *result.Value + + return len(string(secrets)) > 0 + }, tc.timeoutFast, tc.retry, "wait for keyvault to have secret for server") + } } func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { @@ -162,7 +178,7 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { // Add any setup steps that needs to be executed before each test rgName := tc.resourceGroupName - sqlServerName := GenerateTestResourceNameWithRandom("sqlserver", 10) + sqlServerName := GenerateTestResourceNameWithRandom("sqlserver1", 10) rgLocation := "westus2" sqlServerNamespacedName := types.NamespacedName{Name: sqlServerName, Namespace: "default"} @@ -193,7 +209,7 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { // Create firewall rule sqlFirewallRuleNamespacedName := types.NamespacedName{ - Name: GenerateTestResourceNameWithRandom("sqlfwr-local", 10), + Name: GenerateTestResourceNameWithRandom("sqlfwr-local1", 10), Namespace: "default", } sqlFirewallRule := v1beta1.NewAzureSQLFirewallRule( @@ -216,11 +232,11 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { // Run user subtests t.Run("group2", func(t *testing.T) { - t.Run("set up user in first db", func(t *testing.T) { + t.Run("set up user in db", func(t *testing.T) { t.Parallel() // create a sql user and verify it provisions - username := "sql-test-user" + helpers.RandomString(10) + username := "user1" + helpers.RandomString(10) roles := []string{"db_owner"} keyVaultSecretFormats := []string{"adonet"} @@ -257,12 +273,10 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { t.Parallel() // create a sql user and verify it provisions - username := "sql-test-user" + helpers.RandomString(10) + username := "user2" + helpers.RandomString(10) roles := []string{"db_owner"} // This test will attempt to persist secrets to the KV that was instantiated as part of the test suite - keyVaultName := tc.keyvaultName - kvSqlUser1 = &v1alpha1.AzureSQLUser{ ObjectMeta: metav1.ObjectMeta{ Name: username, @@ -273,14 +287,14 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { DbName: sqlDatabaseName, ResourceGroup: rgName, Roles: roles, - KeyVaultToStoreSecrets: keyVaultName, + KeyVaultToStoreSecrets: tc.keyvaultName, }, } EnsureInstance(ctx, t, tc, kvSqlUser1) // Check that the user's secret is in the keyvault - keyVaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials(), config.SecretNamingVersion()) + keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials(), config.SecretNamingVersion()) assert.Eventually(func() bool { key := makeSQLUserSecretKey(keyVaultSecretClient, kvSqlUser1) @@ -296,13 +310,11 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { t.Parallel() // create a sql user and verify it provisions - username := "sql-test-user" + helpers.RandomString(10) + username := "user3" + helpers.RandomString(10) roles := []string{"db_owner"} formats := []string{"adonet"} // This test will attempt to persist secrets to the KV that was instantiated as part of the test suite - keyVaultName := tc.keyvaultName - kvSqlUser2 = &v1alpha1.AzureSQLUser{ ObjectMeta: metav1.ObjectMeta{ Name: username, @@ -313,7 +325,7 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { DbName: sqlDatabaseName, ResourceGroup: rgName, Roles: roles, - KeyVaultToStoreSecrets: keyVaultName, + KeyVaultToStoreSecrets: tc.keyvaultName, KeyVaultSecretFormats: formats, }, } @@ -321,7 +333,7 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { EnsureInstance(ctx, t, tc, kvSqlUser2) // Check that the user's secret is in the keyvault - keyVaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials(), config.SecretNamingVersion()) + keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials(), config.SecretNamingVersion()) assert.Eventually(func() bool { key := makeSQLUserSecretKey(keyVaultSecretClient, kvSqlUser2) @@ -416,6 +428,116 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) { EnsureDelete(ctx, t, tc, sqlServerInstance) } +func TestAzureSqlServerKVSecretAndUser_SecretNamedCorrectly(t *testing.T) { + t.Parallel() + defer PanicRecover(t) + ctx := context.Background() + assert := assert.New(t) + + // Add any setup steps that needs to be executed before each test + rgName := tc.resourceGroupName + sqlServerName := GenerateTestResourceNameWithRandom("kvsqlserv", 10) + rgLocation := "westus2" + + sqlServerNamespacedName := types.NamespacedName{Name: sqlServerName, Namespace: "default"} + sqlServerInstance := v1beta1.NewAzureSQLServer(sqlServerNamespacedName, rgName, rgLocation) + sqlServerInstance.Spec.KeyVaultToStoreSecrets = tc.keyvaultName + + // create and wait + RequireInstance(ctx, t, tc, sqlServerInstance) + + //verify secret exists in k8s for server 1 + assertSQLServerAdminSecretCreated(ctx, t, sqlServerInstance) + + sqlDatabaseName := GenerateTestResourceNameWithRandom("sqldatabase", 10) + // Create the SqlDatabase object and expect the Reconcile to be created + sqlDatabaseInstance := &v1beta1.AzureSqlDatabase{ + ObjectMeta: metav1.ObjectMeta{ + Name: sqlDatabaseName, + Namespace: "default", + }, + Spec: v1beta1.AzureSqlDatabaseSpec{ + Location: rgLocation, + ResourceGroup: rgName, + Server: sqlServerName, + Edition: 0, + }, + } + + EnsureInstance(ctx, t, tc, sqlDatabaseInstance) + + // Create firewall rule + sqlFirewallRuleNamespacedName := types.NamespacedName{ + Name: GenerateTestResourceNameWithRandom("sqlfwr-local2", 10), + Namespace: "default", + } + sqlFirewallRule := v1beta1.NewAzureSQLFirewallRule( + sqlFirewallRuleNamespacedName, + rgName, + sqlServerName, + "1.1.1.1", + "255.255.255.255", + ) + EnsureInstance(ctx, t, tc, sqlFirewallRule) + + var kvSqlUser1 *v1alpha1.AzureSQLUser + + // Run user subtests + t.Run("group2", func(t *testing.T) { + + t.Run("set up user in db, specifying adminSecretName", func(t *testing.T) { + t.Parallel() + + // create a sql user and verify it provisions + username := "user4" + helpers.RandomString(10) + roles := []string{"db_owner"} + + var adminSecret string + if tc.secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 { + adminSecret = fmt.Sprintf("%s-%s", sqlServerInstance.Namespace, sqlServerInstance.Name) + } else { + adminSecret = sqlServerInstance.Name + } + + // This test will attempt to persist secrets to the KV that was instantiated as part of the test suite + kvSqlUser1 = &v1alpha1.AzureSQLUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: username, + Namespace: "default", + }, + Spec: v1alpha1.AzureSQLUserSpec{ + Server: sqlServerName, + DbName: sqlDatabaseName, + ResourceGroup: rgName, + Roles: roles, + KeyVaultToStoreSecrets: tc.keyvaultName, + AdminSecretKeyVault: tc.keyvaultName, + AdminSecret: adminSecret, + }, + } + + EnsureInstance(ctx, t, tc, kvSqlUser1) + + // Check that the user's secret is in the keyvault + keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials(), config.SecretNamingVersion()) + + assert.Eventually(func() bool { + key := makeSQLUserSecretKey(keyVaultSecretClient, kvSqlUser1) + key.Name = key.Name + var secrets, err = keyVaultSecretClient.Get(ctx, key, secrets.Flatten(true)) + assert.NoError(err) + + return len(string(secrets["secret"])) > 0 + }, tc.timeoutFast, tc.retry, "wait for keyvault to show azure sql user credentials with custom formats") + + t.Log(kvSqlUser1.Status) + }) + }) + + // Delete the SQL server + EnsureDelete(ctx, t, tc, sqlServerInstance) +} + func makeSQLUserSecretKey(secretClient *kvsecrets.SecretClient, instance *v1alpha1.AzureSQLUser) secrets.SecretKey { var keyNamespace string if secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 { @@ -425,3 +547,11 @@ func makeSQLUserSecretKey(secretClient *kvsecrets.SecretClient, instance *v1alph } return secrets.SecretKey{Name: instance.Name, Namespace: keyNamespace, Kind: "azuresqluser"} } + +func makeSQLServerKeyVaultSecretName(instance *v1beta1.AzureSqlServer) string { + if tc.secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 { + return fmt.Sprintf("%s-%s", instance.Namespace, instance.Name) + } else { + return fmt.Sprintf("%s-%s-%s", "azuresqlserver", instance.Namespace, instance.Name) + } +} diff --git a/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser.go b/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser.go index 7b79e5220f2..32266beadf4 100644 --- a/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser.go +++ b/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser.go @@ -216,8 +216,8 @@ func findBadChars(stack string) error { func makeSecretKey(secretClient secrets.SecretClient, instance *v1alpha1.AzureSQLManagedUser) secrets.SecretKey { if secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 { - if len(instance.Spec.KeyVaultSecretPrefix) != 0 { // If KeyVaultSecretPrefix is specified, use that for secrets - return secrets.SecretKey{Name: instance.Spec.KeyVaultSecretPrefix, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} + if len(instance.Spec.KeyVaultSecretPrefix) != 0 { // If KeyVaultSecretPrefix is specified, use that for secrets. Namespace is ignored in this case + return secrets.SecretKey{Name: instance.Spec.KeyVaultSecretPrefix, Namespace: "", Kind: instance.TypeMeta.Kind} } return secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} } diff --git a/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go b/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go index b0f2abc94da..435e6dc5411 100644 --- a/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqluser/azuresqluser_reconcile.go @@ -26,7 +26,6 @@ import ( ) func GetAdminSecretKey(adminSecretName string, namespace string) secrets.SecretKey { - // TODO: Is there a better way to get TypeMeta here? return secrets.SecretKey{Name: adminSecretName, Namespace: namespace, Kind: reflect.TypeOf(v1beta1.AzureSqlServer{}).Name()} } @@ -41,6 +40,11 @@ func (s *AzureSqlUserManager) getAdminSecret(ctx context.Context, instance *v1al // if the admin secret keyvault is not specified, fall back to global secretclient if len(instance.Spec.AdminSecretKeyVault) != 0 { adminSecretClient = keyvaultSecrets.New(instance.Spec.AdminSecretKeyVault, s.Creds, s.SecretClient.GetSecretNamingVersion()) + + // This is here for legacy reasons + if len(instance.Spec.AdminSecret) != 0 && s.SecretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 { + adminSecretKey.Namespace = "" + } } // get admin creds for server diff --git a/pkg/secrets/keyvault/client.go b/pkg/secrets/keyvault/client.go index 4897e1502be..681f13491f7 100644 --- a/pkg/secrets/keyvault/client.go +++ b/pkg/secrets/keyvault/client.go @@ -289,16 +289,16 @@ func (k *SecretClient) Get(ctx context.Context, key secrets.SecretKey, opts ...s } func (k *SecretClient) makeLegacySecretName(key secrets.SecretKey) (string, error) { - if len(key.Namespace) == 0 { - return "", errors.Errorf("secret key missing required namespace field, %s", key) - } if len(key.Name) == 0 { return "", errors.Errorf("secret key missing required name field, %s", key) } var parts []string - parts = append(parts, key.Namespace) + // A few secrets allow empty namespace + if len(key.Namespace) != 0 { + parts = append(parts, key.Namespace) + } parts = append(parts, key.Name) return strings.Join(parts, "-"), nil From a15b8f07a01d1bda8ce7c970e23b53db547dc7c3 Mon Sep 17 00:00:00 2001 From: Matthew Christopher Date: Tue, 16 Feb 2021 11:26:28 -0800 Subject: [PATCH 2/5] Increase build timeout a bit --- Makefile | 2 +- azure-pipelines.yml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 889450e47a8..8d107a3c4bf 100644 --- a/Makefile +++ b/Makefile @@ -64,7 +64,7 @@ test-integration-controllers: generate fmt vet manifests .PHONY: test-v1-secret-naming test-v1-secret-naming: generate fmt vet manifests TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=false REQUEUE_AFTER=20 AZURE_SECRET_NAMING_VERSION=1 \ - go test -v -run "^.*_SecretNamedCorrectly$$" -tags "$(BUILD_TAGS)" -coverprofile=reports/v1-secret-naming-coverage-output.txt -coverpkg=./... -covermode count -parallel 4 -timeout 10m \ + go test -v -run "^.*_SecretNamedCorrectly$$" -tags "$(BUILD_TAGS)" -coverprofile=reports/v1-secret-naming-coverage-output.txt -coverpkg=./... -covermode count -parallel 4 -timeout 15m \ ./controllers/... # Run Resource Manager tests against the configured cluster diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 5afd1290ded..07d245bc355 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -15,7 +15,8 @@ resources: - repo: self pool: - vmImage: 'ubuntu-latest' + vmImage: 'ubuntu-latest' + timeoutInMinutes: 80 variables: tag: '$(Build.BuildId)' From 169b7ac9e3be3d72c4b4841f24463c325c0fa8c9 Mon Sep 17 00:00:00 2001 From: Matthew Christopher Date: Tue, 16 Feb 2021 16:09:44 -0800 Subject: [PATCH 3/5] Don't create many different randoms in test --- pkg/helpers/stringhelper.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/helpers/stringhelper.go b/pkg/helpers/stringhelper.go index ebf16733034..47c64ece59b 100644 --- a/pkg/helpers/stringhelper.go +++ b/pkg/helpers/stringhelper.go @@ -8,10 +8,8 @@ import ( "encoding/base64" "encoding/json" "fmt" - "math/rand" "regexp" "strings" - "time" "unicode" "github.com/sethvargo/go-password/password" @@ -75,7 +73,6 @@ func RemoveString(slice []string, s string) (result []string) { } func randomStringWithCharset(length int, charset string) string { - var seededRand = rand.New(rand.NewSource(time.Now().UnixNano())) b := make([]byte, length) for i := range b { b[i] = charset[seededRand.Intn(len(charset))] From 0fbbe450d8c9e9d71d1d0c051b0224656ba648c0 Mon Sep 17 00:00:00 2001 From: Matthew Christopher Date: Tue, 16 Feb 2021 16:50:39 -0800 Subject: [PATCH 4/5] SecretClient should not be modified --- .../appinsights/api_keys_reconcile.go | 19 ++++++++--- .../appinsights/appinsights.go | 19 ++++++----- pkg/resourcemanager/appinsights/manager.go | 6 ++-- .../azuresqlfailovergroup_reconcile.go | 14 ++++---- .../azuresqlmanageduser_reconcile.go | 14 +++++--- .../azuresqlserver_reconcile.go | 14 ++++---- .../cosmosdbs/cosmosdb_reconcile.go | 23 +++++++------ pkg/resourcemanager/eventhubs/hub.go | 32 ++++++++++++------- pkg/resourcemanager/mysql/server/reconcile.go | 28 ++++++++-------- pkg/resourcemanager/psql/server/server.go | 12 +++---- .../psql/server/server_reconcile.go | 14 ++++---- .../actions/rediscacheaction_reconcile.go | 5 +-- .../rediscaches/redis/rediscache_reconcile.go | 12 ++++--- pkg/resourcemanager/rediscaches/shared.go | 7 ++-- 14 files changed, 133 insertions(+), 86 deletions(-) diff --git a/pkg/resourcemanager/appinsights/api_keys_reconcile.go b/pkg/resourcemanager/appinsights/api_keys_reconcile.go index 732c8a810c7..e35d1963a38 100644 --- a/pkg/resourcemanager/appinsights/api_keys_reconcile.go +++ b/pkg/resourcemanager/appinsights/api_keys_reconcile.go @@ -31,8 +31,9 @@ func (c *InsightsAPIKeysClient) Ensure(ctx context.Context, obj runtime.Object, opt(options) } + secretClient := c.SecretClient if options.SecretClient != nil { - c.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance.Status.Provisioning = true @@ -74,7 +75,7 @@ func (c *InsightsAPIKeysClient) Ensure(ctx context.Context, obj runtime.Object, case http.StatusBadRequest: // if the key already exists it is fine only if the secret exists if strings.Contains(azerr.Type, "already exists") { - if _, err := c.SecretClient.Get(ctx, secretKey); err != nil { + if _, err := secretClient.Get(ctx, secretKey); err != nil { instance.Status.Message = "api key exists but no key could be recovered" instance.Status.FailedProvisioning = true } @@ -90,7 +91,7 @@ func (c *InsightsAPIKeysClient) Ensure(ctx context.Context, obj runtime.Object, } // when create is successful we have to store the apikey somewhere - err = c.SecretClient.Upsert( + err = secretClient.Upsert( ctx, secretKey, map[string][]byte{"apiKey": []byte(*apiKey.APIKey)}, @@ -114,11 +115,21 @@ func (c *InsightsAPIKeysClient) Ensure(ctx context.Context, obj runtime.Object, } func (c *InsightsAPIKeysClient) Delete(ctx context.Context, obj runtime.Object, opts ...resourcemanager.ConfigOption) (bool, error) { + options := &resourcemanager.Options{} + for _, opt := range opts { + opt(options) + } + instance, err := c.convert(obj) if err != nil { return false, err } + secretClient := c.SecretClient + if options.SecretClient != nil { + secretClient = options.SecretClient + } + // can't delete without an id and it probably wasn't provisioned by us if it's missing if instance.Status.ResourceId == "" { return false, nil @@ -151,7 +162,7 @@ func (c *InsightsAPIKeysClient) Delete(ctx context.Context, obj runtime.Object, } secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - err = c.SecretClient.Delete(ctx, secretKey) + err = secretClient.Delete(ctx, secretKey) if err != nil { return true, err } diff --git a/pkg/resourcemanager/appinsights/appinsights.go b/pkg/resourcemanager/appinsights/appinsights.go index d59f337aef8..872b09dd744 100644 --- a/pkg/resourcemanager/appinsights/appinsights.go +++ b/pkg/resourcemanager/appinsights/appinsights.go @@ -110,7 +110,7 @@ func (m *Manager) CreateAppInsights( } // StoreSecrets upserts the secret information for this app insight -func (m *Manager) StoreSecrets(ctx context.Context, instrumentationKey string, instance *v1alpha1.AppInsights) error { +func (m *Manager) StoreSecrets(ctx context.Context, secretClient secrets.SecretClient, instrumentationKey string, instance *v1alpha1.AppInsights) error { // build the connection string data := map[string][]byte{ @@ -120,7 +120,7 @@ func (m *Manager) StoreSecrets(ctx context.Context, instrumentationKey string, i // upsert secretKey := m.makeSecretKey(instance) - return m.SecretClient.Upsert(ctx, + return secretClient.Upsert(ctx, secretKey, data, secrets.WithOwner(instance), @@ -129,9 +129,9 @@ func (m *Manager) StoreSecrets(ctx context.Context, instrumentationKey string, i } // DeleteSecret deletes the secret information for this app insight -func (m *Manager) DeleteSecret(ctx context.Context, instance *v1alpha1.AppInsights) error { +func (m *Manager) DeleteSecret(ctx context.Context, secretClient secrets.SecretClient, instance *v1alpha1.AppInsights) error { secretKey := m.makeSecretKey(instance) - return m.SecretClient.Delete(ctx, secretKey) + return secretClient.Delete(ctx, secretKey) } // Ensure checks the desired state of the operator @@ -141,8 +141,9 @@ func (m *Manager) Ensure(ctx context.Context, obj runtime.Object, opts ...resour opt(options) } + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := m.convert(obj) @@ -163,6 +164,7 @@ func (m *Manager) Ensure(ctx context.Context, obj runtime.Object, opts ...resour if comp.ApplicationInsightsComponentProperties != nil { properties := *comp.ApplicationInsightsComponentProperties err = m.StoreSecrets(ctx, + secretClient, *properties.InstrumentationKey, instance, ) @@ -225,8 +227,9 @@ func (m *Manager) Delete(ctx context.Context, obj runtime.Object, opts ...resour opt(options) } + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := m.convert(obj) @@ -249,7 +252,7 @@ func (m *Manager) Delete(ctx context.Context, obj runtime.Object, opts ...resour if helpers.ContainsString(catch, azerr.Type) { return true, nil } else if helpers.ContainsString(gone, azerr.Type) { - m.DeleteSecret(ctx, instance) + m.DeleteSecret(ctx, secretClient, instance) return false, nil } return true, err @@ -258,7 +261,7 @@ func (m *Manager) Delete(ctx context.Context, obj runtime.Object, opts ...resour if err == nil { if response.Status != "InProgress" { - m.DeleteSecret(ctx, instance) + m.DeleteSecret(ctx, secretClient, instance) return false, nil } } diff --git a/pkg/resourcemanager/appinsights/manager.go b/pkg/resourcemanager/appinsights/manager.go index 1f1ffe121da..4f4b01d8e9d 100644 --- a/pkg/resourcemanager/appinsights/manager.go +++ b/pkg/resourcemanager/appinsights/manager.go @@ -9,6 +9,8 @@ import ( "github.com/Azure/azure-sdk-for-go/services/appinsights/mgmt/2015-05-01/insights" "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/resourcemanager" + "github.com/Azure/azure-service-operator/pkg/secrets" + "github.com/Azure/go-autorest/autorest" ) @@ -24,9 +26,9 @@ type ApplicationInsightsManager interface { DeleteAppInsights(ctx context.Context, resourceGroupName string, resourceName string) (autorest.Response, error) GetAppInsights(ctx context.Context, resourceGroupName string, resourceName string) (insights.ApplicationInsightsComponent, error) - StoreSecrets(ctx context.Context, instrumentationKey string, instance *v1alpha1.AppInsights) error + StoreSecrets(ctx context.Context, secretClient secrets.SecretClient, instrumentationKey string, instance *v1alpha1.AppInsights) error - DeleteSecret(ctx context.Context, instance *v1alpha1.AppInsights) error + DeleteSecret(ctx context.Context, secretClient secrets.SecretClient, instance *v1alpha1.AppInsights) error // ARM Client resourcemanager.ARMClient diff --git a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go index 3e1539dedb5..f0dee68c293 100644 --- a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go @@ -28,8 +28,9 @@ func (fg *AzureSqlFailoverGroupManager) Ensure(ctx context.Context, obj runtime. opt(options) } + secretClient := fg.SecretClient if options.SecretClient != nil { - fg.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := fg.convert(obj) @@ -86,7 +87,7 @@ func (fg *AzureSqlFailoverGroupManager) Ensure(ctx context.Context, obj runtime. } secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - _, err := fg.SecretClient.Get(ctx, secretKey) + _, err := secretClient.Get(ctx, secretKey) // We make the same assumption many other places in the code make which is that if we cannot // get the secret it must not exist. if err != nil { @@ -94,7 +95,7 @@ func (fg *AzureSqlFailoverGroupManager) Ensure(ctx context.Context, obj runtime. secret := fg.NewSecret(instance) // create or update the secret - err = fg.SecretClient.Upsert( + err = secretClient.Upsert( ctx, secretKey, secret, @@ -142,8 +143,9 @@ func (fg *AzureSqlFailoverGroupManager) Delete(ctx context.Context, obj runtime. opt(options) } + secretClient := fg.SecretClient if options.SecretClient != nil { - fg.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := fg.convert(obj) @@ -180,7 +182,7 @@ func (fg *AzureSqlFailoverGroupManager) Delete(ctx context.Context, obj runtime. if helpers.ContainsString(finished, azerr.Type) { // Best case deletion of secret - fg.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } instance.Status.Message = fmt.Sprintf("AzureSqlFailoverGroup Delete failed with: %s", err.Error()) @@ -189,7 +191,7 @@ func (fg *AzureSqlFailoverGroupManager) Delete(ctx context.Context, obj runtime. instance.Status.Message = fmt.Sprintf("Delete AzureSqlFailoverGroup succeeded") // Best case deletion of secret - fg.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } diff --git a/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go index 318b73c6a75..34a473b1802 100644 --- a/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlmanageduser/azuresqlmanageduser_reconcile.go @@ -36,8 +36,9 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj opt(options) } + secretClient := s.SecretClient if options.SecretClient != nil { - s.SecretClient = options.SecretClient + secretClient = options.SecretClient } _, err = s.GetDB(ctx, instance.Spec.ResourceGroup, instance.Spec.Server, instance.Spec.DbName) @@ -117,7 +118,7 @@ func (s *AzureSqlManagedUserManager) Ensure(ctx context.Context, obj runtime.Obj return false, fmt.Errorf("GrantUserRoles failed: %v", err) } - err = s.UpdateSecret(ctx, instance, s.SecretClient) + err = s.UpdateSecret(ctx, instance, secretClient) if err != nil { instance.Status.Message = fmt.Sprintf("Updating secret failed: %v", err) return false, fmt.Errorf("updating secret failed") @@ -141,6 +142,11 @@ func (s *AzureSqlManagedUserManager) Delete(ctx context.Context, obj runtime.Obj return false, err } + secretClient := s.SecretClient + if options.SecretClient != nil { + secretClient = options.SecretClient + } + requestedUsername := instance.Spec.ManagedIdentityName if len(requestedUsername) == 0 { requestedUsername = instance.Name @@ -159,7 +165,7 @@ func (s *AzureSqlManagedUserManager) Delete(ctx context.Context, obj runtime.Obj azerr := errhelp.NewAzureError(err) if helpers.ContainsString(catch, azerr.Type) { // Best case deletion of secrets - s.DeleteSecrets(ctx, instance, s.SecretClient) + s.DeleteSecrets(ctx, instance, secretClient) return false, nil } @@ -199,7 +205,7 @@ func (s *AzureSqlManagedUserManager) Delete(ctx context.Context, obj runtime.Obj } // Best case deletion of secrets - s.DeleteSecrets(ctx, instance, s.SecretClient) + s.DeleteSecrets(ctx, instance, secretClient) instance.Status.Message = fmt.Sprintf("Delete AzureSqlManagedUser succeeded") diff --git a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go index 08f6b5df285..5ea48e97e66 100644 --- a/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver_reconcile.go @@ -34,8 +34,9 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, opt(options) } + secretClient := s.SecretClient if options.SecretClient != nil { - s.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := s.convert(obj) @@ -49,7 +50,7 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, // Check to see if secret already exists for admin username/password // create or update the secret secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - secret, err := s.SecretClient.Get(ctx, secretKey) + secret, err := secretClient.Get(ctx, secretKey) if err != nil { if instance.Status.Provisioned { instance.Status.Message = err.Error() @@ -79,7 +80,7 @@ func (s *AzureSqlServerManager) Ensure(ctx context.Context, obj runtime.Object, if err != nil { return false, err } - err = s.SecretClient.Upsert( + err = secretClient.Upsert( ctx, secretKey, secret, @@ -252,8 +253,9 @@ func (s *AzureSqlServerManager) Delete(ctx context.Context, obj runtime.Object, opt(options) } + secretClient := s.SecretClient if options.SecretClient != nil { - s.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := s.convert(obj) @@ -293,7 +295,7 @@ func (s *AzureSqlServerManager) Delete(ctx context.Context, obj runtime.Object, if helpers.ContainsString(finished, azerr.Type) { //Best effort deletion of secrets - s.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } @@ -301,7 +303,7 @@ func (s *AzureSqlServerManager) Delete(ctx context.Context, obj runtime.Object, } //Best effort deletion of secrets - s.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 500bf3ab5be..ac11d81824d 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb" + "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/helpers" @@ -27,8 +28,9 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o opt(options) } + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := m.convert(obj) @@ -92,7 +94,7 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o if instance.Status.State == "Succeeded" { // provisioning is complete, update the secrets - if err = m.createOrUpdateSecret(ctx, instance, db); err != nil { + if err = m.createOrUpdateSecret(ctx, secretClient, instance, db); err != nil { instance.Status.Message = err.Error() return false, err } @@ -149,7 +151,7 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return false, err } - if err = m.createOrUpdateSecret(ctx, instance, db); err != nil { + if err = m.createOrUpdateSecret(ctx, secretClient, instance, db); err != nil { instance.Status.Message = err.Error() return false, err } @@ -170,8 +172,9 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o opt(options) } + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := m.convert(obj) @@ -200,7 +203,7 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o errhelp.ResourceGroupNotFoundErrorCode, } if helpers.ContainsString(notFound, azerr.Type) { - _ = m.deleteSecret(ctx, instance) + _ = m.deleteSecret(ctx, secretClient, instance) return false, nil } @@ -209,7 +212,7 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o return false, err } - _ = m.deleteSecret(ctx, instance) + _ = m.deleteSecret(ctx, secretClient, instance) return false, nil } @@ -248,7 +251,7 @@ func (m *AzureCosmosDBManager) convert(obj runtime.Object) (*v1alpha1.CosmosDB, return db, nil } -func (m *AzureCosmosDBManager) createOrUpdateSecret(ctx context.Context, instance *v1alpha1.CosmosDB, db *documentdb.DatabaseAccount) error { +func (m *AzureCosmosDBManager) createOrUpdateSecret(ctx context.Context, secretClient secrets.SecretClient, instance *v1alpha1.CosmosDB, db *documentdb.DatabaseAccount) error { connStrResult, err := m.ListConnectionStrings(ctx, instance.Spec.ResourceGroup, instance.ObjectMeta.Name) if err != nil { return err @@ -283,10 +286,10 @@ func (m *AzureCosmosDBManager) createOrUpdateSecret(ctx context.Context, instanc } } - return m.SecretClient.Upsert(ctx, secretKey, secretData) + return secretClient.Upsert(ctx, secretKey, secretData) } -func (m *AzureCosmosDBManager) deleteSecret(ctx context.Context, instance *v1alpha1.CosmosDB) error { +func (m *AzureCosmosDBManager) deleteSecret(ctx context.Context, secretClient secrets.SecretClient, instance *v1alpha1.CosmosDB) error { secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - return m.SecretClient.Delete(ctx, secretKey) + return secretClient.Delete(ctx, secretKey) } diff --git a/pkg/resourcemanager/eventhubs/hub.go b/pkg/resourcemanager/eventhubs/hub.go index a5b31002489..5326de35e1a 100644 --- a/pkg/resourcemanager/eventhubs/hub.go +++ b/pkg/resourcemanager/eventhubs/hub.go @@ -151,9 +151,9 @@ func (e *azureEventHubManager) createOrUpdateAccessPolicyEventHub(resourcegroup return nil } -func (e *azureEventHubManager) createEventhubSecrets(ctx context.Context, secretName string, instance *azurev1alpha1.Eventhub, data map[string][]byte) error { +func (e *azureEventHubManager) createEventhubSecrets(ctx context.Context, secretClient secrets.SecretClient, secretName string, instance *azurev1alpha1.Eventhub, data map[string][]byte) error { secretKey := secrets.SecretKey{Name: secretName, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - return e.SecretClient.Upsert(ctx, + return secretClient.Upsert(ctx, secretKey, data, secrets.WithOwner(instance), @@ -161,10 +161,10 @@ func (e *azureEventHubManager) createEventhubSecrets(ctx context.Context, secret ) } -func (e *azureEventHubManager) deleteEventhubSecrets(ctx context.Context, secretName string, instance *azurev1alpha1.Eventhub) error { +func (e *azureEventHubManager) deleteEventhubSecrets(ctx context.Context, secretClient secrets.SecretClient, secretName string, instance *azurev1alpha1.Eventhub) error { secretKey := secrets.SecretKey{Name: secretName, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - err := e.SecretClient.Delete(ctx, secretKey) + err := secretClient.Delete(ctx, secretKey) if err != nil { return err } @@ -172,13 +172,20 @@ func (e *azureEventHubManager) deleteEventhubSecrets(ctx context.Context, secret return nil } -func (e *azureEventHubManager) listAccessKeysAndCreateSecrets(resourcegroup string, eventhubNamespace string, eventhubName string, secretName string, authorizationRuleName string, instance *azurev1alpha1.Eventhub) error { +func (e *azureEventHubManager) listAccessKeysAndCreateSecrets( + ctx context.Context, + secretClient secrets.SecretClient, + resourceGroup string, + eventhubNamespace string, + eventhubName string, + secretName string, + authorizationRuleName string, + instance *azurev1alpha1.Eventhub) error { var err error var result model.AccessKeys - ctx := context.Background() - result, err = e.ListKeys(ctx, resourcegroup, eventhubNamespace, eventhubName, authorizationRuleName) + result, err = e.ListKeys(ctx, resourceGroup, eventhubNamespace, eventhubName, authorizationRuleName) if err != nil { //log error and kill it return err @@ -195,6 +202,7 @@ func (e *azureEventHubManager) listAccessKeysAndCreateSecrets(resourcegroup stri } err = e.createEventhubSecrets( ctx, + secretClient, secretName, instance, data, @@ -226,8 +234,9 @@ func (m *azureEventHubManager) Ensure(ctx context.Context, obj runtime.Object, o captureDescription := instance.Spec.Properties.CaptureDescription secretName := instance.Spec.SecretName + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } if len(secretName) == 0 { @@ -278,7 +287,7 @@ func (m *azureEventHubManager) Ensure(ctx context.Context, obj runtime.Object, o return false, err } - err = m.listAccessKeysAndCreateSecrets(resourcegroup, eventhubNamespace, eventhubName, secretName, instance.Spec.AuthorizationRule.Name, instance) + err = m.listAccessKeysAndCreateSecrets(ctx, secretClient, resourcegroup, eventhubNamespace, eventhubName, secretName, instance.Spec.AuthorizationRule.Name, instance) if err != nil { // catch secret existing and fail reconciliation @@ -326,8 +335,9 @@ func (e *azureEventHubManager) Delete(ctx context.Context, obj runtime.Object, o resourcegroup := instance.Spec.ResourceGroup secretName := instance.Spec.SecretName + secretClient := e.SecretClient if options.SecretClient != nil { - e.SecretClient = options.SecretClient + secretClient = options.SecretClient } if len(secretName) == 0 { @@ -352,7 +362,7 @@ func (e *azureEventHubManager) Delete(ctx context.Context, obj runtime.Object, o if resp.StatusCode == http.StatusNoContent { //Delete the secrets as best effort before successful return after delete - e.deleteEventhubSecrets(ctx, secretName, instance) + e.deleteEventhubSecrets(ctx, secretClient, secretName, instance) return false, nil } diff --git a/pkg/resourcemanager/mysql/server/reconcile.go b/pkg/resourcemanager/mysql/server/reconcile.go index 06d815cd156..21243609e50 100644 --- a/pkg/resourcemanager/mysql/server/reconcile.go +++ b/pkg/resourcemanager/mysql/server/reconcile.go @@ -31,8 +31,9 @@ func (m *MySQLServerClient) Ensure(ctx context.Context, obj runtime.Object, opts opt(options) } + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := m.convert(obj) @@ -54,13 +55,13 @@ func (m *MySQLServerClient) Ensure(ctx context.Context, obj runtime.Object, opts } // Check to see if secret exists and if yes retrieve the admin login and password - secret, err := m.GetOrPrepareSecret(ctx, instance) + secret, err := m.GetOrPrepareSecret(ctx, secretClient, instance) if err != nil { instance.Status.Message = fmt.Sprintf("Failed to get or prepare secret: %s", err.Error()) return false, err } - err = m.AddServerCredsToSecrets(ctx, secret, instance) + err = m.AddServerCredsToSecrets(ctx, secretClient, secret, instance) if err != nil { return false, err } @@ -114,7 +115,7 @@ func (m *MySQLServerClient) Ensure(ctx context.Context, obj runtime.Object, opts } if server.UserVisibleState == mysql.ServerStateReady { // Update secret with FQ name of the server. We ignore the error. - m.UpdateServerNameInSecret(ctx, secret, *server.FullyQualifiedDomainName, instance) + m.UpdateServerNameInSecret(ctx, secretClient, secret, *server.FullyQualifiedDomainName, instance) instance.Status.Provisioned = true instance.Status.Provisioning = false @@ -205,8 +206,9 @@ func (m *MySQLServerClient) Delete(ctx context.Context, obj runtime.Object, opts opt(options) } + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := m.convert(obj) @@ -239,7 +241,7 @@ func (m *MySQLServerClient) Delete(ctx context.Context, obj runtime.Object, opts if status != "InProgress" { // Best case deletion of secrets secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - m.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } } @@ -286,10 +288,10 @@ func (m *MySQLServerClient) convert(obj runtime.Object) (*v1alpha2.MySQLServer, } // AddServerCredsToSecrets saves the server's admin credentials in the secret store -func (m *MySQLServerClient) AddServerCredsToSecrets(ctx context.Context, data map[string][]byte, instance *azurev1alpha2.MySQLServer) error { +func (m *MySQLServerClient) AddServerCredsToSecrets(ctx context.Context, secretClient secrets.SecretClient, data map[string][]byte, instance *azurev1alpha2.MySQLServer) error { secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - err := m.SecretClient.Upsert(ctx, + err := secretClient.Upsert(ctx, secretKey, data, secrets.WithOwner(instance), @@ -303,12 +305,12 @@ func (m *MySQLServerClient) AddServerCredsToSecrets(ctx context.Context, data ma } // UpdateSecretWithFullServerName updates the secret with the fully qualified server name -func (m *MySQLServerClient) UpdateServerNameInSecret(ctx context.Context, data map[string][]byte, fullservername string, instance *azurev1alpha2.MySQLServer) error { +func (m *MySQLServerClient) UpdateServerNameInSecret(ctx context.Context, secretClient secrets.SecretClient, data map[string][]byte, fullservername string, instance *azurev1alpha2.MySQLServer) error { secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} data["fullyQualifiedServerName"] = []byte(fullservername) - err := m.SecretClient.Upsert(ctx, + err := secretClient.Upsert(ctx, secretKey, data, secrets.WithOwner(instance), @@ -322,7 +324,7 @@ func (m *MySQLServerClient) UpdateServerNameInSecret(ctx context.Context, data m } // GetOrPrepareSecret gets tje admin credentials if they are stored or generates some if not -func (m *MySQLServerClient) GetOrPrepareSecret(ctx context.Context, instance *azurev1alpha2.MySQLServer) (map[string][]byte, error) { +func (m *MySQLServerClient) GetOrPrepareSecret(ctx context.Context, secretClient secrets.SecretClient, instance *azurev1alpha2.MySQLServer) (map[string][]byte, error) { createmode := instance.Spec.CreateMode // If createmode == default, then this is a new server creation, so generate username/password @@ -336,7 +338,7 @@ func (m *MySQLServerClient) GetOrPrepareSecret(ctx context.Context, instance *az if strings.EqualFold(createmode, "default") { // new Mysql server creation // See if secret already exists and return if it does key = secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - if stored, err := m.SecretClient.Get(ctx, key); err == nil { + if stored, err := secretClient.Get(ctx, key); err == nil { return stored, nil } // Generate random username password if secret does not exist already @@ -351,7 +353,7 @@ func (m *MySQLServerClient) GetOrPrepareSecret(ctx context.Context, instance *az // Get the username and password from the source server's secret key = secrets.SecretKey{Name: sourceServer, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - if sourceSecret, err := m.SecretClient.Get(ctx, key); err == nil { + if sourceSecret, err := secretClient.Get(ctx, key); err == nil { username = string(sourceSecret["username"]) password = string(sourceSecret["password"]) } diff --git a/pkg/resourcemanager/psql/server/server.go b/pkg/resourcemanager/psql/server/server.go index 57a74c82d6a..36a091e2d6d 100644 --- a/pkg/resourcemanager/psql/server/server.go +++ b/pkg/resourcemanager/psql/server/server.go @@ -193,10 +193,10 @@ func (c *PSQLServerClient) GetServer(ctx context.Context, resourcegroup string, return client.Get(ctx, resourcegroup, servername) } -func (c *PSQLServerClient) AddServerCredsToSecrets(ctx context.Context, data map[string][]byte, instance *v1alpha2.PostgreSQLServer) error { +func (c *PSQLServerClient) AddServerCredsToSecrets(ctx context.Context, secretClient secrets.SecretClient, data map[string][]byte, instance *v1alpha2.PostgreSQLServer) error { secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - err := c.SecretClient.Upsert(ctx, + err := secretClient.Upsert(ctx, secretKey, data, secrets.WithOwner(instance), @@ -209,12 +209,12 @@ func (c *PSQLServerClient) AddServerCredsToSecrets(ctx context.Context, data map return nil } -func (c *PSQLServerClient) UpdateSecretWithFullServerName(ctx context.Context, data map[string][]byte, instance *v1alpha2.PostgreSQLServer, fullServerName string) error { +func (c *PSQLServerClient) UpdateSecretWithFullServerName(ctx context.Context, secretClient secrets.SecretClient, data map[string][]byte, instance *v1alpha2.PostgreSQLServer, fullServerName string) error { secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} data["fullyQualifiedServerName"] = []byte(fullServerName) - err := c.SecretClient.Upsert(ctx, + err := secretClient.Upsert(ctx, secretKey, data, secrets.WithOwner(instance), @@ -227,13 +227,13 @@ func (c *PSQLServerClient) UpdateSecretWithFullServerName(ctx context.Context, d return nil } -func (c *PSQLServerClient) GetOrPrepareSecret(ctx context.Context, instance *v1alpha2.PostgreSQLServer) (map[string][]byte, error) { +func (c *PSQLServerClient) GetOrPrepareSecret(ctx context.Context, secretClient secrets.SecretClient, instance *v1alpha2.PostgreSQLServer) (map[string][]byte, error) { usernameLength := 8 secret := map[string][]byte{} secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - if stored, err := c.SecretClient.Get(ctx, secretKey); err == nil { + if stored, err := secretClient.Get(ctx, secretKey); err == nil { return stored, nil } diff --git a/pkg/resourcemanager/psql/server/server_reconcile.go b/pkg/resourcemanager/psql/server/server_reconcile.go index 3b76bf2d914..0f8e92027e3 100644 --- a/pkg/resourcemanager/psql/server/server_reconcile.go +++ b/pkg/resourcemanager/psql/server/server_reconcile.go @@ -29,8 +29,9 @@ func (c *PSQLServerClient) Ensure(ctx context.Context, obj runtime.Object, opts opt(options) } + secretClient := c.SecretClient if options.SecretClient != nil { - c.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := c.convert(obj) @@ -52,13 +53,13 @@ func (c *PSQLServerClient) Ensure(ctx context.Context, obj runtime.Object, opts } // Check to see if secret exists and if yes retrieve the admin login and password - secret, err := c.GetOrPrepareSecret(ctx, instance) + secret, err := c.GetOrPrepareSecret(ctx, secretClient, instance) if err != nil { return false, err } // Update secret with the fully qualified server name - err = c.AddServerCredsToSecrets(ctx, secret, instance) + err = c.AddServerCredsToSecrets(ctx, secretClient, secret, instance) if err != nil { instance.Status.Message = err.Error() return false, err @@ -82,7 +83,7 @@ func (c *PSQLServerClient) Ensure(ctx context.Context, obj runtime.Object, opts if getServer.UserVisibleState == psql.ServerStateReady { // Update the secret with fully qualified server name. Ignore error as we have the admin creds which is critical. - c.UpdateSecretWithFullServerName(ctx, secret, instance, *getServer.FullyQualifiedDomainName) + c.UpdateSecretWithFullServerName(ctx, secretClient, secret, instance, *getServer.FullyQualifiedDomainName) instance.Status.Message = resourcemanager.SuccessMsg instance.Status.ResourceId = *getServer.ID @@ -204,8 +205,9 @@ func (c *PSQLServerClient) Delete(ctx context.Context, obj runtime.Object, opts opt(options) } + secretClient := c.SecretClient if options.SecretClient != nil { - c.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := c.convert(obj) @@ -238,7 +240,7 @@ func (c *PSQLServerClient) Delete(ctx context.Context, obj runtime.Object, opts if status != "InProgress" { // Best case deletion of secrets secretKey := secrets.SecretKey{Name: instance.Name, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - c.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } } diff --git a/pkg/resourcemanager/rediscaches/actions/rediscacheaction_reconcile.go b/pkg/resourcemanager/rediscaches/actions/rediscacheaction_reconcile.go index 0a9a554ca23..f650104a982 100644 --- a/pkg/resourcemanager/rediscaches/actions/rediscacheaction_reconcile.go +++ b/pkg/resourcemanager/rediscaches/actions/rediscacheaction_reconcile.go @@ -20,8 +20,9 @@ func (m *AzureRedisCacheActionManager) Ensure(ctx context.Context, obj runtime.O opt(options) } + secretClient := m.SecretClient if options.SecretClient != nil { - m.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := m.convert(obj) @@ -61,7 +62,7 @@ func (m *AzureRedisCacheActionManager) Ensure(ctx context.Context, obj runtime.O ResourceGroupName: instance.Spec.ResourceGroup, }, } - if err = m.ListKeysAndCreateSecrets(ctx, cacheInstance); err != nil { + if err = m.ListKeysAndCreateSecrets(ctx, secretClient, cacheInstance); err != nil { instance.Status.Provisioning = true instance.Status.Provisioned = false instance.Status.FailedProvisioning = true diff --git a/pkg/resourcemanager/rediscaches/redis/rediscache_reconcile.go b/pkg/resourcemanager/rediscaches/redis/rediscache_reconcile.go index 0c1e6d3c0df..66f03fcd6bd 100644 --- a/pkg/resourcemanager/rediscaches/redis/rediscache_reconcile.go +++ b/pkg/resourcemanager/rediscaches/redis/rediscache_reconcile.go @@ -26,8 +26,9 @@ func (rc *AzureRedisCacheManager) Ensure(ctx context.Context, obj runtime.Object opt(options) } + secretClient := rc.SecretClient if options.SecretClient != nil { - rc.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := rc.convert(obj) @@ -44,7 +45,7 @@ func (rc *AzureRedisCacheManager) Ensure(ctx context.Context, obj runtime.Object // succeeded! so end reconcilliation successfully if newRc.ProvisioningState == "Succeeded" { - err = rc.ListKeysAndCreateSecrets(ctx, instance) + err = rc.ListKeysAndCreateSecrets(ctx, secretClient, instance) if err != nil { instance.Status.Message = err.Error() return false, err @@ -115,8 +116,9 @@ func (rc *AzureRedisCacheManager) Delete(ctx context.Context, obj runtime.Object opt(options) } + secretClient := rc.SecretClient if options.SecretClient != nil { - rc.SecretClient = options.SecretClient + secretClient = options.SecretClient } instance, err := rc.convert(obj) @@ -138,7 +140,7 @@ func (rc *AzureRedisCacheManager) Delete(ctx context.Context, obj runtime.Object if err != nil { if resp.StatusCode == http.StatusNotFound { // Best case deletion of secrets - rc.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } return false, err @@ -166,7 +168,7 @@ func (rc *AzureRedisCacheManager) Delete(ctx context.Context, obj runtime.Object } if helpers.ContainsString(finished, azerr.Type) { // Best case deletion of secrets - rc.SecretClient.Delete(ctx, secretKey) + secretClient.Delete(ctx, secretKey) return false, nil } return true, err diff --git a/pkg/resourcemanager/rediscaches/shared.go b/pkg/resourcemanager/rediscaches/shared.go index d0e30d28caa..e95cd9159a2 100644 --- a/pkg/resourcemanager/rediscaches/shared.go +++ b/pkg/resourcemanager/rediscaches/shared.go @@ -46,14 +46,14 @@ func (m *AzureRedisManager) ListKeys(ctx context.Context, resourceGroupName stri } // CreateSecrets creates a secret for a redis cache -func (m *AzureRedisManager) CreateSecrets(ctx context.Context, instance *v1alpha1.RedisCache, data map[string][]byte) error { +func (m *AzureRedisManager) CreateSecrets(ctx context.Context, secretClient secrets.SecretClient, instance *v1alpha1.RedisCache, data map[string][]byte) error { secretName := instance.Spec.SecretName if secretName == "" { secretName = instance.Name } secretKey := secrets.SecretKey{Name: secretName, Namespace: instance.Namespace, Kind: instance.TypeMeta.Kind} - err := m.SecretClient.Upsert( + err := secretClient.Upsert( ctx, secretKey, data, @@ -68,7 +68,7 @@ func (m *AzureRedisManager) CreateSecrets(ctx context.Context, instance *v1alpha } // ListKeysAndCreateSecrets lists keys and creates secrets -func (m *AzureRedisManager) ListKeysAndCreateSecrets(ctx context.Context, instance *v1alpha1.RedisCache) error { +func (m *AzureRedisManager) ListKeysAndCreateSecrets(ctx context.Context, secretClient secrets.SecretClient, instance *v1alpha1.RedisCache) error { var err error var result redis.AccessKeys @@ -83,6 +83,7 @@ func (m *AzureRedisManager) ListKeysAndCreateSecrets(ctx context.Context, instan err = m.CreateSecrets( ctx, + secretClient, instance, data, ) From d52c644e1db8b3d7730cd055c618b4b732bb865f Mon Sep 17 00:00:00 2001 From: Matthew Christopher Date: Wed, 17 Feb 2021 11:13:30 -0800 Subject: [PATCH 5/5] Change region VM tests are run in - Due to capacity constraints. We can move back later. --- controllers/azurevirtualmachine_controller_test.go | 14 +++++++++----- ...azurevirtualmachineextension_controller_test.go | 13 ++++++++----- controllers/azurevmscaleset_controller_test.go | 11 +++++++---- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/controllers/azurevirtualmachine_controller_test.go b/controllers/azurevirtualmachine_controller_test.go index 7ae5bb7cd41..69dfcfd0408 100644 --- a/controllers/azurevirtualmachine_controller_test.go +++ b/controllers/azurevirtualmachine_controller_test.go @@ -27,6 +27,7 @@ func TestVirtualMachineControllerNoResourceGroup(t *testing.T) { sshPublicKeyData := GenerateTestResourceNameWithRandom("ssh", 10) nicName := GenerateTestResourceNameWithRandom("nic", 10) platformImageUrn := "Canonical:UbuntuServer:16.04-LTS:latest" + location := tc.resourceGroupLocation // Create a VM vmInstance := &azurev1alpha1.AzureVirtualMachine{ @@ -35,7 +36,7 @@ func TestVirtualMachineControllerNoResourceGroup(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureVirtualMachineSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: rgName, VMSize: vmSize, OSType: osType, @@ -56,6 +57,9 @@ func TestVirtualMachineHappyPathWithNicPipVNetAndSubnet(t *testing.T) { defer PanicRecover(t) ctx := context.Background() + // location := tc.resourceGroupLocation + location := "australiaeast" + // Create a vnet with a subnet vnetName := GenerateTestResourceNameWithRandom("vnt", 10) subnetName := GenerateTestResourceNameWithRandom("snt", 10) @@ -70,7 +74,7 @@ func TestVirtualMachineHappyPathWithNicPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.VirtualNetworkSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, AddressSpace: "110.0.0.0/8", Subnets: []azurev1alpha1.VNetSubnets{vnetSubNetInstance}, @@ -90,7 +94,7 @@ func TestVirtualMachineHappyPathWithNicPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzurePublicIPAddressSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, PublicIPAllocationMethod: publicIPAllocationMethod, IdleTimeoutInMinutes: idleTimeoutInMinutes, @@ -109,7 +113,7 @@ func TestVirtualMachineHappyPathWithNicPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureNetworkInterfaceSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, VNetName: vnetName, SubnetName: subnetName, @@ -134,7 +138,7 @@ func TestVirtualMachineHappyPathWithNicPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureVirtualMachineSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, VMSize: vmSize, OSType: osType, diff --git a/controllers/azurevirtualmachineextension_controller_test.go b/controllers/azurevirtualmachineextension_controller_test.go index 7c05a5ee9d4..55cbd191976 100644 --- a/controllers/azurevirtualmachineextension_controller_test.go +++ b/controllers/azurevirtualmachineextension_controller_test.go @@ -52,6 +52,9 @@ func TestVirtualMachineExtensionHappyPath(t *testing.T) { defer PanicRecover(t) ctx := context.Background() + // location := tc.resourceGroupLocation + location := "australiaeast" + // Create a vnet with a subnet vnetName := GenerateTestResourceNameWithRandom("vnt", 10) subnetName := GenerateTestResourceNameWithRandom("snt", 10) @@ -66,7 +69,7 @@ func TestVirtualMachineExtensionHappyPath(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.VirtualNetworkSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, AddressSpace: "110.0.0.0/8", Subnets: []azurev1alpha1.VNetSubnets{vnetSubNetInstance}, @@ -86,7 +89,7 @@ func TestVirtualMachineExtensionHappyPath(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzurePublicIPAddressSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, PublicIPAllocationMethod: publicIPAllocationMethod, IdleTimeoutInMinutes: idleTimeoutInMinutes, @@ -105,7 +108,7 @@ func TestVirtualMachineExtensionHappyPath(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureNetworkInterfaceSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, VNetName: vnetName, SubnetName: subnetName, @@ -130,7 +133,7 @@ func TestVirtualMachineExtensionHappyPath(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureVirtualMachineSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, VMSize: vmSize, OSType: osType, @@ -151,7 +154,7 @@ func TestVirtualMachineExtensionHappyPath(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureVirtualMachineExtensionSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, VMName: vmName, AutoUpgradeMinorVersion: true, diff --git a/controllers/azurevmscaleset_controller_test.go b/controllers/azurevmscaleset_controller_test.go index 9391a7f7d23..c518bdc2226 100644 --- a/controllers/azurevmscaleset_controller_test.go +++ b/controllers/azurevmscaleset_controller_test.go @@ -66,6 +66,9 @@ func TestVMScaleSetHappyPathWithLbPipVNetAndSubnet(t *testing.T) { defer PanicRecover(t) ctx := context.Background() + // location := tc.resourceGroupLocation + location := "australiaeast" + // Create a vnet with a subnet vnetName := GenerateTestResourceNameWithRandom("vnt", 10) subnetName := GenerateTestResourceNameWithRandom("snt", 10) @@ -80,7 +83,7 @@ func TestVMScaleSetHappyPathWithLbPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.VirtualNetworkSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, AddressSpace: "110.0.0.0/8", Subnets: []azurev1alpha1.VNetSubnets{vnetSubNetInstance}, @@ -100,7 +103,7 @@ func TestVMScaleSetHappyPathWithLbPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzurePublicIPAddressSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, PublicIPAllocationMethod: publicIPAllocationMethod, IdleTimeoutInMinutes: idleTimeoutInMinutes, @@ -124,7 +127,7 @@ func TestVMScaleSetHappyPathWithLbPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureLoadBalancerSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, PublicIPAddressName: pipName, BackendAddressPoolName: bpName, @@ -152,7 +155,7 @@ func TestVMScaleSetHappyPathWithLbPipVNetAndSubnet(t *testing.T) { Namespace: "default", }, Spec: azurev1alpha1.AzureVMScaleSetSpec{ - Location: tc.resourceGroupLocation, + Location: location, ResourceGroup: tc.resourceGroupName, VMSize: vmSize, Capacity: capacity,