diff --git a/controllers/api/v1alpha1/cfservicebinding_types.go b/controllers/api/v1alpha1/cfservicebinding_types.go index f95858069..c8b692f24 100644 --- a/controllers/api/v1alpha1/cfservicebinding_types.go +++ b/controllers/api/v1alpha1/cfservicebinding_types.go @@ -24,9 +24,9 @@ import ( ) const ( - BindingFailedCondition = "BindingFailed" - BindingRequestedCondition = "BindingRequested" - UnbindingRequestedCondition = "UnbindingRequested" + BindingFailedCondition = "BindingFailed" + + UnbindingFailedCondition = "UnbindingFailed" ServiceInstanceTypeAnnotationKey = "korifi.cloudfoundry.org/service-instance-type" PlanGUIDLabelKey = "korifi.cloudfoundry.org/plan-guid" @@ -57,20 +57,6 @@ type CFServiceBindingStatus struct { // +optional Binding v1.LocalObjectReference `json:"binding"` - // The - // [operation](https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#binding) - // of the bind request to the the OSBAPI broker. Only makes sense for - // bindings to managed service instances - // +optional - BindingOperation string `json:"bindingOperation"` - - // The - // [operation](https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#unbinding) - // of the unbind request to the the OSBAPI broker. Only makes sense for - // bindings to managed service instances - // +optional - UnbindingOperation string `json:"unbindingOperation"` - // A reference to the Secret containing the binding Credentials object. For // bindings to user-provided services this refers to the credentials secret // from the service instance. For managed services the secret contains the diff --git a/controllers/controllers/services/bindings/controller.go b/controllers/controllers/services/bindings/controller.go index b1013044b..6ed4c96c1 100644 --- a/controllers/controllers/services/bindings/controller.go +++ b/controllers/controllers/services/bindings/controller.go @@ -23,6 +23,7 @@ import ( "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/tools" "code.cloudfoundry.org/korifi/tools/k8s" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/go-logr/logr" servicebindingv1beta1 "github.com/servicebinding/runtime/apis/v1beta1" @@ -125,7 +126,14 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBinding *ko cfServiceBinding.Annotations = tools.SetMapValue(cfServiceBinding.Annotations, korifiv1alpha1.ServiceInstanceTypeAnnotationKey, string(cfServiceInstance.Spec.Type)) - err = controllerutil.SetOwnerReference(cfServiceInstance, cfServiceBinding, r.scheme) + if err = k8s.Patch(ctx, r.k8sClient, cfServiceInstance, func() { + controllerutil.AddFinalizer(cfServiceInstance, metav1.FinalizerDeleteDependents) + }); err != nil { + log.Info("error when setting the foreground deletion finalizer on the service instance", "reason", err) + return ctrl.Result{}, err + } + + err = controllerutil.SetOwnerReference(cfServiceInstance, cfServiceBinding, r.scheme, controllerutil.WithBlockOwnerDeletion(true)) if err != nil { log.Info("error when making the service instance owner of the service binding", "reason", err) return ctrl.Result{}, err diff --git a/controllers/controllers/services/bindings/controller_test.go b/controllers/controllers/services/bindings/controller_test.go index c258b486c..ae8772074 100644 --- a/controllers/controllers/services/bindings/controller_test.go +++ b/controllers/controllers/services/bindings/controller_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" "code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi" @@ -67,7 +68,7 @@ var _ = Describe("CFServiceBinding", func() { Expect(adminClient.Create(ctx, binding)).To(Succeed()) }) - Describe("user-provided instances", func() { + Describe("user-provided bindings", func() { var ( instance *korifiv1alpha1.CFServiceInstance instanceCredentialsSecret *corev1.Secret @@ -116,11 +117,19 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) + It("sets the foregroundDeletion finalizer on the service instance", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + g.Expect(instance.Finalizers).To(ContainElement(metav1.FinalizerDeleteDependents)) + }).Should(Succeed()) + }) + It("sets an owner reference from the instance to the binding", func() { Eventually(func(g Gomega) { g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) g.Expect(binding.OwnerReferences).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ - "Name": Equal(instance.Name), + "Name": Equal(instance.Name), + "BlockOwnerDeletion": PointTo(BeTrue()), }))) }).Should(Succeed()) }) @@ -475,7 +484,7 @@ var _ = Describe("CFServiceBinding", func() { }) }) - Describe("managed service instances", func() { + Describe("managed service bindings", func() { var ( brokerClient *fake.BrokerClient @@ -549,7 +558,6 @@ var _ = Describe("CFServiceBinding", func() { Credentials: map[string]any{ "foo": "bar", }, - Complete: true, }, nil) instance = &korifiv1alpha1.CFServiceInstance{ @@ -677,7 +685,6 @@ var _ = Describe("CFServiceBinding", func() { "foo": "bar", "type": "please-ignore-me", }, - Complete: true, }, nil) }) @@ -704,7 +711,6 @@ var _ = Describe("CFServiceBinding", func() { "foo": "bar", "provider": "please-ignore-me", }, - Complete: true, }, nil) }) @@ -788,7 +794,7 @@ var _ = Describe("CFServiceBinding", func() { BeforeEach(func() { brokerClient.BindReturns(osbapi.BindResponse{ Operation: "operation-1", - Complete: false, + IsAsync: true, }, nil) brokerClient.GetServiceBindingLastOperationReturns(osbapi.LastOperationResponse{ @@ -807,29 +813,11 @@ var _ = Describe("CFServiceBinding", func() { }).Should(Succeed()) }) - It("sets the BindRequested condition", 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.BindingRequestedCondition)), - HasStatus(Equal(metav1.ConditionTrue)), - ))) - }).Should(Succeed()) - - Consistently(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.BindingRequestedCondition)), - HasStatus(Equal(metav1.ConditionTrue)), - ))) - }).Should(Succeed()) - }) - It("keeps checking last operation", func() { Eventually(func(g Gomega) { g.Expect(brokerClient.GetServiceBindingLastOperationCallCount()).To(BeNumerically(">", 1)) _, actualLastOpPayload := brokerClient.GetServiceBindingLastOperationArgsForCall(1) - g.Expect(actualLastOpPayload).To(Equal(osbapi.GetServiceBindingLastOperationRequest{ + g.Expect(actualLastOpPayload).To(Equal(osbapi.GetBindingLastOperationRequest{ InstanceID: instance.Name, BindingID: binding.Name, GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ @@ -892,7 +880,7 @@ var _ = Describe("CFServiceBinding", func() { State: "succeeded", }, nil) - brokerClient.GetServiceBindingReturns(osbapi.GetBindingResponse{ + brokerClient.BindReturns(osbapi.BindResponse{ Credentials: map[string]any{ "foo": "bar", }, @@ -943,68 +931,246 @@ var _ = Describe("CFServiceBinding", func() { }))) }).Should(Succeed()) }) + }) + }) - When("getting the binding fails", func() { - BeforeEach(func() { - brokerClient.GetServiceBindingReturns(osbapi.GetBindingResponse{}, errors.New("get-binding-err")) - }) + Describe("bind failures", func() { + When("bind fails with recoverable error", func() { + BeforeEach(func() { + brokerClient.BindReturns(osbapi.BindResponse{}, errors.New("binding-failed")) + }) - It("sets the ready condition to false", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + It("keeps trying to bind", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.BindCallCount()).To(BeNumerically(">", 1)) + _, payload := brokerClient.BindArgsForCall(1) + g.Expect(payload).To(Equal(osbapi.BindPayload{ + InstanceID: instance.Name, + BindingID: binding.Name, + BindRequest: osbapi.BindRequest{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id", + AppGUID: cfAppGUID, + BindResource: osbapi.BindResource{ + AppGUID: cfAppGUID, + }, + }, + })) + }).Should(Succeed()) + }) - g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + It("sets ready to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElements( + SatisfyAll( HasType(Equal(korifiv1alpha1.StatusConditionReady)), HasStatus(Equal(metav1.ConditionFalse)), - HasMessage(ContainSubstring("get-binding-err")), - ))) - }).Should(Succeed()) - }) + ), + )) + }).Should(Succeed()) }) }) - }) - When("binding fails with the broker", func() { - BeforeEach(func() { - brokerClient.BindReturns(osbapi.BindResponse{}, errors.New("binding-failed")) - }) + When("bind fails with unrecoverable error", func() { + BeforeEach(func() { + brokerClient.BindReturns(osbapi.BindResponse{}, osbapi.UnrecoverableError{Status: http.StatusConflict}) + }) - It("fails the binding", func() { - Eventually(func(g Gomega) { - g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) - g.Expect(binding.Status.Conditions).To(ContainElements( - SatisfyAll( - HasType(Equal(korifiv1alpha1.StatusConditionReady)), - HasStatus(Equal(metav1.ConditionFalse)), - ), - SatisfyAll( - HasType(Equal(korifiv1alpha1.BindingFailedCondition)), - HasStatus(Equal(metav1.ConditionTrue)), - HasReason(Equal("BindingFailed")), - HasMessage(ContainSubstring("binding-failed")), - ), - )) - }).Should(Succeed()) + It("fails the binding", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(binding.Status.Conditions).To(ContainElements( + SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ), + SatisfyAll( + HasType(Equal(korifiv1alpha1.BindingFailedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + HasReason(Equal("BindingFailed")), + ), + )) + }).Should(Succeed()) + }) }) }) - When("the binding is deleted", func() { + Describe("unbind", func() { BeforeEach(func() { brokerClient.UnbindReturns(osbapi.UnbindResponse{}, nil) }) JustBeforeEach(func() { - // For deletion test we want to request deletion and verify the behaviour when finalization fails. - // Therefore we use the standard k8s client instead of `adminClient` as it ensures that the object is deleted Expect(k8sManager.GetClient().Delete(ctx, binding)).To(Succeed()) }) + It("unbinds the binding with the broker", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.UnbindCallCount()).To(Equal(1)) + _, actualUnbindRequest := brokerClient.UnbindArgsForCall(0) + Expect(actualUnbindRequest).To(Equal(osbapi.UnbindPayload{ + BindingID: binding.Name, + InstanceID: instance.Name, + UnbindRequestParameters: osbapi.UnbindRequestParameters{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id", + }, + })) + }).Should(Succeed()) + }) + It("deletes the binding", func() { Eventually(func(g Gomega) { err := adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding) g.Expect(k8serrors.IsNotFound(err)).To(BeTrue()) }).Should(Succeed()) }) + + When("unbind fails with recoverable error", func() { + BeforeEach(func() { + brokerClient.UnbindReturns(osbapi.UnbindResponse{}, errors.New("unbinding-failed")) + }) + + It("does not delete the binding", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + }).Should(Succeed()) + }) + + It("keeps trying to unbind with the broker", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + g.Expect(brokerClient.UnbindCallCount()).To(BeNumerically(">", 1)) + }).Should(Succeed()) + }) + }) + + When("unbind fails with unrecoverable error", func() { + BeforeEach(func() { + brokerClient.UnbindReturns(osbapi.UnbindResponse{}, osbapi.UnrecoverableError{Status: http.StatusGone}) + }) + + It("fails the binding", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + + g.Expect(binding.Status.Conditions).To(ContainElements( + SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + ), + SatisfyAll( + HasType(Equal(korifiv1alpha1.UnbindingFailedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + HasReason(Equal("UnbindingFailed")), + HasMessage(ContainSubstring("The server responded with status: 410")), + ), + )) + }).Should(Succeed()) + }) + }) + + When("the unbind is asynchronous", func() { + BeforeEach(func() { + brokerClient.UnbindReturns(osbapi.UnbindResponse{ + IsAsync: true, + Operation: "unbind-op", + }, nil) + }) + + It("does not delete the binding", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed()) + }).Should(Succeed()) + }) + + When("the last operation is in progress", func() { + BeforeEach(func() { + brokerClient.GetServiceBindingLastOperationReturns(osbapi.LastOperationResponse{ + State: "in progress", + }, nil) + }) + + It("sets the ready 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("UnbindingInProgress")), + ))) + }).Should(Succeed()) + }) + + It("keeps checking last operation", func() { + Eventually(func(g Gomega) { + g.Expect(brokerClient.GetServiceBindingLastOperationCallCount()).To(BeNumerically(">", 1)) + _, actualLastOpPayload := brokerClient.GetServiceBindingLastOperationArgsForCall(1) + g.Expect(actualLastOpPayload).To(Equal(osbapi.GetBindingLastOperationRequest{ + InstanceID: instance.Name, + BindingID: binding.Name, + GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ + ServiceId: "service-offering-id", + PlanID: "service-plan-id", + Operation: "unbind-op", + }, + })) + }).Should(Succeed()) + }) + }) + + When("the last operation is failed", func() { + BeforeEach(func() { + brokerClient.GetServiceBindingLastOperationReturns(osbapi.LastOperationResponse{ + State: "failed", + }, nil) + }) + + It("sets the failed condition", 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)), + ))) + + g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.UnbindingFailedCondition)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + }) + + When("the broker unbind succeeds", func() { + BeforeEach(func() { + brokerClient.UnbindReturns(osbapi.UnbindResponse{}, nil) + }) + + It("deletes the binding", func() { + Eventually(func(g Gomega) { + err := adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding) + g.Expect(k8serrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + }) + }) + + When("the binding is gone", func() { + BeforeEach(func() { + brokerClient.UnbindReturns(osbapi.UnbindResponse{}, osbapi.GoneError{}) + }) + + It("deletes the binding", func() { + Eventually(func(g Gomega) { + err := adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding) + g.Expect(k8serrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + }) + }) + }) }) }) }) diff --git a/controllers/controllers/services/bindings/managed/controller.go b/controllers/controllers/services/bindings/managed/controller.go index 0c0763813..1300dec35 100644 --- a/controllers/controllers/services/bindings/managed/controller.go +++ b/controllers/controllers/services/bindings/managed/controller.go @@ -2,6 +2,7 @@ package managed import ( "context" + "fmt" "time" "code.cloudfoundry.org/korifi/controllers/controllers/services/bindings/sbio" @@ -39,15 +40,9 @@ func NewReconciler(k8sClient client.Client, brokerClientFactory osbapi.BrokerCli } } -func (r *ManagedBindingsReconciler) ReconcileResource(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding, cfServiceInstance *korifiv1alpha1.CFServiceInstance) (ctrl.Result, error) { +func (r *ManagedBindingsReconciler) ReconcileResource(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding, _ *korifiv1alpha1.CFServiceInstance) (ctrl.Result, error) { log := logr.FromContextOrDiscard(ctx).WithName("reconcile-managed-service-binding") - if !cfServiceBinding.GetDeletionTimestamp().IsZero() { - return r.finalizeCFServiceBinding(ctx, cfServiceBinding) - } - - cfServiceBinding.Labels = tools.SetMapValue(cfServiceBinding.Labels, korifiv1alpha1.PlanGUIDLabelKey, cfServiceInstance.Spec.PlanGUID) - assets, err := r.assets.GetServiceBindingAssets(ctx, cfServiceBinding) if err != nil { log.Error(err, "failed to get service binding assets") @@ -60,6 +55,10 @@ func (r *ManagedBindingsReconciler) ReconcileResource(ctx context.Context, cfSer return ctrl.Result{}, err } + if !cfServiceBinding.GetDeletionTimestamp().IsZero() { + return r.finalizeCFServiceBinding(ctx, cfServiceBinding, assets, osbapiClient) + } + if isReconciled(cfServiceBinding) { return ctrl.Result{}, nil } @@ -68,12 +67,24 @@ func (r *ManagedBindingsReconciler) ReconcileResource(ctx context.Context, cfSer return ctrl.Result{}, k8s.NewNotReadyError().WithReason("BindingFailed").WithNoRequeue() } - credentials, err := r.bind(ctx, cfServiceBinding, assets, osbapiClient) + cfServiceBinding.Labels = tools.SetMapValue(cfServiceBinding.Labels, korifiv1alpha1.PlanGUIDLabelKey, assets.ServicePlan.Name) + + bindResponse, err := r.bind(ctx, cfServiceBinding, assets, osbapiClient) if err != nil { return ctrl.Result{}, err } - err = r.reconcileCredentials(ctx, cfServiceBinding, credentials) + if bindResponse.IsAsync { + var lastOpResponse osbapi.LastOperationResponse + lastOpResponse, err = r.pollLastOperation(ctx, cfServiceBinding, assets, osbapiClient, bindResponse.Operation) + if err != nil { + return ctrl.Result{}, err + } + + return r.processBindOperation(cfServiceBinding, lastOpResponse) + } + + err = r.reconcileCredentials(ctx, cfServiceBinding, bindResponse.Credentials) if err != nil { return ctrl.Result{}, err } @@ -96,20 +107,7 @@ func (r *ManagedBindingsReconciler) bind( cfServiceBinding *korifiv1alpha1.CFServiceBinding, assets osbapi.ServiceBindingAssets, osbapiClient osbapi.BrokerClient, -) (map[string]any, error) { - if !isBindRequested(cfServiceBinding) { - return r.requestBind(ctx, cfServiceBinding, assets, osbapiClient) - } - - return r.pollBindOperation(ctx, cfServiceBinding, assets, osbapiClient) -} - -func (r *ManagedBindingsReconciler) requestBind( - ctx context.Context, - cfServiceBinding *korifiv1alpha1.CFServiceBinding, - assets osbapi.ServiceBindingAssets, - osbapiClient osbapi.BrokerClient, -) (map[string]any, error) { +) (osbapi.BindResponse, error) { log := logr.FromContextOrDiscard(ctx) bindResponse, err := osbapiClient.Bind(ctx, osbapi.BindPayload{ @@ -125,64 +123,35 @@ func (r *ManagedBindingsReconciler) requestBind( }, }) if err != nil { - log.Error(err, "failed to bind service") - - meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.BindingFailedCondition, - Status: metav1.ConditionTrue, - ObservedGeneration: cfServiceBinding.Generation, - LastTransitionTime: metav1.NewTime(time.Now()), - Reason: "BindingFailed", - Message: err.Error(), - }) - - return nil, k8s.NewNotReadyError().WithReason("BindingFailed") - } - - cfServiceBinding.Status.BindingOperation = bindResponse.Operation - meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.BindingRequestedCondition, - Status: metav1.ConditionTrue, - ObservedGeneration: cfServiceBinding.Generation, - LastTransitionTime: metav1.NewTime(time.Now()), - Reason: "BindingRequested", - }) + log.Error(err, "failed to bind") + + if osbapi.IsUnrecoveralbeError(err) { + meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.BindingFailedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: cfServiceBinding.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "BindingFailed", + Message: err.Error(), + }) + return osbapi.BindResponse{}, k8s.NewNotReadyError().WithReason("BindingFailed") + } - if bindResponse.Complete { - return bindResponse.Credentials, nil + return osbapi.BindResponse{}, err } - return nil, k8s.NewNotReadyError().WithReason("BindingInProgress").WithRequeue() + return bindResponse, nil } -func (r *ManagedBindingsReconciler) pollBindOperation( - ctx context.Context, +func (r *ManagedBindingsReconciler) processBindOperation( cfServiceBinding *korifiv1alpha1.CFServiceBinding, - assets osbapi.ServiceBindingAssets, - osbapiClient osbapi.BrokerClient, -) (map[string]any, error) { - log := logr.FromContextOrDiscard(ctx) - - lastOperation, err := osbapiClient.GetServiceBindingLastOperation(ctx, osbapi.GetServiceBindingLastOperationRequest{ - InstanceID: cfServiceBinding.Spec.Service.Name, - BindingID: cfServiceBinding.Name, - GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ - ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, - PlanID: assets.ServicePlan.Spec.BrokerCatalog.ID, - Operation: cfServiceBinding.Status.BindingOperation, - }, - }) - if err != nil { - log.Error(err, "failed to get last operation", "operation", cfServiceBinding.Status.BindingOperation) - return nil, k8s.NewNotReadyError().WithCause(err).WithReason("GetLastOperationFailed") - } + lastOperation osbapi.LastOperationResponse, +) (ctrl.Result, error) { if lastOperation.State == "in progress" { - log.Info("binding operation in progress", "operation", cfServiceBinding.Status.BindingOperation) - return nil, k8s.NewNotReadyError().WithReason("BindingInProgress").WithRequeue() + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("BindingInProgress").WithRequeue() } if lastOperation.State == "failed" { - log.Error(nil, "last operation has failed", "operation", cfServiceBinding.Status.BindingOperation, "description", lastOperation.Description) meta.SetStatusCondition(&cfServiceBinding.Status.Conditions, metav1.Condition{ Type: korifiv1alpha1.BindingFailedCondition, Status: metav1.ConditionTrue, @@ -191,21 +160,10 @@ func (r *ManagedBindingsReconciler) pollBindOperation( Reason: "BindingFailed", Message: lastOperation.Description, }) - return nil, k8s.NewNotReadyError().WithReason("BindingFailed") - } - - binding, err := osbapiClient.GetServiceBinding(ctx, osbapi.GetServiceBindingRequest{ - InstanceID: cfServiceBinding.Spec.Service.Name, - BindingID: cfServiceBinding.Name, - ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, - PlanID: assets.ServicePlan.Spec.BrokerCatalog.ID, - }) - if err != nil { - log.Error(err, "failed to get binding") - return nil, err + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("BindingFailed") } - return binding.Credentials, nil + return ctrl.Result{}, nil } func (r *ManagedBindingsReconciler) reconcileCredentials(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding, creds map[string]any) error { @@ -259,12 +217,28 @@ func (r *ManagedBindingsReconciler) reconcileCredentials(ctx context.Context, cf func (r *ManagedBindingsReconciler) finalizeCFServiceBinding( ctx context.Context, serviceBinding *korifiv1alpha1.CFServiceBinding, + assets osbapi.ServiceBindingAssets, + osbapiClient osbapi.BrokerClient, ) (ctrl.Result, error) { log := logr.FromContextOrDiscard(ctx).WithName("finalize-managed-service-binding") + unbindResponse, err := r.deleteServiceBinding(ctx, serviceBinding, assets, osbapiClient) + if err != nil { + return ctrl.Result{}, err + } + if unbindResponse.IsAsync { + lastOpresponse, err := r.pollLastOperation(ctx, serviceBinding, assets, osbapiClient, unbindResponse.Operation) + if err != nil { + return ctrl.Result{}, err + } + + return r.processUnbindLastOperation(serviceBinding, lastOpresponse) + } + if controllerutil.RemoveFinalizer(serviceBinding, korifiv1alpha1.CFServiceBindingFinalizerName) { log.V(1).Info("finalizer removed") } + return ctrl.Result{}, nil } @@ -281,8 +255,86 @@ func (r *ManagedBindingsReconciler) reconcileSBServiceBinding(ctx context.Contex return sbServiceBinding, nil } -func isBindRequested(binding *korifiv1alpha1.CFServiceBinding) bool { - return meta.IsStatusConditionTrue(binding.Status.Conditions, korifiv1alpha1.BindingRequestedCondition) +func (r *ManagedBindingsReconciler) pollLastOperation( + ctx context.Context, + serviceBinding *korifiv1alpha1.CFServiceBinding, + assets osbapi.ServiceBindingAssets, + osbapiClient osbapi.BrokerClient, + operationID string, +) (osbapi.LastOperationResponse, error) { + log := logr.FromContextOrDiscard(ctx).WithName("poll-operation") + + lastOpResponse, err := osbapiClient.GetServiceBindingLastOperation(ctx, osbapi.GetBindingLastOperationRequest{ + InstanceID: serviceBinding.Spec.Service.Name, + BindingID: serviceBinding.Name, + GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ + ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, + PlanID: assets.ServicePlan.Spec.BrokerCatalog.ID, + Operation: operationID, + }, + }) + if err != nil { + log.Error(err, "getting service binding last operation failed") + return osbapi.LastOperationResponse{}, k8s.NewNotReadyError().WithCause(err).WithReason("GetLastOperationFailed") + + } + return lastOpResponse, nil +} + +func (r *ManagedBindingsReconciler) processUnbindLastOperation( + serviceBinding *korifiv1alpha1.CFServiceBinding, + lastOpResponse osbapi.LastOperationResponse, +) (ctrl.Result, error) { + if lastOpResponse.State == "in progress" { + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("UnbindingInProgress").WithRequeue() + } + if lastOpResponse.State == "failed" { + meta.SetStatusCondition(&serviceBinding.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.UnbindingFailedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: serviceBinding.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "UnbindingFailed", + Message: lastOpResponse.Description, + }) + return ctrl.Result{}, k8s.NewNotReadyError().WithReason("UnbindFailed") + + } + + return ctrl.Result{Requeue: true}, nil +} + +func (r *ManagedBindingsReconciler) deleteServiceBinding( + ctx context.Context, + serviceBinding *korifiv1alpha1.CFServiceBinding, + assets osbapi.ServiceBindingAssets, + osbapiClient osbapi.BrokerClient, +) (osbapi.UnbindResponse, error) { + unbindResponse, err := osbapiClient.Unbind(ctx, osbapi.UnbindPayload{ + InstanceID: serviceBinding.Spec.Service.Name, + BindingID: serviceBinding.Name, + UnbindRequestParameters: osbapi.UnbindRequestParameters{ + ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, + PlanID: assets.ServicePlan.Spec.BrokerCatalog.ID, + }, + }) + if osbapi.IgnoreGone(err) != nil { + if osbapi.IsUnrecoveralbeError(err) { + meta.SetStatusCondition(&serviceBinding.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.UnbindingFailedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: serviceBinding.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "UnbindingFailed", + Message: err.Error(), + }) + return osbapi.UnbindResponse{}, k8s.NewNotReadyError().WithReason("UnbindingFailed") + } + + return osbapi.UnbindResponse{}, fmt.Errorf("failed to unbind: %w", err) + } + + return unbindResponse, nil } func isFailed(binding *korifiv1alpha1.CFServiceBinding) bool { diff --git a/controllers/controllers/services/instances/managed/controller.go b/controllers/controllers/services/instances/managed/controller.go index db5e91a8f..a97860cc7 100644 --- a/controllers/controllers/services/instances/managed/controller.go +++ b/controllers/controllers/services/instances/managed/controller.go @@ -188,20 +188,20 @@ func (r *Reconciler) provisionServiceInstance( serviceInstance *korifiv1alpha1.CFServiceInstance, assets osbapi.ServiceInstanceAssets, osbapiClient osbapi.BrokerClient, -) (osbapi.ServiceInstanceOperationResponse, error) { +) (osbapi.ProvisionResponse, error) { log := logr.FromContextOrDiscard(ctx).WithName("provision-service-instance") parametersMap, err := getServiceInstanceParameters(serviceInstance) if err != nil { log.Error(err, "failed to get service instance parameters") - return osbapi.ServiceInstanceOperationResponse{}, + return osbapi.ProvisionResponse{}, fmt.Errorf("failed to get service instance parameters: %w", err) } namespace, err := r.getNamespace(ctx, serviceInstance.Namespace) if err != nil { log.Error(err, "failed to get namespace") - return osbapi.ServiceInstanceOperationResponse{}, err + return osbapi.ProvisionResponse{}, err } serviceInstance.Status.LastOperation = services.LastOperation{ @@ -209,10 +209,10 @@ func (r *Reconciler) provisionServiceInstance( State: "initial", } - var provisionResponse osbapi.ServiceInstanceOperationResponse - provisionResponse, err = osbapiClient.Provision(ctx, osbapi.InstanceProvisionPayload{ + var provisionResponse osbapi.ProvisionResponse + provisionResponse, err = osbapiClient.Provision(ctx, osbapi.ProvisionPayload{ InstanceID: serviceInstance.Name, - InstanceProvisionRequest: osbapi.InstanceProvisionRequest{ + ProvisionRequest: osbapi.ProvisionRequest{ ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, PlanID: assets.ServicePlan.Spec.BrokerCatalog.ID, SpaceGUID: namespace.Labels[korifiv1alpha1.SpaceGUIDKey], @@ -233,11 +233,11 @@ func (r *Reconciler) provisionServiceInstance( Reason: "ProvisionFailed", Message: err.Error(), }) - return osbapi.ServiceInstanceOperationResponse{}, + return osbapi.ProvisionResponse{}, k8s.NewNotReadyError().WithReason("ProvisionFailed") } - return osbapi.ServiceInstanceOperationResponse{}, err + return osbapi.ProvisionResponse{}, err } return provisionResponse, nil @@ -304,19 +304,19 @@ func (r *Reconciler) deprovisionServiceInstance( serviceInstance *korifiv1alpha1.CFServiceInstance, assets osbapi.ServiceInstanceAssets, osbapiClient osbapi.BrokerClient, -) (osbapi.ServiceInstanceOperationResponse, error) { +) (osbapi.ProvisionResponse, error) { serviceInstance.Status.LastOperation = services.LastOperation{ Type: "delete", State: "initial", } - deprovisionResponse, err := osbapiClient.Deprovision(ctx, osbapi.InstanceDeprovisionPayload{ + deprovisionResponse, err := osbapiClient.Deprovision(ctx, osbapi.DeprovisionPayload{ ID: serviceInstance.Name, - InstanceDeprovisionRequest: osbapi.InstanceDeprovisionRequest{ + DeprovisionRequest: osbapi.DeprovisionRequest{ ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, PlanID: assets.ServicePlan.Spec.BrokerCatalog.ID, }, }) - if err != nil { + if osbapi.IgnoreGone(err) != nil { if osbapi.IsUnrecoveralbeError(err) { serviceInstance.Status.LastOperation.State = "failed" meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ @@ -327,11 +327,11 @@ func (r *Reconciler) deprovisionServiceInstance( Reason: "DeprovisionFailed", Message: err.Error(), }) - return osbapi.ServiceInstanceOperationResponse{}, + return osbapi.ProvisionResponse{}, k8s.NewNotReadyError().WithReason("DeprovisionFailed") } - return osbapi.ServiceInstanceOperationResponse{}, fmt.Errorf("failed to deprovision service instance: %w", err) + return osbapi.ProvisionResponse{}, fmt.Errorf("failed to deprovision service instance: %w", err) } return deprovisionResponse, nil @@ -368,7 +368,7 @@ func (r *Reconciler) pollLastOperation( operationID string, ) (osbapi.LastOperationResponse, error) { log := logr.FromContextOrDiscard(ctx).WithName("poll-operation") - lastOpResponse, err := osbapiClient.GetServiceInstanceLastOperation(ctx, osbapi.GetServiceInstanceLastOperationRequest{ + lastOpResponse, err := osbapiClient.GetServiceInstanceLastOperation(ctx, osbapi.GetInstanceLastOperationRequest{ InstanceID: serviceInstance.Name, GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ ServiceId: assets.ServiceOffering.Spec.BrokerCatalog.ID, diff --git a/controllers/controllers/services/instances/managed/controller_test.go b/controllers/controllers/services/instances/managed/controller_test.go index ef7e83564..9f71d1676 100644 --- a/controllers/controllers/services/instances/managed/controller_test.go +++ b/controllers/controllers/services/instances/managed/controller_test.go @@ -38,7 +38,7 @@ var _ = Describe("CFServiceInstance", func() { brokerClient = new(fake.BrokerClient) brokerClientFactory.CreateClientReturns(brokerClient, nil) - brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{ + brokerClient.ProvisionReturns(osbapi.ProvisionResponse{ IsAsync: true, Operation: "operation-1", }, nil) @@ -185,9 +185,9 @@ var _ = Describe("CFServiceInstance", func() { Eventually(func(g Gomega) { g.Expect(brokerClient.ProvisionCallCount()).NotTo(BeZero()) _, payload := brokerClient.ProvisionArgsForCall(0) - g.Expect(payload).To(Equal(osbapi.InstanceProvisionPayload{ + g.Expect(payload).To(Equal(osbapi.ProvisionPayload{ InstanceID: instance.Name, - InstanceProvisionRequest: osbapi.InstanceProvisionRequest{ + ProvisionRequest: osbapi.ProvisionRequest{ ServiceId: "service-offering-id", PlanID: "service-plan-id", SpaceGUID: "space-guid", @@ -200,7 +200,7 @@ var _ = Describe("CFServiceInstance", func() { g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(BeNumerically(">", 0)) _, lastOp := brokerClient.GetServiceInstanceLastOperationArgsForCall(brokerClient.GetServiceInstanceLastOperationCallCount() - 1) - g.Expect(lastOp).To(Equal(osbapi.GetServiceInstanceLastOperationRequest{ + g.Expect(lastOp).To(Equal(osbapi.GetInstanceLastOperationRequest{ InstanceID: instance.Name, GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ ServiceId: "service-offering-id", @@ -298,7 +298,7 @@ var _ = Describe("CFServiceInstance", func() { When("the provisioning is synchronous", func() { BeforeEach(func() { - brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{}, nil) + brokerClient.ProvisionReturns(osbapi.ProvisionResponse{}, nil) }) It("does not check last operation", func() { @@ -321,16 +321,16 @@ var _ = Describe("CFServiceInstance", func() { When("service provisioning fails with recoverable error", func() { BeforeEach(func() { - brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{}, errors.New("provision-failed")) + brokerClient.ProvisionReturns(osbapi.ProvisionResponse{}, errors.New("provision-failed")) }) It("keeps trying to provision the instance", func() { Eventually(func(g Gomega) { g.Expect(brokerClient.ProvisionCallCount()).To(BeNumerically(">", 1)) _, provisionPayload := brokerClient.ProvisionArgsForCall(1) - g.Expect(provisionPayload).To(Equal(osbapi.InstanceProvisionPayload{ + g.Expect(provisionPayload).To(Equal(osbapi.ProvisionPayload{ InstanceID: instance.Name, - InstanceProvisionRequest: osbapi.InstanceProvisionRequest{ + ProvisionRequest: osbapi.ProvisionRequest{ ServiceId: "service-offering-id", PlanID: "service-plan-id", SpaceGUID: "space-guid", @@ -357,7 +357,7 @@ var _ = Describe("CFServiceInstance", func() { When("service provisioning fails with unrecoverable error", func() { BeforeEach(func() { - brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{}, osbapi.UnrecoverableError{Status: http.StatusBadRequest}) + brokerClient.ProvisionReturns(osbapi.ProvisionResponse{}, osbapi.UnrecoverableError{Status: http.StatusBadRequest}) }) It("fails the instance", func() { @@ -441,7 +441,7 @@ var _ = Describe("CFServiceInstance", func() { Eventually(func(g Gomega) { g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(BeNumerically(">", 1)) _, actualLastOpPayload := brokerClient.GetServiceInstanceLastOperationArgsForCall(1) - g.Expect(actualLastOpPayload).To(Equal(osbapi.GetServiceInstanceLastOperationRequest{ + g.Expect(actualLastOpPayload).To(Equal(osbapi.GetInstanceLastOperationRequest{ InstanceID: instance.Name, GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ ServiceId: "service-offering-id", @@ -730,7 +730,7 @@ var _ = Describe("CFServiceInstance", func() { Describe("instance deletion", func() { BeforeEach(func() { - brokerClient.DeprovisionReturns(osbapi.ServiceInstanceOperationResponse{ + brokerClient.DeprovisionReturns(osbapi.ProvisionResponse{ Operation: "deprovision-op", }, nil) }) @@ -748,9 +748,9 @@ var _ = Describe("CFServiceInstance", func() { g.Expect(brokerClient.DeprovisionCallCount()).To(Equal(1)) _, actualDeprovisionRequest := brokerClient.DeprovisionArgsForCall(0) - Expect(actualDeprovisionRequest).To(Equal(osbapi.InstanceDeprovisionPayload{ + Expect(actualDeprovisionRequest).To(Equal(osbapi.DeprovisionPayload{ ID: instance.Name, - InstanceDeprovisionRequest: osbapi.InstanceDeprovisionRequest{ + DeprovisionRequest: osbapi.DeprovisionRequest{ ServiceId: "service-offering-id", PlanID: "service-plan-id", }, @@ -760,7 +760,7 @@ var _ = Describe("CFServiceInstance", func() { When("deprovision fails", func() { BeforeEach(func() { - brokerClient.DeprovisionReturns(osbapi.ServiceInstanceOperationResponse{}, errors.New("deprovision-failed")) + brokerClient.DeprovisionReturns(osbapi.ProvisionResponse{}, errors.New("deprovision-failed")) }) It("the instance is not deleted", func() { @@ -773,7 +773,7 @@ var _ = Describe("CFServiceInstance", func() { When("the instance is deleted asynchronously", func() { BeforeEach(func() { - brokerClient.DeprovisionReturns(osbapi.ServiceInstanceOperationResponse{ + brokerClient.DeprovisionReturns(osbapi.ProvisionResponse{ IsAsync: true, Operation: "deprovision-op", }, nil) @@ -812,7 +812,7 @@ var _ = Describe("CFServiceInstance", func() { Eventually(func(g Gomega) { g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(BeNumerically(">", 1)) _, actualLastOpPayload := brokerClient.GetServiceInstanceLastOperationArgsForCall(1) - g.Expect(actualLastOpPayload).To(Equal(osbapi.GetServiceInstanceLastOperationRequest{ + g.Expect(actualLastOpPayload).To(Equal(osbapi.GetInstanceLastOperationRequest{ InstanceID: instance.Name, GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ ServiceId: "service-offering-id", @@ -861,16 +861,16 @@ var _ = Describe("CFServiceInstance", func() { When("service deprovisioning fails with recoverable error", func() { BeforeEach(func() { - brokerClient.DeprovisionReturns(osbapi.ServiceInstanceOperationResponse{}, errors.New("deprovision-failed")) + brokerClient.DeprovisionReturns(osbapi.ProvisionResponse{}, errors.New("deprovision-failed")) }) It("keeps trying to deprovision the instance", func() { Eventually(func(g Gomega) { g.Expect(brokerClient.DeprovisionCallCount()).To(BeNumerically(">", 1)) _, actualDeprovisionRequest := brokerClient.DeprovisionArgsForCall(0) - g.Expect(actualDeprovisionRequest).To(Equal(osbapi.InstanceDeprovisionPayload{ + g.Expect(actualDeprovisionRequest).To(Equal(osbapi.DeprovisionPayload{ ID: instance.Name, - InstanceDeprovisionRequest: osbapi.InstanceDeprovisionRequest{ + DeprovisionRequest: osbapi.DeprovisionRequest{ ServiceId: "service-offering-id", PlanID: "service-plan-id", }, @@ -892,7 +892,7 @@ var _ = Describe("CFServiceInstance", func() { When("service deprovisioning fails with unrecoverable error", func() { BeforeEach(func() { - brokerClient.DeprovisionReturns(osbapi.ServiceInstanceOperationResponse{}, osbapi.UnrecoverableError{Status: http.StatusGone}) + brokerClient.DeprovisionReturns(osbapi.ProvisionResponse{}, osbapi.UnrecoverableError{Status: http.StatusUnprocessableEntity}) }) It("fails the instance", func() { @@ -908,7 +908,7 @@ var _ = Describe("CFServiceInstance", func() { HasType(Equal(korifiv1alpha1.DeprovisioningFailedCondition)), HasStatus(Equal(metav1.ConditionTrue)), HasReason(Equal("DeprovisionFailed")), - HasMessage(ContainSubstring("The server responded with status: 410")), + HasMessage(ContainSubstring("The server responded with status: 422")), ), )) }).Should(Succeed()) @@ -925,6 +925,19 @@ var _ = Describe("CFServiceInstance", func() { }).Should(Succeed()) }) }) + + When("the instance is gone", func() { + BeforeEach(func() { + brokerClient.DeprovisionReturns(osbapi.ProvisionResponse{}, osbapi.GoneError{}) + }) + + It("deletes the instance", func() { + Eventually(func(g Gomega) { + err := adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance) + g.Expect(k8serrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + }) + }) }) }) }) diff --git a/controllers/controllers/services/osbapi/client.go b/controllers/controllers/services/osbapi/client.go index ecf62c4f6..81e310c28 100644 --- a/controllers/controllers/services/osbapi/client.go +++ b/controllers/controllers/services/osbapi/client.go @@ -20,12 +20,6 @@ func (g GoneError) Error() string { return "The operation resource is gone" } -type ConflictError struct{} - -func (c ConflictError) Error() string { - return "The service binding already exists" -} - type UnrecoverableError struct { Status int } @@ -82,7 +76,7 @@ func (c *Client) GetCatalog(ctx context.Context) (Catalog, error) { return catalog, nil } -func (c *Client) Provision(ctx context.Context, payload InstanceProvisionPayload) (ServiceInstanceOperationResponse, error) { +func (c *Client) Provision(ctx context.Context, payload ProvisionPayload) (ProvisionResponse, error) { statusCode, respBytes, err := c.newBrokerRequester(). forBroker(c.broker). async(). @@ -91,32 +85,32 @@ func (c *Client) Provision(ctx context.Context, payload InstanceProvisionPayload "/v2/service_instances/"+payload.InstanceID, http.MethodPut, nil, - payload.InstanceProvisionRequest, + payload.ProvisionRequest, ) if err != nil { - return ServiceInstanceOperationResponse{}, fmt.Errorf("provision request failed: %w", err) + return ProvisionResponse{}, fmt.Errorf("provision request failed: %w", err) } if statusCode == http.StatusBadRequest || statusCode == http.StatusConflict || statusCode == http.StatusUnprocessableEntity { - return ServiceInstanceOperationResponse{}, UnrecoverableError{Status: statusCode} + return ProvisionResponse{}, UnrecoverableError{Status: statusCode} } if statusCode >= 300 { - return ServiceInstanceOperationResponse{}, fmt.Errorf("provision request failed with status code: %d", statusCode) + return ProvisionResponse{}, fmt.Errorf("provision request failed with status code: %d", statusCode) } - response := ServiceInstanceOperationResponse{ + response := ProvisionResponse{ IsAsync: statusCode == http.StatusAccepted, } err = json.Unmarshal(respBytes, &response) if err != nil { - return ServiceInstanceOperationResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) + return ProvisionResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) } return response, nil } -func (c *Client) Deprovision(ctx context.Context, payload InstanceDeprovisionPayload) (ServiceInstanceOperationResponse, error) { +func (c *Client) Deprovision(ctx context.Context, payload DeprovisionPayload) (ProvisionResponse, error) { statusCode, respBytes, err := c.newBrokerRequester(). forBroker(c.broker). async(). @@ -125,33 +119,37 @@ func (c *Client) Deprovision(ctx context.Context, payload InstanceDeprovisionPay "/v2/service_instances/"+payload.ID, http.MethodDelete, nil, - payload.InstanceDeprovisionRequest, + payload.DeprovisionRequest, ) if err != nil { - return ServiceInstanceOperationResponse{}, fmt.Errorf("deprovision request failed: %w", err) + return ProvisionResponse{}, fmt.Errorf("deprovision request failed: %w", err) } - if statusCode == http.StatusBadRequest || statusCode == http.StatusGone || statusCode == http.StatusUnprocessableEntity { - return ServiceInstanceOperationResponse{}, UnrecoverableError{Status: statusCode} + if statusCode == http.StatusGone { + return ProvisionResponse{}, GoneError{} + } + + if statusCode == http.StatusBadRequest || statusCode == http.StatusUnprocessableEntity { + return ProvisionResponse{}, UnrecoverableError{Status: statusCode} } if statusCode >= 300 { - return ServiceInstanceOperationResponse{}, fmt.Errorf("deprovision request failed with status code: %d", statusCode) + return ProvisionResponse{}, fmt.Errorf("deprovision request failed with status code: %d", statusCode) } - response := ServiceInstanceOperationResponse{ + response := ProvisionResponse{ IsAsync: statusCode == http.StatusAccepted, } err = json.Unmarshal(respBytes, &response) if err != nil { - return ServiceInstanceOperationResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) + return ProvisionResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) } return response, nil } -func (c *Client) GetServiceInstanceLastOperation(ctx context.Context, request GetServiceInstanceLastOperationRequest) (LastOperationResponse, error) { +func (c *Client) GetServiceInstanceLastOperation(ctx context.Context, request GetInstanceLastOperationRequest) (LastOperationResponse, error) { statusCode, respBytes, err := c.newBrokerRequester(). forBroker(c.broker). sendRequest( @@ -186,36 +184,6 @@ func (c *Client) GetServiceInstanceLastOperation(ctx context.Context, request Ge return response, nil } -func (c *Client) GetServiceBinding(ctx context.Context, request GetServiceBindingRequest) (GetBindingResponse, error) { - statusCode, respBytes, err := c.newBrokerRequester(). - forBroker(c.broker). - sendRequest( - ctx, - "/v2/service_instances/"+request.InstanceID+"/service_bindings/"+request.BindingID, - http.MethodGet, - map[string]string{ - "service_id": request.ServiceId, - "plan_id": request.PlanID, - }, - nil, - ) - if err != nil { - return GetBindingResponse{}, fmt.Errorf("get binding request failed: %w", err) - } - - if statusCode != http.StatusOK { - return GetBindingResponse{}, fmt.Errorf("get binding request failed with code: %d", statusCode) - } - - var response GetBindingResponse - err = json.Unmarshal(respBytes, &response) - if err != nil { - return GetBindingResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) - } - - return response, nil -} - func (c *Client) Bind(ctx context.Context, payload BindPayload) (BindResponse, error) { statusCode, respBytes, err := c.newBrokerRequester(). forBroker(c.broker). @@ -231,17 +199,16 @@ func (c *Client) Bind(ctx context.Context, payload BindPayload) (BindResponse, e return BindResponse{}, fmt.Errorf("bind request failed: %w", err) } - if statusCode == http.StatusConflict { - return BindResponse{}, ConflictError{} + if statusCode == http.StatusBadRequest || statusCode == http.StatusConflict || statusCode == http.StatusUnprocessableEntity { + return BindResponse{}, UnrecoverableError{Status: statusCode} } if statusCode >= 300 { return BindResponse{}, fmt.Errorf("binding request failed with code: %d", statusCode) } - var response BindResponse - if statusCode == http.StatusCreated { - response.Complete = true + response := BindResponse{ + IsAsync: statusCode == http.StatusAccepted, } err = json.Unmarshal(respBytes, &response) @@ -252,7 +219,7 @@ func (c *Client) Bind(ctx context.Context, payload BindPayload) (BindResponse, e return response, nil } -func (c *Client) GetServiceBindingLastOperation(ctx context.Context, request GetServiceBindingLastOperationRequest) (LastOperationResponse, error) { +func (c *Client) GetServiceBindingLastOperation(ctx context.Context, request GetBindingLastOperationRequest) (LastOperationResponse, error) { statusCode, respBytes, err := c.newBrokerRequester(). forBroker(c.broker). sendRequest( @@ -309,11 +276,17 @@ func (c *Client) Unbind(ctx context.Context, payload UnbindPayload) (UnbindRespo return UnbindResponse{}, GoneError{} } + if statusCode == http.StatusBadRequest || statusCode == http.StatusUnprocessableEntity { + return UnbindResponse{}, UnrecoverableError{Status: statusCode} + } + if statusCode >= 300 { return UnbindResponse{}, fmt.Errorf("unbind request failed with status code: %d", statusCode) } - var response UnbindResponse + response := UnbindResponse{ + IsAsync: statusCode == http.StatusAccepted, + } err = json.Unmarshal(respBytes, &response) if err != nil { return UnbindResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) diff --git a/controllers/controllers/services/osbapi/client_test.go b/controllers/controllers/services/osbapi/client_test.go index c6ec2c199..972e1f624 100644 --- a/controllers/controllers/services/osbapi/client_test.go +++ b/controllers/controllers/services/osbapi/client_test.go @@ -119,7 +119,7 @@ var _ = Describe("OSBAPI Client", func() { Describe("Instances", func() { Describe("Provision", func() { var ( - provisionResp osbapi.ServiceInstanceOperationResponse + provisionResp osbapi.ProvisionResponse provisionErr error ) @@ -132,9 +132,9 @@ var _ = Describe("OSBAPI Client", func() { }) JustBeforeEach(func() { - provisionResp, provisionErr = brokerClient.Provision(ctx, osbapi.InstanceProvisionPayload{ + provisionResp, provisionErr = brokerClient.Provision(ctx, osbapi.ProvisionPayload{ InstanceID: "my-service-instance", - InstanceProvisionRequest: osbapi.InstanceProvisionRequest{ + ProvisionRequest: osbapi.ProvisionRequest{ ServiceId: "service-guid", PlanID: "plan-guid", SpaceGUID: "space-guid", @@ -182,7 +182,7 @@ var _ = Describe("OSBAPI Client", func() { It("provisions the service synchronously", func() { Expect(provisionErr).NotTo(HaveOccurred()) - Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{})) + Expect(provisionResp).To(Equal(osbapi.ProvisionResponse{})) }) When("the broker accepts the provision request", func() { @@ -198,7 +198,7 @@ var _ = Describe("OSBAPI Client", func() { It("provisions the service asynchronously", func() { Expect(provisionErr).NotTo(HaveOccurred()) - Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{ + Expect(provisionResp).To(Equal(osbapi.ProvisionResponse{ IsAsync: true, Operation: "provision_op1", })) @@ -248,7 +248,7 @@ var _ = Describe("OSBAPI Client", func() { Describe("Deprovision", func() { var ( - deprovisionResp osbapi.ServiceInstanceOperationResponse + deprovisionResp osbapi.ProvisionResponse deprovisionErr error ) @@ -263,9 +263,9 @@ var _ = Describe("OSBAPI Client", func() { }) JustBeforeEach(func() { - deprovisionResp, deprovisionErr = brokerClient.Deprovision(ctx, osbapi.InstanceDeprovisionPayload{ + deprovisionResp, deprovisionErr = brokerClient.Deprovision(ctx, osbapi.DeprovisionPayload{ ID: "my-service-instance", - InstanceDeprovisionRequest: osbapi.InstanceDeprovisionRequest{ + DeprovisionRequest: osbapi.DeprovisionRequest{ ServiceId: "service-guid", PlanID: "plan-guid", }, @@ -274,7 +274,7 @@ var _ = Describe("OSBAPI Client", func() { It("deprovisions the service synchronously", func() { Expect(deprovisionErr).NotTo(HaveOccurred()) - Expect(deprovisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{ + Expect(deprovisionResp).To(Equal(osbapi.ProvisionResponse{ IsAsync: false, Operation: "deprovision_op1", })) @@ -322,7 +322,7 @@ var _ = Describe("OSBAPI Client", func() { It("deprovisions the service asynchronously", func() { Expect(deprovisionErr).NotTo(HaveOccurred()) - Expect(deprovisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{ + Expect(deprovisionResp).To(Equal(osbapi.ProvisionResponse{ IsAsync: true, Operation: "deprovision_op1", })) @@ -339,13 +339,13 @@ var _ = Describe("OSBAPI Client", func() { }) }) - When("the provision request fails with 410 Gone error", func() { + When("the deprovision request fails with 410 Gone error", func() { BeforeEach(func() { brokerServer = brokerServer.WithResponse("/v2/service_instances/{id}", nil, http.StatusGone) }) - It("returns an unrecoverable error", func() { - Expect(deprovisionErr).To(Equal(osbapi.UnrecoverableError{Status: http.StatusGone})) + It("returns a Gone error", func() { + Expect(deprovisionErr).To(Equal(osbapi.GoneError{})) }) }) @@ -378,7 +378,7 @@ var _ = Describe("OSBAPI Client", func() { var ( lastOpResp osbapi.LastOperationResponse lastOpErr error - lastOperationRequest osbapi.GetServiceInstanceLastOperationRequest + lastOperationRequest osbapi.GetInstanceLastOperationRequest ) BeforeEach(func() { @@ -391,7 +391,7 @@ var _ = Describe("OSBAPI Client", func() { http.StatusOK, ) - lastOperationRequest = osbapi.GetServiceInstanceLastOperationRequest{ + lastOperationRequest = osbapi.GetInstanceLastOperationRequest{ InstanceID: "my-service-instance", GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ ServiceId: "service-guid", @@ -434,7 +434,7 @@ var _ = Describe("OSBAPI Client", func() { When("request parameters are not specified", func() { BeforeEach(func() { - lastOperationRequest = osbapi.GetServiceInstanceLastOperationRequest{ + lastOperationRequest = osbapi.GetInstanceLastOperationRequest{ InstanceID: "my-service-instance", } }) @@ -560,7 +560,6 @@ var _ = Describe("OSBAPI Client", func() { Credentials: map[string]any{ "foo": "bar", }, - Complete: true, })) }) @@ -579,7 +578,7 @@ var _ = Describe("OSBAPI Client", func() { Expect(bindErr).NotTo(HaveOccurred()) Expect(bindResp).To(Equal(osbapi.BindResponse{ Operation: "bind_op1", - Complete: false, + IsAsync: true, })) }) }) @@ -598,7 +597,7 @@ var _ = Describe("OSBAPI Client", func() { }) }) - When("binding request fails with 409 Confilct", func() { + When("binding request fails with 409 Conflict", func() { BeforeEach(func() { brokerServer = brokerServer.WithResponse( "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", @@ -607,8 +606,36 @@ var _ = Describe("OSBAPI Client", func() { ) }) - It("returns a confilct error", func() { - Expect(bindErr).To(BeAssignableToTypeOf(osbapi.ConflictError{})) + It("returns an unrecoverable error", func() { + Expect(bindErr).To(BeAssignableToTypeOf(osbapi.UnrecoverableError{})) + }) + }) + + When("binding request fails with 422 Unprocessable Entity", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse( + "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", + nil, + http.StatusUnprocessableEntity, + ) + }) + + It("returns an unrecoverable error", func() { + Expect(bindErr).To(BeAssignableToTypeOf(osbapi.UnrecoverableError{})) + }) + }) + + When("binding request fails with 400 Bad Request", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse( + "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", + nil, + http.StatusBadRequest, + ) + }) + + It("returns an unrecoverable error", func() { + Expect(bindErr).To(BeAssignableToTypeOf(osbapi.UnrecoverableError{})) }) }) }) @@ -631,7 +658,7 @@ var _ = Describe("OSBAPI Client", func() { }) JustBeforeEach(func() { - lastOpResp, lastOpErr = brokerClient.GetServiceBindingLastOperation(ctx, osbapi.GetServiceBindingLastOperationRequest{ + lastOpResp, lastOpErr = brokerClient.GetServiceBindingLastOperation(ctx, osbapi.GetBindingLastOperationRequest{ InstanceID: "my-service-instance", BindingID: "my-binding-id", GetLastOperationRequestParameters: osbapi.GetLastOperationRequestParameters{ @@ -693,166 +720,114 @@ var _ = Describe("OSBAPI Client", func() { }) }) - Describe("GetServiceBinding", func() { + Describe("Unbind", func() { var ( - getBindingResponse osbapi.GetBindingResponse - getBindingErr error + unbindResp osbapi.UnbindResponse + unbindErr error ) BeforeEach(func() { brokerServer.WithResponse( "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", - map[string]any{ - "credentials": map[string]any{ - "credentialKey": "credentialValue", - }, - }, + nil, http.StatusOK, ) }) JustBeforeEach(func() { - getBindingResponse, getBindingErr = brokerClient.GetServiceBinding(ctx, osbapi.GetServiceBindingRequest{ - InstanceID: "my-service-instance", - BindingID: "my-binding-id", - ServiceId: "service-guid", - PlanID: "plan-guid", - }) - }) - - It("gets the binding", func() { - Expect(getBindingErr).NotTo(HaveOccurred()) - Expect(getBindingResponse).To(Equal(osbapi.GetBindingResponse{ - Credentials: map[string]any{ - "credentialKey": "credentialValue", + unbindResp, unbindErr = brokerClient.Unbind(ctx, osbapi.UnbindPayload{ + InstanceID: "instance-id", + BindingID: "binding-id", + UnbindRequestParameters: osbapi.UnbindRequestParameters{ + ServiceId: "service-guid", + PlanID: "plan-guid", }, - })) + }) }) - It("sends correct request to broker", func() { - Expect(getBindingErr).NotTo(HaveOccurred()) + It("sends an unbind request to broker", func() { + Expect(unbindErr).NotTo(HaveOccurred()) requests := brokerServer.ServedRequests() Expect(requests).To(HaveLen(1)) - Expect(requests[0].Method).To(Equal(http.MethodGet)) - Expect(requests[0].URL.Path).To(Equal("/v2/service_instances/my-service-instance/service_bindings/my-binding-id")) + Expect(requests[0].Method).To(Equal(http.MethodDelete)) + Expect(requests[0].URL.Path).To(Equal("/v2/service_instances/instance-id/service_bindings/binding-id")) Expect(requests[0].URL.Query()).To(BeEquivalentTo(map[string][]string{ - "service_id": {"service-guid"}, - "plan_id": {"plan-guid"}, + "service_id": {"service-guid"}, + "plan_id": {"plan-guid"}, + "accepts_incomplete": {"true"}, })) + }) - requestBytes, err := io.ReadAll(requests[0].Body) - Expect(err).NotTo(HaveOccurred()) - Expect(requestBytes).To(BeEmpty()) + It("responds synchronously", func() { + Expect(unbindErr).NotTo(HaveOccurred()) + Expect(unbindResp.IsComplete()).To(BeTrue()) }) - When("getting the binding fails", func() { + When("the broker accepts the unbind request", func() { BeforeEach(func() { brokerServer = brokerServer.WithResponse( "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", - nil, - http.StatusTeapot, + map[string]any{ + "operation": "unbind_op1", + }, + http.StatusAccepted, ) }) - It("returns an error", func() { - Expect(getBindingErr).To(MatchError(ContainSubstring("get binding request failed"))) + It("unbinds the service asynchronously", func() { + Expect(unbindErr).NotTo(HaveOccurred()) + Expect(unbindResp).To(Equal(osbapi.UnbindResponse{ + IsAsync: true, + Operation: "unbind_op1", + })) }) }) - }) - }) - Describe("Unbind", func() { - var ( - unbindResp osbapi.UnbindResponse - unbindErr error - ) - - BeforeEach(func() { - brokerServer.WithResponse( - "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", - nil, - http.StatusOK, - ) - }) + When("the unbind request fails", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse( + "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", + nil, + http.StatusTeapot, + ) + }) - JustBeforeEach(func() { - unbindResp, unbindErr = brokerClient.Unbind(ctx, osbapi.UnbindPayload{ - InstanceID: "instance-id", - BindingID: "binding-id", - UnbindRequestParameters: osbapi.UnbindRequestParameters{ - ServiceId: "service-guid", - PlanID: "plan-guid", - }, + It("returns an error", func() { + Expect(unbindErr).To(MatchError(ContainSubstring("unbind request failed"))) + }) }) - }) - It("sends an unbind request to broker", func() { - Expect(unbindErr).NotTo(HaveOccurred()) - requests := brokerServer.ServedRequests() - - Expect(requests).To(HaveLen(1)) - - Expect(requests[0].Method).To(Equal(http.MethodDelete)) - Expect(requests[0].URL.Path).To(Equal("/v2/service_instances/instance-id/service_bindings/binding-id")) - - Expect(requests[0].URL.Query()).To(BeEquivalentTo(map[string][]string{ - "service_id": {"service-guid"}, - "plan_id": {"plan-guid"}, - "accepts_incomplete": {"true"}, - })) - }) - - It("responds synchronously", func() { - Expect(unbindErr).NotTo(HaveOccurred()) - Expect(unbindResp.IsComplete()).To(BeTrue()) - }) - - When("broker return 202 Accepted", func() { - BeforeEach(func() { - brokerServer = brokerServer.WithResponse( - "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", - map[string]any{ - "operation": "operation-id", - }, - http.StatusAccepted, - ) - }) + When("the unbind request fails with 400 BadRequest error", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", nil, http.StatusBadRequest) + }) - It("responds asynchronously", func() { - Expect(unbindErr).NotTo(HaveOccurred()) - Expect(unbindResp.IsComplete()).To(BeFalse()) - Expect(unbindResp.Operation).To(Equal("operation-id")) + It("returns an unrecoverable error", func() { + Expect(unbindErr).To(Equal(osbapi.UnrecoverableError{Status: http.StatusBadRequest})) + }) }) - }) - When("the binding does not exist", func() { - BeforeEach(func() { - brokerServer = brokerServer.WithResponse( - "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", - nil, - http.StatusGone, - ) - }) + When("the unbind request fails with 410 Gone error", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", nil, http.StatusGone) + }) - It("returns a gone error", func() { - Expect(unbindErr).To(BeAssignableToTypeOf(osbapi.GoneError{})) + It("returns a Gone error", func() { + Expect(unbindErr).To(Equal(osbapi.GoneError{})) + }) }) - }) - When("the unbind request fails", func() { - BeforeEach(func() { - brokerServer = brokerServer.WithResponse( - "/v2/service_instances/{instance_id}/service_bindings/{binding_id}", - nil, - http.StatusTeapot, - ) - }) + When("the unbind request fails with 422 Unprocessable entity error", func() { + BeforeEach(func() { + brokerServer = brokerServer.WithResponse("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", nil, http.StatusUnprocessableEntity) + }) - It("returns an error", func() { - Expect(unbindErr).To(MatchError(ContainSubstring("unbind request failed"))) + It("returns an unrecoverable error", func() { + Expect(unbindErr).To(Equal(osbapi.UnrecoverableError{Status: http.StatusUnprocessableEntity})) + }) }) }) }) diff --git a/controllers/controllers/services/osbapi/clientfactory.go b/controllers/controllers/services/osbapi/clientfactory.go index eb30f934f..085a08c25 100644 --- a/controllers/controllers/services/osbapi/clientfactory.go +++ b/controllers/controllers/services/osbapi/clientfactory.go @@ -17,14 +17,13 @@ import ( //counterfeiter:generate -o fake -fake-name BrokerClient code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi.BrokerClient type BrokerClient interface { - Provision(context.Context, InstanceProvisionPayload) (ServiceInstanceOperationResponse, error) - Deprovision(context.Context, InstanceDeprovisionPayload) (ServiceInstanceOperationResponse, error) - GetServiceInstanceLastOperation(context.Context, GetServiceInstanceLastOperationRequest) (LastOperationResponse, error) + Provision(context.Context, ProvisionPayload) (ProvisionResponse, error) + Deprovision(context.Context, DeprovisionPayload) (ProvisionResponse, error) + GetServiceInstanceLastOperation(context.Context, GetInstanceLastOperationRequest) (LastOperationResponse, error) GetCatalog(context.Context) (Catalog, error) Bind(context.Context, BindPayload) (BindResponse, error) Unbind(context.Context, UnbindPayload) (UnbindResponse, error) - GetServiceBindingLastOperation(context.Context, GetServiceBindingLastOperationRequest) (LastOperationResponse, error) - GetServiceBinding(context.Context, GetServiceBindingRequest) (GetBindingResponse, error) + GetServiceBindingLastOperation(context.Context, GetBindingLastOperationRequest) (LastOperationResponse, error) } //counterfeiter:generate -o fake -fake-name BrokerClientFactory code.cloudfoundry.org/korifi/controllers/controllers/services/osbapi.BrokerClientFactory diff --git a/controllers/controllers/services/osbapi/fake/broker_client.go b/controllers/controllers/services/osbapi/fake/broker_client.go index c27849378..a273dda20 100644 --- a/controllers/controllers/services/osbapi/fake/broker_client.go +++ b/controllers/controllers/services/osbapi/fake/broker_client.go @@ -23,18 +23,18 @@ type BrokerClient struct { result1 osbapi.BindResponse result2 error } - DeprovisionStub func(context.Context, osbapi.InstanceDeprovisionPayload) (osbapi.ServiceInstanceOperationResponse, error) + DeprovisionStub func(context.Context, osbapi.DeprovisionPayload) (osbapi.ProvisionResponse, error) deprovisionMutex sync.RWMutex deprovisionArgsForCall []struct { arg1 context.Context - arg2 osbapi.InstanceDeprovisionPayload + arg2 osbapi.DeprovisionPayload } deprovisionReturns struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error } deprovisionReturnsOnCall map[int]struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error } GetCatalogStub func(context.Context) (osbapi.Catalog, error) @@ -50,11 +50,11 @@ type BrokerClient struct { result1 osbapi.Catalog result2 error } - GetServiceBindingStub func(context.Context, osbapi.GetServiceBindingRequest) (osbapi.GetBindingResponse, error) + GetServiceBindingStub func(context.Context, osbapi.GetBindingRequest) (osbapi.GetBindingResponse, error) getServiceBindingMutex sync.RWMutex getServiceBindingArgsForCall []struct { arg1 context.Context - arg2 osbapi.GetServiceBindingRequest + arg2 osbapi.GetBindingRequest } getServiceBindingReturns struct { result1 osbapi.GetBindingResponse @@ -64,11 +64,11 @@ type BrokerClient struct { result1 osbapi.GetBindingResponse result2 error } - GetServiceBindingLastOperationStub func(context.Context, osbapi.GetServiceBindingLastOperationRequest) (osbapi.LastOperationResponse, error) + GetServiceBindingLastOperationStub func(context.Context, osbapi.GetBindingLastOperationRequest) (osbapi.LastOperationResponse, error) getServiceBindingLastOperationMutex sync.RWMutex getServiceBindingLastOperationArgsForCall []struct { arg1 context.Context - arg2 osbapi.GetServiceBindingLastOperationRequest + arg2 osbapi.GetBindingLastOperationRequest } getServiceBindingLastOperationReturns struct { result1 osbapi.LastOperationResponse @@ -78,11 +78,11 @@ type BrokerClient struct { result1 osbapi.LastOperationResponse result2 error } - GetServiceInstanceLastOperationStub func(context.Context, osbapi.GetServiceInstanceLastOperationRequest) (osbapi.LastOperationResponse, error) + GetServiceInstanceLastOperationStub func(context.Context, osbapi.GetInstanceLastOperationRequest) (osbapi.LastOperationResponse, error) getServiceInstanceLastOperationMutex sync.RWMutex getServiceInstanceLastOperationArgsForCall []struct { arg1 context.Context - arg2 osbapi.GetServiceInstanceLastOperationRequest + arg2 osbapi.GetInstanceLastOperationRequest } getServiceInstanceLastOperationReturns struct { result1 osbapi.LastOperationResponse @@ -92,18 +92,18 @@ type BrokerClient struct { result1 osbapi.LastOperationResponse result2 error } - ProvisionStub func(context.Context, osbapi.InstanceProvisionPayload) (osbapi.ServiceInstanceOperationResponse, error) + ProvisionStub func(context.Context, osbapi.ProvisionPayload) (osbapi.ProvisionResponse, error) provisionMutex sync.RWMutex provisionArgsForCall []struct { arg1 context.Context - arg2 osbapi.InstanceProvisionPayload + arg2 osbapi.ProvisionPayload } provisionReturns struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error } provisionReturnsOnCall map[int]struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error } UnbindStub func(context.Context, osbapi.UnbindPayload) (osbapi.UnbindResponse, error) @@ -189,12 +189,12 @@ func (fake *BrokerClient) BindReturnsOnCall(i int, result1 osbapi.BindResponse, }{result1, result2} } -func (fake *BrokerClient) Deprovision(arg1 context.Context, arg2 osbapi.InstanceDeprovisionPayload) (osbapi.ServiceInstanceOperationResponse, error) { +func (fake *BrokerClient) Deprovision(arg1 context.Context, arg2 osbapi.DeprovisionPayload) (osbapi.ProvisionResponse, error) { fake.deprovisionMutex.Lock() ret, specificReturn := fake.deprovisionReturnsOnCall[len(fake.deprovisionArgsForCall)] fake.deprovisionArgsForCall = append(fake.deprovisionArgsForCall, struct { arg1 context.Context - arg2 osbapi.InstanceDeprovisionPayload + arg2 osbapi.DeprovisionPayload }{arg1, arg2}) stub := fake.DeprovisionStub fakeReturns := fake.deprovisionReturns @@ -215,41 +215,41 @@ func (fake *BrokerClient) DeprovisionCallCount() int { return len(fake.deprovisionArgsForCall) } -func (fake *BrokerClient) DeprovisionCalls(stub func(context.Context, osbapi.InstanceDeprovisionPayload) (osbapi.ServiceInstanceOperationResponse, error)) { +func (fake *BrokerClient) DeprovisionCalls(stub func(context.Context, osbapi.DeprovisionPayload) (osbapi.ProvisionResponse, error)) { fake.deprovisionMutex.Lock() defer fake.deprovisionMutex.Unlock() fake.DeprovisionStub = stub } -func (fake *BrokerClient) DeprovisionArgsForCall(i int) (context.Context, osbapi.InstanceDeprovisionPayload) { +func (fake *BrokerClient) DeprovisionArgsForCall(i int) (context.Context, osbapi.DeprovisionPayload) { fake.deprovisionMutex.RLock() defer fake.deprovisionMutex.RUnlock() argsForCall := fake.deprovisionArgsForCall[i] return argsForCall.arg1, argsForCall.arg2 } -func (fake *BrokerClient) DeprovisionReturns(result1 osbapi.ServiceInstanceOperationResponse, result2 error) { +func (fake *BrokerClient) DeprovisionReturns(result1 osbapi.ProvisionResponse, result2 error) { fake.deprovisionMutex.Lock() defer fake.deprovisionMutex.Unlock() fake.DeprovisionStub = nil fake.deprovisionReturns = struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error }{result1, result2} } -func (fake *BrokerClient) DeprovisionReturnsOnCall(i int, result1 osbapi.ServiceInstanceOperationResponse, result2 error) { +func (fake *BrokerClient) DeprovisionReturnsOnCall(i int, result1 osbapi.ProvisionResponse, result2 error) { fake.deprovisionMutex.Lock() defer fake.deprovisionMutex.Unlock() fake.DeprovisionStub = nil if fake.deprovisionReturnsOnCall == nil { fake.deprovisionReturnsOnCall = make(map[int]struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error }) } fake.deprovisionReturnsOnCall[i] = struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error }{result1, result2} } @@ -318,12 +318,12 @@ func (fake *BrokerClient) GetCatalogReturnsOnCall(i int, result1 osbapi.Catalog, }{result1, result2} } -func (fake *BrokerClient) GetServiceBinding(arg1 context.Context, arg2 osbapi.GetServiceBindingRequest) (osbapi.GetBindingResponse, error) { +func (fake *BrokerClient) GetServiceBinding(arg1 context.Context, arg2 osbapi.GetBindingRequest) (osbapi.GetBindingResponse, error) { fake.getServiceBindingMutex.Lock() ret, specificReturn := fake.getServiceBindingReturnsOnCall[len(fake.getServiceBindingArgsForCall)] fake.getServiceBindingArgsForCall = append(fake.getServiceBindingArgsForCall, struct { arg1 context.Context - arg2 osbapi.GetServiceBindingRequest + arg2 osbapi.GetBindingRequest }{arg1, arg2}) stub := fake.GetServiceBindingStub fakeReturns := fake.getServiceBindingReturns @@ -344,13 +344,13 @@ func (fake *BrokerClient) GetServiceBindingCallCount() int { return len(fake.getServiceBindingArgsForCall) } -func (fake *BrokerClient) GetServiceBindingCalls(stub func(context.Context, osbapi.GetServiceBindingRequest) (osbapi.GetBindingResponse, error)) { +func (fake *BrokerClient) GetServiceBindingCalls(stub func(context.Context, osbapi.GetBindingRequest) (osbapi.GetBindingResponse, error)) { fake.getServiceBindingMutex.Lock() defer fake.getServiceBindingMutex.Unlock() fake.GetServiceBindingStub = stub } -func (fake *BrokerClient) GetServiceBindingArgsForCall(i int) (context.Context, osbapi.GetServiceBindingRequest) { +func (fake *BrokerClient) GetServiceBindingArgsForCall(i int) (context.Context, osbapi.GetBindingRequest) { fake.getServiceBindingMutex.RLock() defer fake.getServiceBindingMutex.RUnlock() argsForCall := fake.getServiceBindingArgsForCall[i] @@ -383,12 +383,12 @@ func (fake *BrokerClient) GetServiceBindingReturnsOnCall(i int, result1 osbapi.G }{result1, result2} } -func (fake *BrokerClient) GetServiceBindingLastOperation(arg1 context.Context, arg2 osbapi.GetServiceBindingLastOperationRequest) (osbapi.LastOperationResponse, error) { +func (fake *BrokerClient) GetServiceBindingLastOperation(arg1 context.Context, arg2 osbapi.GetBindingLastOperationRequest) (osbapi.LastOperationResponse, error) { fake.getServiceBindingLastOperationMutex.Lock() ret, specificReturn := fake.getServiceBindingLastOperationReturnsOnCall[len(fake.getServiceBindingLastOperationArgsForCall)] fake.getServiceBindingLastOperationArgsForCall = append(fake.getServiceBindingLastOperationArgsForCall, struct { arg1 context.Context - arg2 osbapi.GetServiceBindingLastOperationRequest + arg2 osbapi.GetBindingLastOperationRequest }{arg1, arg2}) stub := fake.GetServiceBindingLastOperationStub fakeReturns := fake.getServiceBindingLastOperationReturns @@ -409,13 +409,13 @@ func (fake *BrokerClient) GetServiceBindingLastOperationCallCount() int { return len(fake.getServiceBindingLastOperationArgsForCall) } -func (fake *BrokerClient) GetServiceBindingLastOperationCalls(stub func(context.Context, osbapi.GetServiceBindingLastOperationRequest) (osbapi.LastOperationResponse, error)) { +func (fake *BrokerClient) GetServiceBindingLastOperationCalls(stub func(context.Context, osbapi.GetBindingLastOperationRequest) (osbapi.LastOperationResponse, error)) { fake.getServiceBindingLastOperationMutex.Lock() defer fake.getServiceBindingLastOperationMutex.Unlock() fake.GetServiceBindingLastOperationStub = stub } -func (fake *BrokerClient) GetServiceBindingLastOperationArgsForCall(i int) (context.Context, osbapi.GetServiceBindingLastOperationRequest) { +func (fake *BrokerClient) GetServiceBindingLastOperationArgsForCall(i int) (context.Context, osbapi.GetBindingLastOperationRequest) { fake.getServiceBindingLastOperationMutex.RLock() defer fake.getServiceBindingLastOperationMutex.RUnlock() argsForCall := fake.getServiceBindingLastOperationArgsForCall[i] @@ -448,12 +448,12 @@ func (fake *BrokerClient) GetServiceBindingLastOperationReturnsOnCall(i int, res }{result1, result2} } -func (fake *BrokerClient) GetServiceInstanceLastOperation(arg1 context.Context, arg2 osbapi.GetServiceInstanceLastOperationRequest) (osbapi.LastOperationResponse, error) { +func (fake *BrokerClient) GetServiceInstanceLastOperation(arg1 context.Context, arg2 osbapi.GetInstanceLastOperationRequest) (osbapi.LastOperationResponse, error) { fake.getServiceInstanceLastOperationMutex.Lock() ret, specificReturn := fake.getServiceInstanceLastOperationReturnsOnCall[len(fake.getServiceInstanceLastOperationArgsForCall)] fake.getServiceInstanceLastOperationArgsForCall = append(fake.getServiceInstanceLastOperationArgsForCall, struct { arg1 context.Context - arg2 osbapi.GetServiceInstanceLastOperationRequest + arg2 osbapi.GetInstanceLastOperationRequest }{arg1, arg2}) stub := fake.GetServiceInstanceLastOperationStub fakeReturns := fake.getServiceInstanceLastOperationReturns @@ -474,13 +474,13 @@ func (fake *BrokerClient) GetServiceInstanceLastOperationCallCount() int { return len(fake.getServiceInstanceLastOperationArgsForCall) } -func (fake *BrokerClient) GetServiceInstanceLastOperationCalls(stub func(context.Context, osbapi.GetServiceInstanceLastOperationRequest) (osbapi.LastOperationResponse, error)) { +func (fake *BrokerClient) GetServiceInstanceLastOperationCalls(stub func(context.Context, osbapi.GetInstanceLastOperationRequest) (osbapi.LastOperationResponse, error)) { fake.getServiceInstanceLastOperationMutex.Lock() defer fake.getServiceInstanceLastOperationMutex.Unlock() fake.GetServiceInstanceLastOperationStub = stub } -func (fake *BrokerClient) GetServiceInstanceLastOperationArgsForCall(i int) (context.Context, osbapi.GetServiceInstanceLastOperationRequest) { +func (fake *BrokerClient) GetServiceInstanceLastOperationArgsForCall(i int) (context.Context, osbapi.GetInstanceLastOperationRequest) { fake.getServiceInstanceLastOperationMutex.RLock() defer fake.getServiceInstanceLastOperationMutex.RUnlock() argsForCall := fake.getServiceInstanceLastOperationArgsForCall[i] @@ -513,12 +513,12 @@ func (fake *BrokerClient) GetServiceInstanceLastOperationReturnsOnCall(i int, re }{result1, result2} } -func (fake *BrokerClient) Provision(arg1 context.Context, arg2 osbapi.InstanceProvisionPayload) (osbapi.ServiceInstanceOperationResponse, error) { +func (fake *BrokerClient) Provision(arg1 context.Context, arg2 osbapi.ProvisionPayload) (osbapi.ProvisionResponse, error) { fake.provisionMutex.Lock() ret, specificReturn := fake.provisionReturnsOnCall[len(fake.provisionArgsForCall)] fake.provisionArgsForCall = append(fake.provisionArgsForCall, struct { arg1 context.Context - arg2 osbapi.InstanceProvisionPayload + arg2 osbapi.ProvisionPayload }{arg1, arg2}) stub := fake.ProvisionStub fakeReturns := fake.provisionReturns @@ -539,41 +539,41 @@ func (fake *BrokerClient) ProvisionCallCount() int { return len(fake.provisionArgsForCall) } -func (fake *BrokerClient) ProvisionCalls(stub func(context.Context, osbapi.InstanceProvisionPayload) (osbapi.ServiceInstanceOperationResponse, error)) { +func (fake *BrokerClient) ProvisionCalls(stub func(context.Context, osbapi.ProvisionPayload) (osbapi.ProvisionResponse, error)) { fake.provisionMutex.Lock() defer fake.provisionMutex.Unlock() fake.ProvisionStub = stub } -func (fake *BrokerClient) ProvisionArgsForCall(i int) (context.Context, osbapi.InstanceProvisionPayload) { +func (fake *BrokerClient) ProvisionArgsForCall(i int) (context.Context, osbapi.ProvisionPayload) { fake.provisionMutex.RLock() defer fake.provisionMutex.RUnlock() argsForCall := fake.provisionArgsForCall[i] return argsForCall.arg1, argsForCall.arg2 } -func (fake *BrokerClient) ProvisionReturns(result1 osbapi.ServiceInstanceOperationResponse, result2 error) { +func (fake *BrokerClient) ProvisionReturns(result1 osbapi.ProvisionResponse, result2 error) { fake.provisionMutex.Lock() defer fake.provisionMutex.Unlock() fake.ProvisionStub = nil fake.provisionReturns = struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error }{result1, result2} } -func (fake *BrokerClient) ProvisionReturnsOnCall(i int, result1 osbapi.ServiceInstanceOperationResponse, result2 error) { +func (fake *BrokerClient) ProvisionReturnsOnCall(i int, result1 osbapi.ProvisionResponse, result2 error) { fake.provisionMutex.Lock() defer fake.provisionMutex.Unlock() fake.ProvisionStub = nil if fake.provisionReturnsOnCall == nil { fake.provisionReturnsOnCall = make(map[int]struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error }) } fake.provisionReturnsOnCall[i] = struct { - result1 osbapi.ServiceInstanceOperationResponse + result1 osbapi.ProvisionResponse result2 error }{result1, result2} } diff --git a/controllers/controllers/services/osbapi/types.go b/controllers/controllers/services/osbapi/types.go index 532f1dfc0..e3cae1343 100644 --- a/controllers/controllers/services/osbapi/types.go +++ b/controllers/controllers/services/osbapi/types.go @@ -30,12 +30,24 @@ type Service struct { Plans []Plan `json:"plans"` } -type InstanceProvisionPayload struct { +type Plan struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Metadata map[string]any `json:"metadata"` + Free bool `json:"free"` + Bindable bool `json:"bindable"` + BindingRotatable bool `json:"binding_rotatable"` + PlanUpdateable bool `json:"plan_updateable"` + Schemas services.ServicePlanSchemas `json:"schemas"` +} + +type ProvisionPayload struct { InstanceID string - InstanceProvisionRequest + ProvisionRequest } -type InstanceProvisionRequest struct { +type ProvisionRequest struct { ServiceId string `json:"service_id"` PlanID string `json:"plan_id"` SpaceGUID string `json:"space_guid"` @@ -43,18 +55,12 @@ type InstanceProvisionRequest struct { Parameters map[string]any `json:"parameters"` } -type GetServiceInstanceLastOperationRequest struct { - InstanceID string - GetLastOperationRequestParameters -} - -type GetServiceBindingLastOperationRequest struct { - InstanceID string - BindingID string - GetLastOperationRequestParameters +type ProvisionResponse struct { + IsAsync bool + Operation string `json:"operation,omitempty"` } -type GetServiceBindingRequest struct { +type GetBindingRequest struct { InstanceID string BindingID string ServiceId string @@ -65,18 +71,28 @@ type GetBindingResponse struct { Credentials map[string]any `json:"credentials"` } +type GetInstanceLastOperationRequest struct { + InstanceID string + GetLastOperationRequestParameters +} + +type GetBindingLastOperationRequest struct { + InstanceID string + BindingID string + GetLastOperationRequestParameters +} + type GetLastOperationRequestParameters struct { ServiceId string PlanID string Operation string } - -type InstanceDeprovisionPayload struct { +type DeprovisionPayload struct { ID string - InstanceDeprovisionRequest + DeprovisionRequest } -type InstanceDeprovisionRequest struct { +type DeprovisionRequest struct { ServiceId string `json:"service_id"` PlanID string `json:"plan_id"` } @@ -98,7 +114,7 @@ type BindPayload struct { type BindResponse struct { Credentials map[string]any `json:"credentials"` Operation string `json:"operation"` - Complete bool + IsAsync bool } type BindResource struct { @@ -117,6 +133,7 @@ type UnbindRequestParameters struct { } type UnbindResponse struct { + IsAsync bool Operation string `json:"operation,omitempty"` } @@ -124,23 +141,6 @@ func (r UnbindResponse) IsComplete() bool { return r.Operation == "" } -type Plan struct { - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description"` - Metadata map[string]any `json:"metadata"` - Free bool `json:"free"` - Bindable bool `json:"bindable"` - BindingRotatable bool `json:"binding_rotatable"` - PlanUpdateable bool `json:"plan_updateable"` - Schemas services.ServicePlanSchemas `json:"schemas"` -} - -type ServiceInstanceOperationResponse struct { - IsAsync bool - Operation string `json:"operation,omitempty"` -} - type LastOperationResponse struct { State string `json:"state"` Description string `json:"description"` diff --git a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebindings.yaml b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebindings.yaml index 0d94732c4..aad66dbdd 100644 --- a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebindings.yaml +++ b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebindings.yaml @@ -134,13 +134,6 @@ spec: type: string type: object x-kubernetes-map-type: atomic - bindingOperation: - description: |- - The - [operation](https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#binding) - of the bind request to the the OSBAPI broker. Only makes sense for - bindings to managed service instances - type: string conditions: items: description: Condition contains details for one aspect of the current @@ -221,13 +214,6 @@ spec: the CFServiceBinding that has been reconciled format: int64 type: integer - unbindingOperation: - description: |- - The - [operation](https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#unbinding) - of the unbind request to the the OSBAPI broker. Only makes sense for - bindings to managed service instances - type: string type: object type: object served: true