Support synchronous service provisioning from OSBAPI Brokers

fixes cloudfoundry#3481

controllers/controllers/services/instances/managed/controller.go

@@ -202,6 +202,10 @@ func (r *Reconciler) provisionServiceInstance(
 		return ctrl.Result{}, fmt.Errorf("failed to provision service: %w", err)
 	}
 
+	if provisionResponse.Complete {
+		return ctrl.Result{}, nil
+	}
+
 	serviceInstance.Status.ProvisionOperation = provisionResponse.Operation
 	meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{
 		Type:               korifiv1alpha1.ProvisionRequestedCondition,
@@ -251,6 +255,13 @@ func (r *Reconciler) finalizeCFServiceInstance(
 		return ctrl.Result{}, err
 	}
 
+	meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{
+		Type:               korifiv1alpha1.DeprovisionRequestedCondition,
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: serviceInstance.Generation,
+		LastTransitionTime: metav1.NewTime(time.Now()),
+		Reason:             "DeprovisionRequested",
+	})
 	return ctrl.Result{Requeue: true}, nil
 }
 
@@ -294,15 +305,7 @@ func (r *Reconciler) deprovisionServiceinstance(ctx context.Context, serviceInst
 	}
 
 	serviceInstance.Status.DeprovisionOperation = deprovisionResponse.Operation
-	meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{
-		Type:               korifiv1alpha1.DeprovisionRequestedCondition,
-		Status:             metav1.ConditionTrue,
-		ObservedGeneration: serviceInstance.Generation,
-		LastTransitionTime: metav1.NewTime(time.Now()),
-		Reason:             "DeprovisionRequested",
-	})
-
-	return k8s.NewNotReadyError().WithReason("DeprovisionRequested").WithRequeue()
+	return nil
 }
 
 func (r *Reconciler) getNamespace(ctx context.Context, namespaceName string) (*corev1.Namespace, error) {

controllers/controllers/services/instances/managed/controller_test.go

@@ -261,6 +261,32 @@ var _ = Describe("CFServiceInstance", func() {
 		})
 	})
 
+	When("the provisioning is synchronous", func() {
+		BeforeEach(func() {
+			brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{
+				Operation: "operation-1",
+				Complete:  true,
+			}, nil)
+		})
+
+		It("does not check last operation", func() {
+			Consistently(func(g Gomega) {
+				g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(Equal(0))
+			}).Should(Succeed())
+		})
+
+		It("set sets ready condition to true", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
+
+				g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll(
+					HasType(Equal(korifiv1alpha1.StatusConditionReady)),
+					HasStatus(Equal(metav1.ConditionTrue)),
+				)))
+			}).Should(Succeed())
+		})
+	})
+
 	When("service provisioning fails", func() {
 		BeforeEach(func() {
 			brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{}, errors.New("provision-failed"))
@@ -544,6 +570,64 @@ var _ = Describe("CFServiceInstance", func() {
 				}).Should(Succeed())
 			})
 		})
+
+		When("the service plan is not available", func() {
+			BeforeEach(func() {
+				Expect(k8s.PatchResource(ctx, adminClient, instance, func() {
+					instance.Spec.PlanGUID = "does-not-exist"
+				})).To(Succeed())
+			})
+
+			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())
+			})
+		})
+
+		When("the service offering does not exist", func() {
+			BeforeEach(func() {
+				Expect(k8s.PatchResource(ctx, adminClient, servicePlan, func() {
+					servicePlan.Labels[korifiv1alpha1.RelServiceOfferingGUIDLabel] = "does-not-exist"
+				})).To(Succeed())
+			})
+
+			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())
+			})
+		})
+
+		When("the service broker does not exist", func() {
+			BeforeEach(func() {
+				Expect(k8s.PatchResource(ctx, adminClient, servicePlan, func() {
+					servicePlan.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel] = "does-not-exist"
+				})).To(Succeed())
+			})
+
+			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())
+			})
+		})
+
+		When("creating the broker client fails", func() {
+			BeforeEach(func() {
+				brokerClientFactory.CreateClientReturns(nil, errors.New("client-create-err"))
+			})
+
+			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())
+			})
+		})
 	})
 
 	When("the service instance is user-provided", func() {

controllers/controllers/services/osbapi/client.go

@@ -70,7 +70,11 @@ func (c *Client) Provision(ctx context.Context, payload InstanceProvisionPayload
 		return ServiceInstanceOperationResponse{}, fmt.Errorf("provision request failed with status code: %d", statusCode)
 	}
 
-	var response ServiceInstanceOperationResponse
+	response := ServiceInstanceOperationResponse{}
+	if statusCode == http.StatusCreated {
+		response.Complete = true
+	}
+
 	err = json.Unmarshal(respBytes, &response)
 	if err != nil {
 		return ServiceInstanceOperationResponse{}, fmt.Errorf("failed to unmarshal response: %w", err)

controllers/controllers/services/osbapi/client_test.go

@@ -138,7 +138,7 @@ var _ = Describe("OSBAPI Client", func() {
 			)
 
 			BeforeEach(func() {
-				brokerServer.WithResponse(
+				brokerServer = broker.NewServer().WithResponse(
 					"/v2/service_instances/{id}",
 					map[string]any{
 						"operation": "provision_op1",
@@ -162,13 +162,6 @@ var _ = Describe("OSBAPI Client", func() {
 				})
 			})
 
-			It("provisions the service", func() {
-				Expect(provisionErr).NotTo(HaveOccurred())
-				Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{
-					Operation: "provision_op1",
-				}))
-			})
-
 			It("sends async provision request to broker", func() {
 				Expect(provisionErr).NotTo(HaveOccurred())
 				requests := brokerServer.ServedRequests()
@@ -203,6 +196,34 @@ var _ = Describe("OSBAPI Client", func() {
 				}))
 			})
 
+			It("provisions the service synchronously", func() {
+				Expect(provisionErr).NotTo(HaveOccurred())
+				Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{
+					Operation: "provision_op1",
+					Complete:  true,
+				}))
+			})
+
+			When("the broker accepts the provision request", func() {
+				BeforeEach(func() {
+					brokerServer = broker.NewServer().WithResponse(
+						"/v2/service_instances/{id}",
+						map[string]any{
+							"operation": "provision_op1",
+						},
+						http.StatusAccepted,
+					)
+				})
+
+				It("provisions the service asynchronously", func() {
+					Expect(provisionErr).NotTo(HaveOccurred())
+					Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{
+						Operation: "provision_op1",
+						Complete:  false,
+					}))
+				})
+			})
+
 			When("the provision request fails", func() {
 				BeforeEach(func() {
 					brokerServer = broker.NewServer().WithHandler("/v2/service_instances/{id}", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {

controllers/controllers/services/osbapi/types.go

@@ -76,6 +76,7 @@ type Plan struct {
 
 type ServiceInstanceOperationResponse struct {
 	Operation string `json:"operation"`
+	Complete  bool
 }
 
 type LastOperationResponse struct {

tests/helpers/broker/broker.go

@@ -32,8 +32,9 @@ func (b *BrokerServer) WithResponse(pattern string, response map[string]any, sta
 	Expect(err).NotTo(HaveOccurred())
 
 	return b.WithHandler(pattern, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		_, _ = w.Write(respBytes)
 		w.WriteHeader(statusCode)
+		_, err := w.Write(respBytes)
+		Expect(err).NotTo(HaveOccurred())
 	}))
 }