Skip to content

Commit

Permalink
Refactor binding controller
Browse files Browse the repository at this point in the history 
Introdce a sub-controller for user provided services that
has the responsibility to create the binding credentials secret

This refactoring makes it possible to support managed services in the
future by adding a managed services credentials sub-controller that
knows how to retrieve credentials from a broker
  • Loading branch information
@zabanov-lab @georgethebeatle
zabanov-lab authored and georgethebeatle committed Oct 2, 2024
1 parent 0d1b9cd commit 2dfdb38
Show file tree
Hide file tree
Showing 6 changed files with 235 additions and 199 deletions.
124 changes: 26 additions & 98 deletions controllers/controllers/services/bindings/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,14 @@ package bindings
import (
"context"
"fmt"
"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/k8s"

"github.com/go-logr/logr"
"github.com/pkg/errors"
servicebindingv1beta1 "github.com/servicebinding/runtime/apis/v1beta1"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -49,19 +45,25 @@ const (
ServiceBindingSecretTypePrefix = "servicebinding.io/"
)

type CredentialsReconciler interface {
ReconcileResource(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding) (ctrl.Result, error)
}

type Reconciler struct {
k8sClient client.Client
scheme *runtime.Scheme
log logr.Logger
k8sClient client.Client
scheme *runtime.Scheme
log logr.Logger
upsiCredentialsReconciler CredentialsReconciler
}

func NewReconciler(
k8sClient client.Client,
scheme *runtime.Scheme,
log logr.Logger,
upsiCredentialsReconciler CredentialsReconciler,
) *k8s.PatchingReconciler[korifiv1alpha1.CFServiceBinding, *korifiv1alpha1.CFServiceBinding] {
cfBindingReconciler := &Reconciler{k8sClient: k8sClient, scheme: scheme, log: log}
return k8s.NewPatchingReconciler[korifiv1alpha1.CFServiceBinding, *korifiv1alpha1.CFServiceBinding](log, k8sClient, cfBindingReconciler)
cfBindingReconciler := &Reconciler{k8sClient: k8sClient, scheme: scheme, log: log, upsiCredentialsReconciler: upsiCredentialsReconciler}
return k8s.NewPatchingReconciler(log, k8sClient, cfBindingReconciler)
}

func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder {
Expand Down Expand Up @@ -134,43 +136,25 @@ func (r *Reconciler) appToServiceBindings(ctx context.Context, o client.Object)
func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding) (ctrl.Result, error) {
log := logr.FromContextOrDiscard(ctx)

cfServiceBinding.Status.ObservedGeneration = cfServiceBinding.Generation
log.V(1).Info("set observed generation", "generation", cfServiceBinding.Status.ObservedGeneration)

cfServiceInstance := new(korifiv1alpha1.CFServiceInstance)
err := r.k8sClient.Get(ctx, types.NamespacedName{Name: cfServiceBinding.Spec.Service.Name, Namespace: cfServiceBinding.Namespace}, cfServiceInstance)
if err != nil {
log.Info("service instance not found", "service-instance", cfServiceBinding.Spec.Service.Name, "error", err)
return ctrl.Result{}, err
}

cfServiceBinding.Status.ObservedGeneration = cfServiceBinding.Generation
log.V(1).Info("set observed generation", "generation", cfServiceBinding.Status.ObservedGeneration)

err = controllerutil.SetOwnerReference(cfServiceInstance, cfServiceBinding, r.scheme)
if err != nil {
log.Info("error when making the service instance owner of the service binding", "reason", err)
return ctrl.Result{}, err
}

if cfServiceInstance.Status.Credentials.Name == "" {
return ctrl.Result{}, k8s.NewNotReadyError().
WithReason("CredentialsSecretNotAvailable").
WithMessage("Service instance credentials not available yet").
WithRequeueAfter(time.Second)
}

credentialsSecret, err := r.reconcileCredentials(ctx, cfServiceInstance, cfServiceBinding)
if err != nil {
if k8serrors.IsInvalid(err) {
err = r.k8sClient.Delete(ctx, &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cfServiceBinding.Name,
Namespace: cfServiceBinding.Namespace,
},
})
return ctrl.Result{Requeue: true}, errors.Wrap(err, "failed to delete outdated binding secret")
}

log.Error(err, "failed to reconcile credentials secret")
return ctrl.Result{}, err
res, err := r.upsiCredentialsReconciler.ReconcileResource(ctx, cfServiceBinding)
if needsRequeue(res) || err != nil {
return res, err
}

cfApp := new(korifiv1alpha1.CFApp)
Expand All @@ -180,7 +164,7 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBinding *ko
return ctrl.Result{}, err
}

