Merge pull request #575 from 3scale/remove-apimanager-secret-ownerref…

…erences

apimanager: Do not set APIManager controller-based ownerReferences on Secrets

pkg/3scale/amp/operator/base_apimanager_logic_reconciler.go

@@ -181,8 +181,17 @@ func (r *BaseAPIManagerLogicReconciler) ReconcilePodMonitor(desired *monitoringv
 
 func (r *BaseAPIManagerLogicReconciler) ReconcileResource(obj, desired common.KubernetesObject, mutatefn reconcilers.MutateFn) error {
 	desired.SetNamespace(r.apiManager.GetNamespace())
-	if err := r.SetOwnerReference(r.apiManager, desired); err != nil {
-		return err
+
+	// Secrets are managed by users so they do not get APIManager-based
+	// owned references. In case we want to react to changes to secrets
+	// in the future we will need to implement an alternative mechanism to
+	// controller-based OwnerReferences due to user-managed secrets might
+	// already have controller-based OwnerReferences and K8s objects
+	// can only be owned by a single controller-based OwnerReference.
+	if desired.GetObjectKind().GroupVersionKind().Kind != "Secret" {
+		if err := r.SetOwnerReference(r.apiManager, desired); err != nil {
+			return err
+		}
 	}
 
 	return r.BaseReconciler.ReconcileResource(obj, desired, r.APIManagerMutator(mutatefn))
@@ -195,14 +204,22 @@ func (r *BaseAPIManagerLogicReconciler) APIManagerMutator(mutateFn reconcilers.M
 		// Metadata
 		updated := helper.EnsureObjectMeta(existing, desired)
 
-		// OwnerRefenrence
-		updatedTmp, err := r.EnsureOwnerReference(r.apiManager, existing)
-		if err != nil {
-			return false, err
+		// Secrets are managed by users so they do not get APIManager-based
+		// owned references. In case we want to react to changes to secrets
+		// in the future we will need to implement an alternative mechanism to
+		// controller-based OwnerReferences due to user-managed secrets might
+		// already have controller-based OwnerReferences and K8s objects
+		// can only be owned by a single controller-based OwnerReference.
+		if existing.GetObjectKind().GroupVersionKind().Kind != "Secret" {
+			// OwnerRefenrence
+			updatedTmp, err := r.EnsureOwnerReference(r.apiManager, existing)
+			if err != nil {
+				return false, err
+			}
+			updated = updated || updatedTmp
 		}
-		updated = updated || updatedTmp
 
-		updatedTmp, err = mutateFn(existing, desired)
+		updatedTmp, err := mutateFn(existing, desired)
 		if err != nil {
 			return false, err
 		}

pkg/3scale/amp/operator/highavailability_reconciler_test.go

@@ -77,21 +77,6 @@ func TestHighAvailabilityReconciler(t *testing.T) {
 				subT.Errorf("error fetching object %s: %v", tc.secretName, err)
 			}
 
-			// Assert owner ref
-			if len(secret.GetOwnerReferences()) == 0 {
-				subT.Errorf("secret %s: owner ref not set", tc.secretName)
-			}
-
-			ownerRef := secret.GetOwnerReferences()[0]
-
-			// apimanager.Kind is empty
-			if ownerRef.Kind != "APIManager" {
-				subT.Errorf("secret %s: owner ref kind not matched. Expected: %s, found: %s", tc.secretName, "APIManager", ownerRef.Kind)
-			}
-
-			if ownerRef.Name != apimanager.Name {
-				subT.Errorf("secret %s: owner ref name not matched. Expected: %s, found: %s", tc.secretName, apimanager.Name, ownerRef.Name)
-			}
 		})
 	}
 }

pkg/3scale/amp/operator/upgrade.go

@@ -5,7 +5,10 @@ import (
 	"fmt"
 	"reflect"
 
+	commonapps "github.com/3scale/3scale-operator/apis/apps"
 	appsv1alpha1 "github.com/3scale/3scale-operator/apis/apps/v1alpha1"
+
+	"github.com/3scale/3scale-operator/pkg/3scale/amp/component"
 	"github.com/3scale/3scale-operator/pkg/common"
 	"github.com/3scale/3scale-operator/pkg/helper"
 	"github.com/3scale/3scale-operator/pkg/reconcilers"
@@ -33,7 +36,15 @@ func NewUpgradeApiManager(b *reconcilers.BaseReconciler, apiManager *appsv1alpha
 }
 
 func (u *UpgradeApiManager) Upgrade() (reconcile.Result, error) {
-	res, err := u.upgradeBackendRouteEnv()
+	res, err := u.deleteAPIManagerOwnerReferencesFromSecrets()
+	if err != nil {
+		return res, fmt.Errorf("Deleting secrets APIManager owner references: %w", err)
+	}
+	if res.Requeue {
+		return res, nil
+	}
+
+	res, err = u.upgradeBackendRouteEnv()
 	if err != nil {
 		return res, fmt.Errorf("Upgrading backend route env vars: %w", err)
 	}
@@ -578,3 +589,50 @@ func (u *UpgradeApiManager) upgradeZyncPodTemplateAnnotations() (reconcile.Resul
 func (u *UpgradeApiManager) Logger() logr.Logger {
 	return u.logger
 }
+
+func (u *UpgradeApiManager) deleteAPIManagerOwnerReferencesFromSecrets() (reconcile.Result, error) {
+	secretsToUpdate := []string{
+		component.BackendSecretInternalApiSecretName,
+		component.BackendSecretBackendListenerSecretName,
+		component.BackendSecretBackendRedisSecretName,
+		component.SystemSecretSystemAppSecretName,
+		component.SystemSecretSystemDatabaseSecretName,
+		component.SystemSecretSystemEventsHookSecretName,
+		component.SystemSecretSystemMasterApicastSecretName,
+		component.SystemSecretSystemMemcachedSecretName,
+		component.SystemSecretSystemRecaptchaSecretName,
+		component.SystemSecretSystemRedisSecretName,
+		component.SystemSecretSystemSeedSecretName,
+		component.SystemSecretSystemSMTPSecretName,
+		component.ZyncSecretName,
+	}
+
+	someChanged := false
+	for _, secretName := range secretsToUpdate {
+		existing := &v1.Secret{}
+		err := u.Client().Get(context.TODO(), types.NamespacedName{Name: secretName, Namespace: u.apiManager.Namespace}, existing)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+
+		apimanagerOwnerReferenceIdx := -1
+		for currIdx, secretOwnerReference := range existing.OwnerReferences {
+			if secretOwnerReference.Kind == commonapps.APIManagerKind &&
+				secretOwnerReference.Controller != nil && *secretOwnerReference.Controller {
+				apimanagerOwnerReferenceIdx = currIdx
+			}
+		}
+
+		if apimanagerOwnerReferenceIdx != -1 {
+			u.Logger().Info(fmt.Sprintf("Removing APIManager OwnerReference from Secret %s", existing.Name))
+			existing.OwnerReferences = append(existing.OwnerReferences[:apimanagerOwnerReferenceIdx], existing.OwnerReferences[apimanagerOwnerReferenceIdx+1:]...)
+			err = u.UpdateResource(existing)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
+			someChanged = true
+		}
+	}
+
+	return reconcile.Result{Requeue: someChanged}, nil
+}