From 99dbc5ef1fff1b29d3b58bb9803356b4100ca1c9 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Tue, 15 Oct 2024 13:13:14 +0000 Subject: [PATCH] Remove support for pre-0.11.0 upsi credential secrets In 0.12.0 we have extended upsi credentials secret to be represented by arbitrary objects, not just plain maps of strings. That required certain migration logic to migrate the secret to the new format and avoid workloads restart during Korifi upgrade. With the upcoming release, workload restart would be performed anyway because of unrelated increments. Provided that 0.12.0 should have migrated any user-provided service instances to the new credentials format, we want to get rid of the migration code. fixes #3520 --- .../services/bindings/controller_test.go | 50 +------- .../services/bindings/upsi/controller.go | 30 ----- .../services/instances/upsi/controller.go | 48 -------- .../instances/upsi/controller_test.go | 109 ------------------ 4 files changed, 1 insertion(+), 236 deletions(-) diff --git a/controllers/controllers/services/bindings/controller_test.go b/controllers/controllers/services/bindings/controller_test.go index 86426554c..cf5c4ffba 100644 --- a/controllers/controllers/services/bindings/controller_test.go +++ b/controllers/controllers/services/bindings/controller_test.go @@ -212,7 +212,7 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) - XWhen("the service instance is managed", func() { + When("the service instance is managed", func() { BeforeEach(func() { Expect(k8s.PatchResource(ctx, adminClient, instance, func() { instance.Spec.Type = korifiv1alpha1.ManagedType @@ -471,52 +471,4 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) }) - - 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()) - }) - }) - }) }) diff --git a/controllers/controllers/services/bindings/upsi/controller.go b/controllers/controllers/services/bindings/upsi/controller.go index db9c37efb..97313ff06 100644 --- a/controllers/controllers/services/bindings/upsi/controller.go +++ b/controllers/controllers/services/bindings/upsi/controller.go @@ -72,39 +72,9 @@ func (r *CredentialsReconciler) 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 *CredentialsReconciler) 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, diff --git a/controllers/controllers/services/instances/upsi/controller.go b/controllers/controllers/services/instances/upsi/controller.go index b889af670..04068d89c 100644 --- a/controllers/controllers/services/instances/upsi/controller.go +++ b/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{}), diff --git a/controllers/controllers/services/instances/upsi/controller_test.go b/controllers/controllers/services/instances/upsi/controller_test.go index 0aa9cfa50..3ca2247bd 100644 --- a/controllers/controllers/services/instances/upsi/controller_test.go +++ b/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() {