Skip to content

Commit

Permalink
SecretClient should not be modified
Browse files Browse the repository at this point in the history
  • Loading branch information
matthchr committed Feb 17, 2021
1 parent 169b7ac commit 72c8234
Show file tree
Hide file tree
Showing 15 changed files with 135 additions and 86 deletions.
19 changes: 15 additions & 4 deletions pkg/resourcemanager/appinsights/api_keys_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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)},
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 11 additions & 8 deletions pkg/resourcemanager/appinsights/appinsights.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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),
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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,
)
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
}
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/resourcemanager/appinsights/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -86,15 +87,15 @@ 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 {
// Create a new secret
secret := fg.NewSecret(instance)

// create or update the secret
err = fg.SecretClient.Upsert(
err = secretClient.Upsert(
ctx,
secretKey,
secret,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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")

Expand Down
2 changes: 2 additions & 0 deletions pkg/resourcemanager/azuresql/azuresqlserver/azuresqlserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package azuresqlserver
import (
"context"
"errors"
"fmt"
"net/http"

"github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql"
Expand All @@ -27,6 +28,7 @@ type AzureSqlServerManager struct {
}

func NewAzureSqlServerManager(creds config.Credentials, secretClient secrets.SecretClient, scheme *runtime.Scheme) *AzureSqlServerManager {
print(fmt.Sprintf("matthchr: SQLServerManager: %T\n", secretClient))
return &AzureSqlServerManager{
Creds: creds,
SecretClient: secretClient,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -293,15 +295,15 @@ 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
}

return false, err
}

//Best effort deletion of secrets
s.SecretClient.Delete(ctx, secretKey)
secretClient.Delete(ctx, secretKey)
return false, nil
}

Expand Down
Loading

0 comments on commit 72c8234

Please sign in to comment.