sbServiceBinding, err := r.reconcileSBServiceBinding(ctx, cfServiceBinding, credentialsSecret)
sbServiceBinding, err := r.reconcileSBServiceBinding(ctx, cfServiceBinding)
if err != nil {
log.Info("error creating/updating servicebinding.io servicebinding", "reason", err)
return ctrl.Result{}, err
Expand All @@ -193,6 +177,10 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBinding *ko
return ctrl.Result{}, nil
}

func needsRequeue(res ctrl.Result) bool {
return !res.IsZero()
}

func isSbServiceBindingReady(sbServiceBinding *servicebindingv1beta1.ServiceBinding) bool {
readyCondition := meta.FindStatusCondition(sbServiceBinding.Status.Conditions, "Ready")
if readyCondition == nil {
Expand All @@ -206,82 +194,22 @@ func isSbServiceBindingReady(sbServiceBinding *servicebindingv1beta1.ServiceBind
return sbServiceBinding.Generation == sbServiceBinding.Status.ObservedGeneration
}

func (r *Reconciler) reconcileCredentials(ctx context.Context, cfServiceInstance *korifiv1alpha1.CFServiceInstance, cfServiceBinding *korifiv1alpha1.CFServiceBinding) (*corev1.Secret, 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 nil, err
}

return bindingSecret, nil
}

func (r *Reconciler) reconcileSBServiceBinding(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding) (*servicebindingv1beta1.ServiceBinding, error) {
credentialsSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: cfServiceInstance.Namespace,
Name: cfServiceInstance.Status.Credentials.Name,
},
}
err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)
if err != nil {
return nil, fmt.Errorf("failed to get service instance credentials secret %q: %w", cfServiceInstance.Status.Credentials.Name, err)
}

bindingSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cfServiceBinding.Name,
Namespace: cfServiceBinding.Namespace,
},
}

_, err = controllerutil.CreateOrPatch(ctx, r.k8sClient, bindingSecret, func() error {
bindingSecret.Type, err = credentials.GetBindingSecretType(credentialsSecret)
if err != nil {
return err
}
bindingSecret.Data, err = credentials.GetServiceBindingIOSecretData(credentialsSecret)
if err != nil {
return err
}

return controllerutil.SetControllerReference(cfServiceBinding, bindingSecret, r.scheme)
})
err := r.k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)
if err != nil {
return nil, errors.Wrap(err, "failed to create binding secret")
return nil, fmt.Errorf("failed to get service binding credentials secret %q: %w", cfServiceBinding.Status.Binding.Name, err)
}

cfServiceBinding.Status.Binding.Name = bindingSecret.Name

