Remove legacy secret

Co-authored-by: Danail Branekov <[email protected]>

Author: uzabanov

controllers/controllers/services/bindings/controller_test.go

@@ -462,54 +462,6 @@ var _ = Describe("CFServiceBinding", func() {
 			})
 		})
 
-		When("the binding references a 'legacy' instance credentials secret", func() {
-			JustBeforeEach(func() {
-				Expect(k8s.Patch(ctx, adminClient, instance, func() {
-					instance.Spec.SecretName = instance.Name
-					instance.Status.Credentials.Name = instance.Name
-				})).To(Succeed())
-
-				Eventually(func(g Gomega) {
-					g.Expect(k8s.Patch(ctx, adminClient, binding, func() {
-						binding.Status.Binding.Name = instance.Name
-					})).To(Succeed())
-
-					// Ensure that the binding controller has observed the patch operation above
-					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
-					g.Expect(binding.Generation).To(Equal(binding.Status.ObservedGeneration))
-					g.Expect(binding.Status.Binding.Name).To(Equal(instance.Name))
-				}).Should(Succeed())
-			})
-
-			It("sets the binding Ready status condition to false", func() {
-				Eventually(func(g Gomega) {
-					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
-					g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll(
-						HasType(Equal(korifiv1alpha1.StatusConditionReady)),
-						HasStatus(Equal(metav1.ConditionFalse)),
-					)))
-				}).Should(Succeed())
-			})
-
-			When("the referenced legacy binding secret exists", func() {
-				BeforeEach(func() {
-					Expect(adminClient.Create(ctx, &corev1.Secret{
-						ObjectMeta: metav1.ObjectMeta{
-							Name:      instance.Name,
-							Namespace: testNamespace,
-						},
-					})).To(Succeed())
-				})
-
-				It("does not update the binding status", func() {
-					Consistently(func(g Gomega) {
-						g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
-						g.Expect(binding.Status.Binding.Name).To(Equal(instance.Name))
-					}).Should(Succeed())
-				})
-			})
-		})
-
 		When("the binding is deleted", func() {
 			JustBeforeEach(func() {
 				Expect(adminClient.Delete(ctx, binding)).To(Succeed())

controllers/controllers/services/bindings/upsi/controller.go

@@ -77,39 +77,9 @@ func (r *UPSIBindingReconciler) ReconcileResource(ctx context.Context, cfService
 	return ctrl.Result{}, nil
 }
 
-func isLegacyServiceBinding(cfServiceBinding *korifiv1alpha1.CFServiceBinding, cfServiceInstance *korifiv1alpha1.CFServiceInstance) bool {
-	if cfServiceBinding.Status.Binding.Name == "" {
-		return false
-	}
-
-	// When reconciling existing legacy service bindings we make
-	// use of the fact that the service binding used to reference
-	// the secret of the sevice instance that shares the sevice
-	// instance name. See ADR 16 for more datails.
-	return cfServiceInstance.Name == cfServiceBinding.Status.Binding.Name && cfServiceInstance.Spec.SecretName == cfServiceBinding.Status.Binding.Name
-}
-
 func (r *UPSIBindingReconciler) reconcileCredentials(ctx context.Context, cfServiceInstance *korifiv1alpha1.CFServiceInstance, cfServiceBinding *korifiv1alpha1.CFServiceBinding) error {
 	cfServiceBinding.Status.Credentials.Name = cfServiceInstance.Status.Credentials.Name
 
-	if isLegacyServiceBinding(cfServiceBinding, cfServiceInstance) {
-		bindingSecret := &corev1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      cfServiceBinding.Status.Binding.Name,
-				Namespace: cfServiceBinding.Namespace,
-			},
-		}
-
-		// For legacy sevice bindings we want to keep the binding secret
-		// unchanged in order to avoid unexpected app restarts. See ADR 16 for more details.
-		err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(bindingSecret), bindingSecret)
-		if err != nil {
-			return err
-		}
-
-		return nil
-	}
-
 	credentialsSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: cfServiceInstance.Namespace,

controllers/controllers/services/instances/upsi/controller.go

@@ -19,11 +19,9 @@ package upsi
 import (
 	"context"
 	"encoding/json"
-	"strings"
 	"time"
 
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
-	"code.cloudfoundry.org/korifi/controllers/controllers/services/credentials"
 	"code.cloudfoundry.org/korifi/controllers/controllers/shared"
 	"code.cloudfoundry.org/korifi/tools"
 	"code.cloudfoundry.org/korifi/tools/k8s"
@@ -38,7 +36,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -127,11 +124,6 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceInstance *k
 		return ctrl.Result{}, notReadyErr
 	}
 
-	credentialsSecret, err = r.reconcileCredentials(ctx, credentialsSecret, cfServiceInstance)
-	if err != nil {
-		return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("FailedReconcilingCredentialsSecret")
-	}
-
 	if err = r.validateCredentials(credentialsSecret); err != nil {
 		return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("SecretInvalid")
 	}
@@ -143,46 +135,6 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceInstance *k
 	return ctrl.Result{}, nil
 }
 
-func (r *Reconciler) reconcileCredentials(ctx context.Context, credentialsSecret *corev1.Secret, cfServiceInstance *korifiv1alpha1.CFServiceInstance) (*corev1.Secret, error) {
-	if !strings.HasPrefix(string(credentialsSecret.Type), credentials.ServiceBindingSecretTypePrefix) {
-		return credentialsSecret, nil
-	}
-
-	log := logr.FromContextOrDiscard(ctx)
-
-	log.Info("migrating legacy secret", "legacy-secret-name", credentialsSecret.Name)
-	migratedSecret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      cfServiceInstance.Name + "-migrated",
-			Namespace: cfServiceInstance.Namespace,
-		},
-	}
-	_, err := controllerutil.CreateOrPatch(ctx, r.k8sClient, migratedSecret, func() error {
-		migratedSecret.Type = corev1.SecretTypeOpaque
-		data := map[string]any{}
-		for k, v := range credentialsSecret.Data {
-			data[k] = string(v)
-		}
-
-		dataBytes, err := json.Marshal(data)
-		if err != nil {
-			log.Error(err, "failed to marshal legacy credentials secret data")
-			return err
-		}
-
-		migratedSecret.Data = map[string][]byte{
-			tools.CredentialsSecretKey: dataBytes,
-		}
-		return controllerutil.SetOwnerReference(cfServiceInstance, migratedSecret, r.scheme)
-	})
-	if err != nil {
-		log.Error(err, "failed to create migrated credentials secret")
-		return nil, err
-	}
-
-	return migratedSecret, nil
-}
-
 func (r *Reconciler) validateCredentials(credentialsSecret *corev1.Secret) error {
 	return errors.Wrapf(
 		json.Unmarshal(credentialsSecret.Data[tools.CredentialsSecretKey], &map[string]any{}),

controllers/controllers/services/instances/upsi/controller_test.go

@@ -1,10 +1,7 @@
 package upsi_test
 
 import (
-	"encoding/json"
-
 	"github.com/google/uuid"
-	. "github.com/onsi/gomega/gstruct"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
@@ -174,112 +171,6 @@ var _ = Describe("CFServiceInstance", func() {
 				})
 			})
 		})
