Skip to content

Commit

Permalink
Remove support for pre-0.11.0 upsi credential secrets
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@danail-branekov
danail-branekov committed Oct 15, 2024
1 parent c20370f commit 99dbc5e
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 236 deletions.
50 changes: 1 addition & 49 deletions controllers/controllers/services/bindings/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
})
})
})
})
30 changes: 0 additions & 30 deletions controllers/controllers/services/bindings/upsi/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 0 additions & 48 deletions controllers/controllers/services/instances/upsi/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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")
}
Expand All @@ -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{}),
Expand Down
109 changes: 0 additions & 109 deletions controllers/controllers/services/instances/upsi/controller_test.go
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 99dbc5e

Please sign in to comment.