return bindingSecret, 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 *Reconciler) reconcileSBServiceBinding(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding, credentialsSecret *corev1.Secret) (*servicebindingv1beta1.ServiceBinding, error) {
sbServiceBinding := r.toSBServiceBinding(cfServiceBinding)

_, err := controllerutil.CreateOrPatch(ctx, r.k8sClient, sbServiceBinding, func() error {
_, err = controllerutil.CreateOrPatch(ctx, r.k8sClient, sbServiceBinding, func() error {
sbServiceBinding.Spec.Name = getSBServiceBindingName(cfServiceBinding)

secretType, hasType := credentialsSecret.Data["type"]
Expand Down
134 changes: 42 additions & 92 deletions controllers/controllers/services/bindings/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,21 @@ var _ = Describe("CFServiceBinding", func() {
}).Should(Succeed())
})

XWhen("the service instance is managed", func() {
BeforeEach(func() {
Expect(k8s.PatchResource(ctx, adminClient, instance, func() {
instance.Spec.Type = korifiv1alpha1.ManagedType
})).To(Succeed())
})

It("does not reconcile it", func() {
Consistently(func(g Gomega) {
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
g.Expect(binding.Status.ObservedGeneration).To(BeZero())
}).Should(Succeed())
})
})

When("the CFServiceBinding has a displayName set", func() {
BeforeEach(func() {
Expect(k8s.PatchResource(ctx, adminClient, binding, func() {
Expand All @@ -234,25 +249,30 @@ var _ = Describe("CFServiceBinding", func() {
})

When("the servicebinding.io binding is ready", func() {
var sbBinding *servicebindingv1beta1.ServiceBinding

BeforeEach(func() {
Eventually(func(g Gomega) {
sbServiceBinding := &servicebindingv1beta1.ServiceBinding{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: fmt.Sprintf("cf-binding-%s", binding.Name),
},
}
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed())
g.Expect(k8s.Patch(ctx, adminClient, sbServiceBinding, func() {
meta.SetStatusCondition(&sbServiceBinding.Status.Conditions, metav1.Condition{
Type: "Ready",
Status: metav1.ConditionTrue,
Reason: "whatever",
Message: "",
})
sbServiceBinding.Status.ObservedGeneration = sbServiceBinding.Generation
})).To(Succeed())
}).Should(Succeed())
sbBinding = &servicebindingv1beta1.ServiceBinding{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: fmt.Sprintf("cf-binding-%s", binding.Name),
},
}
Expect(adminClient.Create(ctx, sbBinding)).To(Succeed())

Expect(k8s.Patch(ctx, adminClient, sbBinding, func() {
meta.SetStatusCondition(&sbBinding.Status.Conditions, metav1.Condition{
Type: "Ready",
Status: metav1.ConditionTrue,
Reason: "whatever",
Message: "",
})

// Patching the object increments its generation. In order to
// ensure that the observed generation matches the generation,
// we set the observed generation to `generation + 1`
sbBinding.Status.ObservedGeneration = sbBinding.Generation + 1
})).To(Succeed())
})

It("sets the binding Ready status condition to true", func() {
Expand All @@ -267,25 +287,9 @@ var _ = Describe("CFServiceBinding", func() {

When("the servicebinding.io binding ready status is outdated", func() {
BeforeEach(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.ConditionTrue)),
)))
}).Should(Succeed())
Eventually(func(g Gomega) {
sbServiceBinding := &servicebindingv1beta1.ServiceBinding{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: fmt.Sprintf("cf-binding-%s", binding.Name),
},
}
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed())
g.Expect(k8s.Patch(ctx, adminClient, sbServiceBinding, func() {
sbServiceBinding.Status.ObservedGeneration = sbServiceBinding.Generation - 1
})).To(Succeed())
}).Should(Succeed())
Expect(k8s.Patch(ctx, adminClient, sbBinding, func() {
sbBinding.Status.ObservedGeneration = sbBinding.Generation - 1
})).To(Succeed())
})

It("sets the binding Ready status condition to false", func() {
Expand All @@ -309,61 +313,8 @@ var _ = Describe("CFServiceBinding", func() {
})
})

When("the servicebinding.io binding is not ready", func() {
BeforeEach(func() {
Eventually(func(g Gomega) {
sbServiceBinding := &servicebindingv1beta1.ServiceBinding{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: fmt.Sprintf("cf-binding-%s", binding.Name),
},
}
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed())
g.Expect(k8s.Patch(ctx, adminClient, sbServiceBinding, func() {
meta.SetStatusCondition(&sbServiceBinding.Status.Conditions, metav1.Condition{
Type: "Ready",
Status: metav1.ConditionFalse,
Reason: "whatever",
Message: "",
})
})).To(Succeed())
}).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)),
HasReason(Equal("ServiceBindingNotReady")),
)))
}).Should(Succeed())
})
})

When("the credentials secret has its 'type' attribute set", func() {
BeforeEach(func() {
Eventually(func(g Gomega) {
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
bindingSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: binding.Namespace,
Name: binding.Status.Binding.Name,
},
}
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(bindingSecret), bindingSecret)).To(Succeed())
g.Expect(bindingSecret.Type).To(BeEquivalentTo("servicebinding.io/user-provided"))

sbServiceBinding := &servicebindingv1beta1.ServiceBinding{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: fmt.Sprintf("cf-binding-%s", binding.Name),
},
}
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed())
}).Should(Succeed())

credentialsBytes, err := json.Marshal(map[string]any{
"type": "my-type",
})
Expand All @@ -386,7 +337,7 @@ var _ = Describe("CFServiceBinding", func() {
})).To(Succeed())
})

It("sets the specified type as secret type", func() {
It("sets the specified type as servicebinding.io secret type", func() {
Eventually(func(g Gomega) {
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
bindingSecret := &corev1.Secret{
Expand All @@ -412,7 +363,6 @@ var _ = Describe("CFServiceBinding", func() {
},
}
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(sbServiceBinding), sbServiceBinding)).To(Succeed())
fmt.Fprintf(GinkgoWriter, "servicebinding.io: %+v\n", sbServiceBinding)
g.Expect(sbServiceBinding.Spec.Type).To(Equal("my-type"))
}).Should(Succeed())
})
Expand Down
Loading

0 comments on commit 2dfdb38

Please sign in to comment.