addressing pr feedback, getting owner in kube secret client

controllers/async_controller.go

@@ -157,8 +157,6 @@ func (r *AsyncReconciler) Reconcile(req ctrl.Request, obj runtime.Object) (resul
 
 	r.Telemetry.LogInfoByInstance("status", "reconciling object", req.String())
 
-	configOptions = append(configOptions, resourcemanager.WithClient(r.Client))
-
 	if len(KeyVaultName) != 0 { //KeyVault was specified in Spec, so use that for secrets
 		configOptions = append(configOptions, resourcemanager.WithSecretClient(keyvaultSecretClient))
 	}

pkg/resourcemanager/interfaces.go

@@ -10,7 +10,6 @@ import (
 	"github.com/Azure/azure-service-operator/pkg/secrets"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 const (
@@ -19,7 +18,6 @@ const (
 
 // Options contains the inputs available for passing to Ensure optionally
 type Options struct {
-	Client       client.Client
 	SecretClient secrets.SecretClient
 }
 
@@ -33,13 +31,6 @@ func WithSecretClient(secretClient secrets.SecretClient) ConfigOption {
 	}
 }
 
-// WithClient can be used to pass in a k8s Client
-func WithClient(client client.Client) ConfigOption {
-	return func(op *Options) {
-		op.Client = client
-	}
-}
-
 type KubeParent struct {
 	Key    types.NamespacedName
 	Target runtime.Object

pkg/resourcemanager/rediscaches/rediscache_reconcile.go

@@ -33,21 +33,16 @@ func (rc *AzureRedisCacheManager) Ensure(ctx context.Context, obj runtime.Object
 		return false, err
 	}
 
-	redisName := instance.Name
+	name := types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}
 	groupName := instance.Spec.ResourceGroupName
-	name := instance.ObjectMeta.Name
-
-	if len(instance.Spec.SecretName) == 0 {
-		instance.Spec.SecretName = redisName
-	}
 
 	// if an error occurs thats ok as it means that it doesn't exist yet
-	newRc, err := rc.GetRedisCache(ctx, groupName, name)
+	newRc, err := rc.GetRedisCache(ctx, groupName, name.Name)
 	if err == nil {
 
 		// succeeded! so end reconcilliation successfully
 		if newRc.ProvisioningState == "Succeeded" {
-			err = rc.ListKeysAndCreateSecrets(ctx, groupName, redisName, instance.Spec.SecretName, instance)
+			err = rc.ListKeysAndCreateSecrets(ctx, instance)
 			if err != nil {
 				instance.Status.Message = err.Error()
 				return false, err

pkg/resourcemanager/rediscaches/rediscacheaction_reconcile.go

@@ -9,7 +9,7 @@ import (
 
 	"github.com/Azure/azure-service-operator/api/v1alpha1"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager"
-
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -20,57 +20,50 @@ func (m *AzureRedisCacheActionManager) Ensure(ctx context.Context, obj runtime.O
 		opt(options)
 	}
 
-	actionInstance, err := m.convert(obj)
+	instance, err := m.convert(obj)
 	if err != nil {
 		return true, err
 	}
 
 	// never re-provision an action
-	if actionInstance.Status.Provisioned {
+	if instance.Status.Provisioned {
 		return true, nil
 	}
 
-	// get the RedisCache instance to see if it's done provisioning and set it as an owner
-	cacheInstance := &v1alpha1.RedisCache{}
-	cacheName := types.NamespacedName{Name: actionInstance.Spec.CacheName, Namespace: actionInstance.Namespace}
-	err = options.Client.Get(ctx, cacheName, cacheInstance)
-	if err != nil {
-		return false, err
-	}
+	rollAllKeys := instance.Spec.ActionName == v1alpha1.RedisCacheActionNameRollAllKeys
 
-	if cacheInstance.Status.FailedProvisioning || cacheInstance.Status.Provisioning {
-		actionInstance.Status.Message = "Waiting for parent RedisCache to finish provisioning"
-		return false, nil
-	}
-
-	rollAllKeys := actionInstance.Spec.ActionName == v1alpha1.RedisCacheActionNameRollAllKeys
-
-	if rollAllKeys || actionInstance.Spec.ActionName == v1alpha1.RedisCacheActionNameRollPrimaryKey {
-		if err = m.RegeneratePrimaryAccessKey(ctx, actionInstance.Spec.ResourceGroup, actionInstance.Spec.CacheName); err != nil {
-			actionInstance.Status.Provisioned = false
-			actionInstance.Status.FailedProvisioning = true
+	if rollAllKeys || instance.Spec.ActionName == v1alpha1.RedisCacheActionNameRollPrimaryKey {
+		if err = m.RegeneratePrimaryAccessKey(ctx, instance.Spec.ResourceGroup, instance.Spec.CacheName); err != nil {
 			return false, err
 		}
 	}
 
-	if rollAllKeys || actionInstance.Spec.ActionName == v1alpha1.RedisCacheActionNameRollSecondaryKey {
-		if err = m.RegenerateSecondaryAccessKey(ctx, actionInstance.Spec.ResourceGroup, actionInstance.Spec.CacheName); err != nil {
-			actionInstance.Status.Provisioned = false
-			actionInstance.Status.FailedProvisioning = true
+	if rollAllKeys || instance.Spec.ActionName == v1alpha1.RedisCacheActionNameRollSecondaryKey {
+		if err = m.RegenerateSecondaryAccessKey(ctx, instance.Spec.ResourceGroup, instance.Spec.CacheName); err != nil {
 			return false, err
 		}
 	}
 
 	// regenerate the secret
-	if err = m.ListKeysAndCreateSecrets(ctx, actionInstance.Spec.ResourceGroup, actionInstance.Spec.CacheName, cacheInstance.Spec.SecretName, cacheInstance); err != nil {
-		actionInstance.Status.Provisioned = false
-		actionInstance.Status.FailedProvisioning = true
+	cacheInstance := &v1alpha1.RedisCache{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      instance.Spec.CacheName,
+			Namespace: instance.Namespace,
+		},
+		Spec: v1alpha1.RedisCacheSpec{
+			SecretName:        instance.Spec.SecretName,
+			ResourceGroupName: instance.Spec.ResourceGroup,
+		},
+	}
+	if err = m.ListKeysAndCreateSecrets(ctx, cacheInstance); err != nil {
+		instance.Status.Provisioned = false
+		instance.Status.FailedProvisioning = true
 		return false, err
 	}
 
 	// successful return
-	actionInstance.Status.Provisioned = true
-	actionInstance.Status.FailedProvisioning = false
+	instance.Status.Provisioned = true
+	instance.Status.FailedProvisioning = false
 	return true, nil
 }
 

pkg/resourcemanager/rediscaches/shared.go

@@ -43,7 +43,12 @@ func (r *AzureRedisManager) ListKeys(ctx context.Context, resourceGroupName stri
 }
 
 // CreateSecrets creates a secret for a redis cache
-func (r *AzureRedisManager) CreateSecrets(ctx context.Context, secretName string, instance *v1alpha1.RedisCache, data map[string][]byte) error {
+func (r *AzureRedisManager) CreateSecrets(ctx context.Context, instance *v1alpha1.RedisCache, data map[string][]byte) error {
+	secretName := instance.Spec.SecretName
+	if secretName == "" {
+		secretName = instance.Name
+	}
+
 	key := types.NamespacedName{Name: secretName, Namespace: instance.Namespace}
 
 	err := r.SecretClient.Upsert(
@@ -61,11 +66,11 @@ func (r *AzureRedisManager) CreateSecrets(ctx context.Context, secretName string
 }
 
 // ListKeysAndCreateSecrets lists keys and creates secrets
-func (r *AzureRedisManager) ListKeysAndCreateSecrets(ctx context.Context, resourceGroupName string, redisCacheName string, secretName string, instance *v1alpha1.RedisCache) error {
+func (r *AzureRedisManager) ListKeysAndCreateSecrets(ctx context.Context, instance *v1alpha1.RedisCache) error {
 	var err error
 	var result redis.AccessKeys
 
-	result, err = r.ListKeys(ctx, resourceGroupName, redisCacheName)
+	result, err = r.ListKeys(ctx, instance.Spec.ResourceGroupName, instance.Name)
 	if err != nil {
 		return err
 	}
@@ -76,7 +81,6 @@ func (r *AzureRedisManager) ListKeysAndCreateSecrets(ctx context.Context, resour
 
 	err = r.CreateSecrets(
 		ctx,
-		secretName,
 		instance,
 		data,
 	)

pkg/secrets/interface.go

@@ -9,7 +9,6 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -20,9 +19,14 @@ type SecretClient interface {
 	Get(ctx context.Context, key types.NamespacedName) (map[string][]byte, error)
 }
 
+type SecretOwner interface {
+	runtime.Object
+	metav1.Object
+}
+
 // Options contains the inputs available for passing to some methods of the secret clients
 type Options struct {
-	Owner     metav1.Object
+	Owner     SecretOwner
 	Scheme    *runtime.Scheme
 	Activates *time.Time
 	Expires   *time.Time
@@ -47,11 +51,10 @@ func WithExpiration(expireAfter *time.Time) SecretOption {
 }
 
 // WithOwner allows setting an owning instance in the options struct
-func WithOwner(owner metav1.Object) SecretOption {
+func WithOwner(owner SecretOwner) SecretOption {
 	return func(op *Options) {
 		op.Owner = owner
 	}
-
 }
 
 // WithScheme allows setting a runtime.Scheme in the options

pkg/secrets/kube/client.go

@@ -86,6 +86,14 @@ func (k *KubeSecretClient) Upsert(ctx context.Context, key types.NamespacedName,
 		}
 
 		if options.Owner != nil && options.Scheme != nil {
+			// the uid is required for SetControllerReference, try to populate it if it isn't
+			if options.Owner.GetUID() == "" {
+				ownerKey := types.NamespacedName{Name: options.Owner.GetName(), Namespace: options.Owner.GetNamespace()}
+				if err := k.KubeClient.Get(ctx, ownerKey, options.Owner); err != nil {
+					return err
+				}
+			}
+
 			if err := controllerutil.SetControllerReference(options.Owner, secret, options.Scheme); err != nil {
 				return err
 			}