-
-		When("the instance credentials secret is in the 'legacy' format", func() {
-			var credentialsSecret *corev1.Secret
-
-			getMigratedSecret := func() *corev1.Secret {
-				migratedSecret := &corev1.Secret{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      instance.Name + "-migrated",
-						Namespace: testNamespace,
-					},
-				}
-				Eventually(func(g Gomega) {
-					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(migratedSecret), migratedSecret)).To(Succeed())
-				}).Should(Succeed())
-
-				return migratedSecret
-			}
-
-			JustBeforeEach(func() {
-				credentialsSecret = &corev1.Secret{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      uuid.NewString(),
-						Namespace: testNamespace,
-					},
-					Type: corev1.SecretType("servicebinding.io/legacy"),
-					StringData: map[string]string{
-						"foo": "bar",
-					},
-				}
-				Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed())
-
-				Expect(k8s.PatchResource(ctx, adminClient, instance, func() {
-					instance.Spec.SecretName = credentialsSecret.Name
-				})).To(Succeed())
-			})
-
-			It("creates a derived secret in the new format", func() {
-				Eventually(func(g Gomega) {
-					migratedSecret := getMigratedSecret()
-					g.Expect(migratedSecret.Type).To(Equal(corev1.SecretTypeOpaque))
-					g.Expect(migratedSecret.Data).To(MatchAllKeys(Keys{
-						tools.CredentialsSecretKey: Not(BeEmpty()),
-					}))
-
-					credentials := map[string]any{}
-					g.Expect(json.Unmarshal(migratedSecret.Data[tools.CredentialsSecretKey], &credentials)).To(Succeed())
-					g.Expect(credentials).To(MatchAllKeys(Keys{
-						"foo": Equal("bar"),
-					}))
-				}).Should(Succeed())
-			})
-
-			It("sets an owner reference from the service instance to the migrated secret", func() {
-				Eventually(func(g Gomega) {
-					migratedSecret := getMigratedSecret()
-					g.Expect(migratedSecret.OwnerReferences).To(ConsistOf(MatchFields(IgnoreExtras, Fields{
-						"Kind": Equal("CFServiceInstance"),
-						"Name": Equal(instance.Name),
-					})))
-				}).Should(Succeed())
-			})
-
-			It("sets the instance credentials secret name and observed version to the migrated secret name and version", func() {
-				Eventually(func(g Gomega) {
-					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
-					g.Expect(instance.Status.Credentials.Name).To(Equal(instance.Name + "-migrated"))
-					g.Expect(instance.Status.CredentialsObservedVersion).To(Equal(getMigratedSecret().ResourceVersion))
-				}).Should(Succeed())
-			})
-
-			It("does not change the original credentials secret", func() {
-				Eventually(func(g Gomega) {
-					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
-					g.Expect(instance.Status.Credentials.Name).NotTo(BeEmpty())
-
-					g.Expect(instance.Spec.SecretName).To(Equal(credentialsSecret.Name))
-
-					previousCredentialsVersion := credentialsSecret.ResourceVersion
-					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed())
-					g.Expect(credentialsSecret.ResourceVersion).To(Equal(previousCredentialsVersion))
-				}).Should(Succeed())
-			})
-
-			When("legacy secret cannot be migrated", func() {
-				BeforeEach(func() {
-					Expect(adminClient.Create(ctx, &corev1.Secret{
-						ObjectMeta: metav1.ObjectMeta{
-							Name:      instance.Name + "-migrated",
-							Namespace: instance.Namespace,
-						},
-						Type: corev1.SecretType("will-clash-with-migrated-secret-type"),
-					})).To(Succeed())
-				})
-
-				It("sets the CredentialSecretAvailable condition to false", func() {
-					Eventually(func(g Gomega) {
-						g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
-						g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll(
-							HasType(Equal(korifiv1alpha1.StatusConditionReady)),
-							HasStatus(Equal(metav1.ConditionFalse)),
-							HasReason(Equal("FailedReconcilingCredentialsSecret")),
-						)))
-					}).Should(Succeed())
-				})
-			})
-		})
 	})
 
 	When("the service instance is managed", func() {