From fdc049c085123493af70d07d32ef78db0232ce88 Mon Sep 17 00:00:00 2001 From: Jonas Kanngiesser Date: Mon, 18 Nov 2024 14:05:57 +0100 Subject: [PATCH] feat: implement `service_instances` and `service_bindings` endpoints --- brokerapi/broker/bind.go | 7 - brokerapi/broker/errors.go | 30 +++ brokerapi/broker/get_binding.go | 75 +++++- brokerapi/broker/get_binding_test.go | 356 +++++++++++++++++++++++++- brokerapi/broker/get_instance.go | 73 +++++- brokerapi/broker/get_instance_test.go | 326 ++++++++++++++++++++++- brokerapi/broker/services_test.go | 7 + brokerapi/broker/update.go | 4 - pkg/broker/service_definition.go | 8 +- pkg/broker/service_definition_test.go | 84 ++++-- 10 files changed, 894 insertions(+), 76 deletions(-) create mode 100644 brokerapi/broker/errors.go diff --git a/brokerapi/broker/bind.go b/brokerapi/broker/bind.go index 9710384a1..ada849344 100644 --- a/brokerapi/broker/bind.go +++ b/brokerapi/broker/bind.go @@ -2,9 +2,7 @@ package broker import ( "context" - "errors" "fmt" - "net/http" "code.cloudfoundry.org/lager/v3" "github.com/cloudfoundry/cloud-service-broker/v2/internal/paramparser" @@ -18,11 +16,6 @@ import ( "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" ) -var ( - invalidUserInputMsg = "User supplied parameters must be in the form of a valid JSON map." - ErrInvalidUserInput = apiresponses.NewFailureResponse(errors.New(invalidUserInputMsg), http.StatusBadRequest, "parsing-user-request") -) - // Bind creates an account with credentials to access an instance of a service. // It is bound to the `PUT /v2/service_instances/:instance_id/service_bindings/:binding_id` endpoint and can be called using the `cf bind-service` command. func (broker *ServiceBroker) Bind(ctx context.Context, instanceID, bindingID string, details domain.BindDetails, _ bool) (domain.Binding, error) { diff --git a/brokerapi/broker/errors.go b/brokerapi/broker/errors.go new file mode 100644 index 000000000..934a40da2 --- /dev/null +++ b/brokerapi/broker/errors.go @@ -0,0 +1,30 @@ +package broker + +import ( + "errors" + "net/http" + + "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" +) + +// typed errors which are not declared in pivotal-cf/brokerapi + +var ( + badRequestMsg = "bad request" + invalidUserInputMsg = "User supplied parameters must be in the form of a valid JSON map." + nonUpdateableParameterMsg = "attempt to update parameter that may result in service instance re-creation and data loss" + notFoundMsg = "not found" + concurrencyErrorMsg = "ConcurrencyError" + + badRequestKey = "bad-request" + invalidUserInputKey = "parsing-user-request" + nonUpdatableParameterKey = "prohibited" + notFoundKey = "not-found" + concurrencyErrorKey = "concurrency-error" + + ErrBadRequest = apiresponses.NewFailureResponse(errors.New(badRequestMsg), http.StatusBadRequest, badRequestKey) + ErrInvalidUserInput = apiresponses.NewFailureResponse(errors.New(invalidUserInputMsg), http.StatusBadRequest, invalidUserInputKey) + ErrNonUpdatableParameter = apiresponses.NewFailureResponse(errors.New(nonUpdateableParameterMsg), http.StatusBadRequest, nonUpdatableParameterKey) + ErrNotFound = apiresponses.NewFailureResponse(errors.New(notFoundMsg), http.StatusNotFound, notFoundKey) + ErrConcurrencyError = apiresponses.NewFailureResponse(errors.New(concurrencyErrorMsg), http.StatusUnprocessableEntity, concurrencyErrorKey) +) diff --git a/brokerapi/broker/get_binding.go b/brokerapi/broker/get_binding.go index f8d66a33d..047bf254e 100644 --- a/brokerapi/broker/get_binding.go +++ b/brokerapi/broker/get_binding.go @@ -2,28 +2,83 @@ package broker import ( "context" - "errors" - "net/http" + "fmt" "code.cloudfoundry.org/lager/v3" "github.com/pivotal-cf/brokerapi/v11/domain" - "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" "github.com/cloudfoundry/cloud-service-broker/v2/utils/correlation" ) // GetBinding fetches an existing service binding. // GET /v2/service_instances/{instance_id}/service_bindings/{binding_id} -// -// NOTE: This functionality is not implemented. -func (broker *ServiceBroker) GetBinding(ctx context.Context, instanceID, bindingID string, _ domain.FetchBindingDetails) (domain.GetBindingSpec, error) { +func (broker *ServiceBroker) GetBinding(ctx context.Context, instanceID, bindingID string, details domain.FetchBindingDetails) (domain.GetBindingSpec, error) { broker.Logger.Info("GetBinding", correlation.ID(ctx), lager.Data{ "instance_id": instanceID, "binding_id": bindingID, + "service_id": details.ServiceID, + "plan_id": details.PlanID, }) - return domain.GetBindingSpec{}, apiresponses.NewFailureResponse( - errors.New("the service_bindings endpoint is unsupported"), - http.StatusBadRequest, - "unsupported") + // check whether instance exists + instanceExists, err := broker.store.ExistsServiceInstanceDetails(instanceID) + if err != nil { + return domain.GetBindingSpec{}, fmt.Errorf("error checking for existing instance: %w", err) + } + if !instanceExists { + return domain.GetBindingSpec{}, ErrNotFound + } + + // get instance details + instanceRecord, err := broker.store.GetServiceInstanceDetails(instanceID) + if err != nil { + return domain.GetBindingSpec{}, fmt.Errorf("error retrieving service instance details: %w", err) + } + + // check whether request parameters (if not empty) match instance details + if len(details.ServiceID) > 0 && details.ServiceID != instanceRecord.ServiceGUID { + return domain.GetBindingSpec{}, ErrNotFound + } + if len(details.PlanID) > 0 && details.PlanID != instanceRecord.PlanGUID { + return domain.GetBindingSpec{}, ErrNotFound + } + + // check whether service plan is bindable + serviceDefinition, _, err := broker.getDefinitionAndProvider(instanceRecord.ServiceGUID) + if err != nil { + return domain.GetBindingSpec{}, fmt.Errorf("error retrieving service definition: %w", err) + } + if !serviceDefinition.Bindable { + return domain.GetBindingSpec{}, ErrBadRequest + } + + // check whether binding exists + // with the current implementation, bind is a synchroneous operation which waits for all resources to be created before binding credentials are stored + // therefore, we can assume the binding operation is completed if it exists at the store + bindingExists, err := broker.store.ExistsServiceBindingCredentials(bindingID, instanceID) + if err != nil { + return domain.GetBindingSpec{}, fmt.Errorf("error checking for existing binding: %w", err) + } + if !bindingExists { + return domain.GetBindingSpec{}, ErrNotFound + } + + // get binding parameters + params, err := broker.store.GetBindRequestDetails(bindingID, instanceID) + if err != nil { + return domain.GetBindingSpec{}, fmt.Errorf("error retrieving bind request details: %w", err) + } + + // broker does not support Log Drain, Route Services, or Volume Mounts + // broker does not support binding metadata + // credentials are returned with synchronous bind request + return domain.GetBindingSpec{ + Credentials: nil, + SyslogDrainURL: "", + RouteServiceURL: "", + VolumeMounts: nil, + Parameters: params, + Endpoints: nil, + Metadata: domain.BindingMetadata{}, + }, nil } diff --git a/brokerapi/broker/get_binding_test.go b/brokerapi/broker/get_binding_test.go index b0ba5f7e7..141055364 100644 --- a/brokerapi/broker/get_binding_test.go +++ b/brokerapi/broker/get_binding_test.go @@ -1,23 +1,361 @@ package broker_test import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/pivotal-cf/brokerapi/v11/domain" - "golang.org/x/net/context" + "context" + "errors" + "fmt" + "net/http" + "code.cloudfoundry.org/lager/v3" "github.com/cloudfoundry/cloud-service-broker/v2/brokerapi/broker" "github.com/cloudfoundry/cloud-service-broker/v2/brokerapi/broker/brokerfakes" + "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" + pkgBroker "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker" + pkgBrokerFakes "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker/brokerfakes" "github.com/cloudfoundry/cloud-service-broker/v2/utils" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/pivotal-cf/brokerapi/v11/domain" + "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" ) var _ = Describe("GetBinding", func() { - It("is not implemented", func() { - serviceBroker, err := broker.New(&broker.BrokerConfig{}, &brokerfakes.FakeStorage{}, utils.NewLogger("brokers-test")) - Expect(err).ToNot(HaveOccurred()) + const ( + orgID = "test-org-id" + spaceID = "test-space-id" + planID = "test-plan-id" + offeringID = "test-service-id" + instanceID = "test-instance-id" + bindingID = "test-binding-id" + ) + + var ( + serviceBroker *broker.ServiceBroker + + fakeStorage *brokerfakes.FakeStorage + fakeServiceProvider *pkgBrokerFakes.FakeServiceProvider + + brokerConfig *broker.BrokerConfig + + bindingParams *storage.JSONObject + ) + + BeforeEach(func() { + fakeStorage = &brokerfakes.FakeStorage{} + fakeServiceProvider = &pkgBrokerFakes.FakeServiceProvider{} + + providerBuilder := func(logger lager.Logger, store pkgBroker.ServiceProviderStorage) pkgBroker.ServiceProvider { + return fakeServiceProvider + } + planUpdatable := true + + brokerConfig = &broker.BrokerConfig{ + Registry: pkgBroker.BrokerRegistry{ + "test-service": &pkgBroker.ServiceDefinition{ + ID: offeringID, + Name: "test-service", + Bindable: true, + Plans: []pkgBroker.ServicePlan{ + { + ServicePlan: domain.ServicePlan{ + ID: planID, + Name: "test-plan", + PlanUpdatable: &planUpdatable, + }, + }, + }, + ProviderBuilder: providerBuilder, + }, + }, + } + + serviceBroker = must(broker.New(brokerConfig, fakeStorage, utils.NewLogger("get-binding-test"))) + + bindingParams = &storage.JSONObject{ + "param1": "value1", + "param2": 3, + "param3": true, + "param4": []string{"a", "b", "c"}, + "param5": map[string]string{"key1": "value", "key2": "value"}, + "param6": struct { + A string + B string + }{"a", "b"}, + } + + fakeStorage.ExistsServiceInstanceDetailsReturns(true, nil) + fakeStorage.GetServiceInstanceDetailsReturns( + storage.ServiceInstanceDetails{ + GUID: instanceID, + Name: "test-instance", + Outputs: storage.JSONObject{}, + ServiceGUID: offeringID, + PlanGUID: planID, + SpaceGUID: spaceID, + OrganizationGUID: orgID, + }, nil) + fakeStorage.ExistsServiceBindingCredentialsReturns(true, nil) + fakeStorage.GetBindRequestDetailsReturns(*bindingParams, nil) + }) + + When("binding exsists", func() { + It("returns binding details and parameters", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).ToNot(HaveOccurred()) + + By("validating response") + Expect(response.SyslogDrainURL).To(BeZero()) + Expect(response.RouteServiceURL).To(BeZero()) + Expect(response.VolumeMounts).To(BeZero()) + Expect(response.Parameters).To(BeEquivalentTo(*bindingParams)) + Expect(response.Endpoints).To(BeZero()) + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked whether binding exists") + Expect(fakeStorage.ExistsServiceBindingCredentialsCallCount()).To(Equal(1)) + + By("validating storage is asked for bind request details") + Expect(fakeStorage.GetBindRequestDetailsCallCount()).To(Equal(1)) + }) + It("does not return binding credentials", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).ToNot(HaveOccurred()) + + By("validating response") + Expect(response.Credentials).To(BeZero()) + + By("validating storage is not asked for binding credentials") + Expect(fakeStorage.GetServiceBindingCredentialsCallCount()).To(Equal(0)) + }) + It("does not return binding metadata", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).ToNot(HaveOccurred()) + + By("validating response") + Expect(response.Metadata).To(BeZero()) + }) + }) + + When("service is not bindable", func() { + BeforeEach(func() { + brokerConfig.Registry["test-service"].Bindable = false + }) + It("returns status code 400 (bad request)", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) + Expect(apiErr.Error()).To(Equal("bad request")) + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusBadRequest)) + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is not asked whether binding exists") + Expect(fakeStorage.ExistsServiceBindingCredentialsCallCount()).To(Equal(0)) + + By("validating storage is not asked for bind request details") + Expect(fakeStorage.GetBindRequestDetailsCallCount()).To(Equal(0)) + }) + }) + + When("instance does not exist", func() { + BeforeEach(func() { + fakeStorage.ExistsServiceInstanceDetailsReturns(false, nil) + }) + It("returns status code 404 (not found)", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "Not Found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 - _, err = serviceBroker.GetBinding(context.TODO(), "instance-id", "binding-id", domain.FetchBindingDetails{}) + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) - Expect(err).To(MatchError("the service_bindings endpoint is unsupported")) + By("validating storage is not asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(0)) + + By("validating storage is not asked whether binding exists") + Expect(fakeStorage.ExistsServiceBindingCredentialsCallCount()).To(Equal(0)) + + By("validating storage is not asked for bind request details") + Expect(fakeStorage.GetBindRequestDetailsCallCount()).To(Equal(0)) + }) + }) + + When("binding does not exist", func() { + BeforeEach(func() { + fakeStorage.ExistsServiceBindingCredentialsReturns(false, nil) + }) + It("returns status code 404 (not found)", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "Not Found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked whether binding exists") + Expect(fakeStorage.ExistsServiceBindingCredentialsCallCount()).To(Equal(1)) + + By("validating storage is not asked for bind request details") + Expect(fakeStorage.GetBindRequestDetailsCallCount()).To(Equal(0)) + }) + }) + + // with the current implementation, bind is a synchroneous operation which waits for all resources to be created + // bindings are stored only in case resource creation is successfull + // -> it is not possible to retrieve a binding that is in progress from the store + // When("binding operation is in progress", func() { + // It("returns status code 404 (not found)", func() { + // Fail("not implemented") + // }) + // }) + + When("service_id is not set", func() { + It("ignores service_id and returns binding details", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{PlanID: planID}) + Expect(err).ToNot(HaveOccurred()) + + By("validating response") + Expect(response.Parameters).To(BeEquivalentTo(*bindingParams)) + }) + }) + + When("service_id does not match service for binding", func() { + It("returns status code 404 (not found)", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: "otherService", PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "Not Found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is not asked whether binding exists") + Expect(fakeStorage.ExistsServiceBindingCredentialsCallCount()).To(Equal(0)) + + By("validating storage is not asked for bind request details") + Expect(fakeStorage.GetBindRequestDetailsCallCount()).To(Equal(0)) + }) + }) + + When("plan_id is not set", func() { + It("ignores plan_id and returns binding details", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID}) + Expect(err).ToNot(HaveOccurred()) + + By("validating response") + Expect(response.Parameters).To(BeEquivalentTo(*bindingParams)) + }) + }) + + When("plan_id does not match plan for binding", func() { + It("returns status code 404 (not found)", func() { + response, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: "otherPlan"}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "Not Found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is not asked whether binding exists") + Expect(fakeStorage.ExistsServiceBindingCredentialsCallCount()).To(Equal(0)) + + By("validating storage is not asked for bind request details") + Expect(fakeStorage.GetBindRequestDetailsCallCount()).To(Equal(0)) + }) + }) + When("fails to check instance existence", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeStorage.ExistsServiceInstanceDetailsReturns(false, errors.New(msg)) + }) + It("returns error", func() { + _, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).To(MatchError(fmt.Sprintf(`error checking for existing instance: %s`, msg))) + }) + }) + When("fails to retrieve instance details", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeStorage.GetServiceInstanceDetailsReturns(storage.ServiceInstanceDetails{}, errors.New(msg)) + }) + It("returns error", func() { + _, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).To(MatchError(fmt.Sprintf(`error retrieving service instance details: %s`, msg))) + }) + }) + When("fails to check binding existence", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeStorage.ExistsServiceBindingCredentialsReturns(false, errors.New(msg)) + }) + It("returns error", func() { + _, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).To(MatchError(fmt.Sprintf(`error checking for existing binding: %s`, msg))) + }) + }) + When("fails to retrieve bind request details", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeStorage.GetBindRequestDetailsReturns(storage.JSONObject{}, errors.New(msg)) + }) + It("returns error", func() { + _, err := serviceBroker.GetBinding(context.TODO(), instanceID, bindingID, domain.FetchBindingDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).To(MatchError(fmt.Sprintf(`error retrieving bind request details: %s`, msg))) + }) }) }) diff --git a/brokerapi/broker/get_instance.go b/brokerapi/broker/get_instance.go index 3f5487991..e9f4a545f 100644 --- a/brokerapi/broker/get_instance.go +++ b/brokerapi/broker/get_instance.go @@ -2,27 +2,80 @@ package broker import ( "context" - "errors" - "net/http" + "fmt" "code.cloudfoundry.org/lager/v3" "github.com/pivotal-cf/brokerapi/v11/domain" - "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" + "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/utils/correlation" ) // GetInstance fetches information about a service instance // GET /v2/service_instances/{instance_id} -// -// NOTE: This functionality is not implemented. -func (broker *ServiceBroker) GetInstance(ctx context.Context, instanceID string, _ domain.FetchInstanceDetails) (domain.GetInstanceDetailsSpec, error) { +func (broker *ServiceBroker) GetInstance(ctx context.Context, instanceID string, details domain.FetchInstanceDetails) (domain.GetInstanceDetailsSpec, error) { broker.Logger.Info("GetInstance", correlation.ID(ctx), lager.Data{ "instance_id": instanceID, + "service_id": details.ServiceID, + "plan_id": details.PlanID, }) - return domain.GetInstanceDetailsSpec{}, apiresponses.NewFailureResponse( - errors.New("the service_instances endpoint is unsupported"), - http.StatusBadRequest, - "unsupported") + // check whether instance exists + exists, err := broker.store.ExistsServiceInstanceDetails(instanceID) + if err != nil { + return domain.GetInstanceDetailsSpec{}, fmt.Errorf("error checking for existing instance: %w", err) + } + if !exists { + return domain.GetInstanceDetailsSpec{}, ErrNotFound + } + + // get instance details + instanceRecord, err := broker.store.GetServiceInstanceDetails(instanceID) + if err != nil { + return domain.GetInstanceDetailsSpec{}, fmt.Errorf("error retrieving service instance details: %w", err) + } + + // check whether request parameters (if not empty) match instance details + if len(details.ServiceID) > 0 && details.ServiceID != instanceRecord.ServiceGUID { + return domain.GetInstanceDetailsSpec{}, ErrNotFound + } + if len(details.PlanID) > 0 && details.PlanID != instanceRecord.PlanGUID { + return domain.GetInstanceDetailsSpec{}, ErrNotFound + } + + // get instance status + _, serviceProvider, err := broker.getDefinitionAndProvider(instanceRecord.ServiceGUID) + if err != nil { + return domain.GetInstanceDetailsSpec{}, fmt.Errorf("error retrieving service definition: %w", err) + } + + done, _, lastOperationType, err := serviceProvider.PollInstance(ctx, instanceRecord.GUID) + if err != nil { + return domain.GetInstanceDetailsSpec{}, fmt.Errorf("error polling instance status: %w", err) + } + + switch lastOperationType { + case models.ProvisionOperationType: + if !done { + return domain.GetInstanceDetailsSpec{}, ErrNotFound + } + case models.UpdateOperationType: + if !done { + return domain.GetInstanceDetailsSpec{}, ErrConcurrencyError + } + } + + // get provision parameters + params, err := broker.store.GetProvisionRequestDetails(instanceID) + if err != nil { + return domain.GetInstanceDetailsSpec{}, fmt.Errorf("error retrieving provision request details: %w", err) + } + + return domain.GetInstanceDetailsSpec{ + ServiceID: instanceRecord.ServiceGUID, + PlanID: instanceRecord.PlanGUID, + DashboardURL: "", + Parameters: params, + Metadata: domain.InstanceMetadata{}, + }, nil } diff --git a/brokerapi/broker/get_instance_test.go b/brokerapi/broker/get_instance_test.go index b5bc744fb..9541610d2 100644 --- a/brokerapi/broker/get_instance_test.go +++ b/brokerapi/broker/get_instance_test.go @@ -1,24 +1,340 @@ package broker_test import ( + "errors" + "fmt" + "net/http" + + "code.cloudfoundry.org/lager/v3" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/pivotal-cf/brokerapi/v11/domain" + "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" "golang.org/x/net/context" "github.com/cloudfoundry/cloud-service-broker/v2/brokerapi/broker/brokerfakes" + "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" + "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" + pkgBroker "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker" + pkgBrokerFakes "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker/brokerfakes" "github.com/cloudfoundry/cloud-service-broker/v2/utils" "github.com/cloudfoundry/cloud-service-broker/v2/brokerapi/broker" ) var _ = Describe("GetInstance", func() { - It("is not implemented", func() { - serviceBroker, err := broker.New(&broker.BrokerConfig{}, &brokerfakes.FakeStorage{}, utils.NewLogger("brokers-test")) - Expect(err).ToNot(HaveOccurred()) - _, err = serviceBroker.GetInstance(context.TODO(), "instance-id", domain.FetchInstanceDetails{}) + const ( + orgID = "test-org-id" + spaceID = "test-space-id" + planID = "test-plan-id" + offeringID = "test-service-id" + instanceID = "test-instance-id" + ) + + var ( + serviceBroker *broker.ServiceBroker + + fakeStorage *brokerfakes.FakeStorage + fakeServiceProvider *pkgBrokerFakes.FakeServiceProvider + + brokerConfig *broker.BrokerConfig + + provisionParams *storage.JSONObject + ) + + BeforeEach(func() { + fakeStorage = &brokerfakes.FakeStorage{} + fakeServiceProvider = &pkgBrokerFakes.FakeServiceProvider{} + + providerBuilder := func(logger lager.Logger, store pkgBroker.ServiceProviderStorage) pkgBroker.ServiceProvider { + return fakeServiceProvider + } + planUpdatable := true + + brokerConfig = &broker.BrokerConfig{ + Registry: pkgBroker.BrokerRegistry{ + "test-service": &pkgBroker.ServiceDefinition{ + ID: offeringID, + Name: "test-service", + Plans: []pkgBroker.ServicePlan{ + { + ServicePlan: domain.ServicePlan{ + ID: planID, + Name: "test-plan", + PlanUpdatable: &planUpdatable, + }, + }, + }, + ProviderBuilder: providerBuilder, + }, + }, + } + + serviceBroker = must(broker.New(brokerConfig, fakeStorage, utils.NewLogger("get-instance-test"))) + + provisionParams = &storage.JSONObject{ + "param1": "value1", + "param2": 3, + "param3": true, + "param4": []string{"a", "b", "c"}, + "param5": map[string]string{"key1": "value", "key2": "value"}, + "param6": struct { + A string + B string + }{"a", "b"}, + } + + fakeStorage.ExistsServiceInstanceDetailsReturns(true, nil) + fakeStorage.GetServiceInstanceDetailsReturns( + storage.ServiceInstanceDetails{ + GUID: instanceID, + Name: "test-instance", + Outputs: storage.JSONObject{}, + ServiceGUID: offeringID, + PlanGUID: planID, + SpaceGUID: spaceID, + OrganizationGUID: orgID, + }, nil) + fakeServiceProvider.PollInstanceReturns(true, "", models.ProvisionOperationType, nil) // Operation status is provision succeeded + fakeStorage.GetProvisionRequestDetailsReturns(*provisionParams, nil) + }) + + When("instance exists and provision succeeded", func() { + It("returns instance details", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).ToNot(HaveOccurred()) + + By("validating response") + Expect(response.ServiceID).To(Equal(offeringID)) + Expect(response.PlanID).To(Equal(planID)) + Expect(response.Parameters).To(BeEquivalentTo(*provisionParams)) + Expect(response.DashboardURL).To(BeEmpty()) // Broker does not set dashboard URL + Expect(response.Metadata).To(BeZero()) // Broker does not support instance metadata + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating service provider asked for instance status") + Expect(fakeServiceProvider.PollInstanceCallCount()).To(Equal(1)) + + By("validating storage is asked for provision request details") + Expect(fakeStorage.GetProvisionRequestDetailsCallCount()).To(Equal(1)) + }) + }) + + When("instance does not exist", func() { + BeforeEach(func() { + fakeStorage.ExistsServiceInstanceDetailsReturns(false, nil) + }) + It("returns status code 404 (not found)", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "Not Found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is not asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(0)) + + By("validating service provider is not asked for instance status") + Expect(fakeServiceProvider.PollInstanceCallCount()).To(Equal(0)) + + By("validating storage is not asked for provision request details") + Expect(fakeStorage.GetProvisionRequestDetailsCallCount()).To(Equal(0)) + }) + }) + + When("instance exists and provision is in progress", func() { + BeforeEach(func() { + fakeServiceProvider.PollInstanceReturns(false, "", models.ProvisionOperationType, nil) // Operation status is provision in progress + }) + // According to OSB Spec, broker must return 404 in case provision is in progress + It("returns status code 404 (not found)", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "not found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating service provider is asked for instance status") + Expect(fakeServiceProvider.PollInstanceCallCount()).To(Equal(1)) + + By("validating storage is not asked for provision request details") + Expect(fakeStorage.GetProvisionRequestDetailsCallCount()).To(Equal(0)) + }) - Expect(err).To(MatchError("the service_instances endpoint is unsupported")) + }) + + When("instance exists and update is in progress", func() { + BeforeEach(func() { + fakeServiceProvider.PollInstanceReturns(false, "", models.UpdateOperationType, nil) // Operation status is update in progress + }) + It("returns status code 422 (Unprocessable Entity) and error code ConcurrencyError", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("ConcurrencyError")) // must contain "ConcurrencyError" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusUnprocessableEntity)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating service provider is asked for instance status") + Expect(fakeServiceProvider.PollInstanceCallCount()).To(Equal(1)) + + By("validating storage is not asked for provision request details") + Expect(fakeStorage.GetProvisionRequestDetailsCallCount()).To(Equal(0)) + }) + + }) + + When("service_id is not set", func() { + It("ignores service_id and returns instance details", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{PlanID: planID}) + Expect(err).ToNot(HaveOccurred()) + Expect(response.ServiceID).To(Equal(offeringID)) + }) + }) + + When("service_id does not match service for instance", func() { + It("returns 404 (not found)", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: "otherService", PlanID: planID}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "not found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating service provider is asked for instance status") + Expect(fakeServiceProvider.PollInstanceCallCount()).To(Equal(0)) + + By("validating storage is not asked for provision request details") + Expect(fakeStorage.GetProvisionRequestDetailsCallCount()).To(Equal(0)) + }) + }) + + When("plan_id is not set", func() { + It("ignores plan_id and returns instance details", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID}) + Expect(err).ToNot(HaveOccurred()) + Expect(response.ServiceID).To(Equal(offeringID)) + }) + }) + + When("plan_id does not match plan for instance", func() { + It("returns 404 (not found)", func() { + response, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: "otherPlan"}) + + By("validating response") + Expect(response).To(BeZero()) + + By("validating error") + apiErr, isFailureResponse := err.(*apiresponses.FailureResponse) + Expect(isFailureResponse).To(BeTrue()) // must be a failure response + Expect(apiErr.Error()).To(Equal("not found")) // must contain "not found" error message + Expect(apiErr.ValidatedStatusCode(nil)).To(Equal(http.StatusNotFound)) // status code must be 404 + + By("validating storage is asked whether instance exists") + Expect(fakeStorage.ExistsServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating storage is asked for instance details") + Expect(fakeStorage.GetServiceInstanceDetailsCallCount()).To(Equal(1)) + + By("validating service provider is asked for instance status") + Expect(fakeServiceProvider.PollInstanceCallCount()).To(Equal(0)) + + By("validating storage is not asked for provision request details") + Expect(fakeStorage.GetProvisionRequestDetailsCallCount()).To(Equal(0)) + }) + }) + + When("fails to check for existing instance", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeStorage.ExistsServiceInstanceDetailsReturns(false, errors.New(msg)) + }) + It("should error", func() { + _, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: "otherPlan"}) + Expect(err).To(MatchError(fmt.Sprintf(`error checking for existing instance: %s`, msg))) + }) + }) + When("fails to retrieve service instance details", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeStorage.GetServiceInstanceDetailsReturns(storage.ServiceInstanceDetails{}, errors.New(msg)) + }) + It("should error", func() { + _, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: "otherPlan"}) + Expect(err).To(MatchError(fmt.Sprintf(`error retrieving service instance details: %s`, msg))) + }) + }) + When("fails to poll instance status", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeServiceProvider.PollInstanceReturns(false, "", "", errors.New(msg)) + }) + It("should error", func() { + _, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).To(MatchError(fmt.Sprintf(`error polling instance status: %s`, msg))) + }) + }) + When("fails to retrieve provision request details", func() { + const ( + msg = "error-msg" + ) + BeforeEach(func() { + fakeStorage.GetProvisionRequestDetailsReturns(nil, errors.New(msg)) + }) + It("should error", func() { + _, err := serviceBroker.GetInstance(context.TODO(), instanceID, domain.FetchInstanceDetails{ServiceID: offeringID, PlanID: planID}) + Expect(err).To(MatchError(fmt.Sprintf(`error retrieving provision request details: %s`, msg))) + }) }) }) diff --git a/brokerapi/broker/services_test.go b/brokerapi/broker/services_test.go index 57e22c15e..a6cf8e9b6 100644 --- a/brokerapi/broker/services_test.go +++ b/brokerapi/broker/services_test.go @@ -43,6 +43,7 @@ var _ = Describe("Services", func() { ID: "second-service-id", Name: "second-service", ProviderDisplayName: "company-name-2", + Bindable: true, Plans: []pkgBroker.ServicePlan{ { ServicePlan: domain.ServicePlan{ @@ -68,6 +69,9 @@ var _ = Describe("Services", func() { Expect(servicesList[0].ID).To(Equal("first-service-id")) Expect(servicesList[0].Name).To(Equal("first-service")) Expect(servicesList[0].Metadata.ProviderDisplayName).To(Equal("company-name-1")) + Expect(servicesList[0].InstancesRetrievable).To(BeTrue()) + Expect(servicesList[0].Bindable).To(BeFalse()) + Expect(servicesList[0].BindingsRetrievable).To(BeFalse()) Expect(len(servicesList[0].Plans)).To(Equal(2)) Expect(servicesList[0].Plans[0].ID).To(Equal("plan-1")) Expect(servicesList[0].Plans[0].Name).To(Equal("test-plan-1")) @@ -77,6 +81,9 @@ var _ = Describe("Services", func() { Expect(servicesList[1].ID).To(Equal("second-service-id")) Expect(servicesList[1].Name).To(Equal("second-service")) Expect(servicesList[1].Metadata.ProviderDisplayName).To(Equal("company-name-2")) + Expect(servicesList[1].InstancesRetrievable).To(BeTrue()) + Expect(servicesList[1].Bindable).To(BeTrue()) + Expect(servicesList[1].BindingsRetrievable).To(BeTrue()) Expect(len(servicesList[1].Plans)).To(Equal(1)) Expect(servicesList[1].Plans[0].ID).To(Equal("plan-3")) Expect(servicesList[1].Plans[0].Name).To(Equal("test-plan-3")) diff --git a/brokerapi/broker/update.go b/brokerapi/broker/update.go index fcbab5fc0..976981f16 100644 --- a/brokerapi/broker/update.go +++ b/brokerapi/broker/update.go @@ -2,9 +2,7 @@ package broker import ( "context" - "errors" "fmt" - "net/http" "code.cloudfoundry.org/lager/v3" "github.com/hashicorp/go-version" @@ -21,8 +19,6 @@ import ( "github.com/cloudfoundry/cloud-service-broker/v2/utils/request" ) -var ErrNonUpdatableParameter = apiresponses.NewFailureResponse(errors.New("attempt to update parameter that may result in service instance re-creation and data loss"), http.StatusBadRequest, "prohibited") - // Update a service instance plan. // This functionality is not implemented and will return an error indicating that plan changes are not supported. func (broker *ServiceBroker) Update(ctx context.Context, instanceID string, details domain.UpdateDetails, asyncAllowed bool) (domain.UpdateServiceSpec, error) { diff --git a/pkg/broker/service_definition.go b/pkg/broker/service_definition.go index ee251f68c..2157101bf 100644 --- a/pkg/broker/service_definition.go +++ b/pkg/broker/service_definition.go @@ -247,9 +247,11 @@ func (svc *ServiceDefinition) CatalogEntry() *Service { ImageUrl: svc.ImageURL, SupportUrl: svc.SupportURL, }, - Tags: svc.Tags, - Bindable: svc.Bindable, - PlanUpdatable: svc.PlanUpdateable, + Tags: svc.Tags, + Bindable: svc.Bindable, + BindingsRetrievable: svc.Bindable, + PlanUpdatable: svc.PlanUpdateable, + InstancesRetrievable: true, }, Plans: svc.Plans, } diff --git a/pkg/broker/service_definition_test.go b/pkg/broker/service_definition_test.go index ec99bb086..52d35ef29 100644 --- a/pkg/broker/service_definition_test.go +++ b/pkg/broker/service_definition_test.go @@ -17,38 +17,66 @@ import ( var _ = Describe("ServiceDefinition", func() { Describe("CatalogEntry", func() { - Context("Metadata", func() { - var serviceDefinition broker.ServiceDefinition + var serviceDefinition broker.ServiceDefinition + + BeforeEach(func() { + serviceDefinition = broker.ServiceDefinition{ + ID: "fa6334bc-5314-4b63-8a74-c0e4b638c950", + Name: "test-def", + Description: "test-def-desc", + DisplayName: "test-def-display-name", + ImageURL: "image-url", + DocumentationURL: "docs-url", + ProviderDisplayName: "provider-display-name", + SupportURL: "support-url", + Tags: []string{"Beta", "Tag"}, + PlanUpdateable: true, + Plans: nil, + } + }) + It("includes all metadata in the catalog", func() { + catalogEntry := serviceDefinition.CatalogEntry() + Expect(catalogEntry.Name).To(Equal("test-def")) + Expect(catalogEntry.Description).To(Equal("test-def-desc")) + Expect(catalogEntry.Metadata.DisplayName).To(Equal("test-def-display-name")) + Expect(catalogEntry.Metadata.ImageUrl).To(Equal("image-url")) + Expect(catalogEntry.Metadata.LongDescription).To(Equal("test-def-desc")) + Expect(catalogEntry.Metadata.DocumentationUrl).To(Equal("docs-url")) + Expect(catalogEntry.Metadata.ProviderDisplayName).To(Equal("provider-display-name")) + Expect(catalogEntry.Metadata.SupportUrl).To(Equal("support-url")) + Expect(catalogEntry.Tags).To(ConsistOf("Beta", "Tag")) + }) + + It("includes instances_retrievable: true", func() { + catalogEntry := serviceDefinition.CatalogEntry() + Expect(catalogEntry.InstancesRetrievable).To(BeTrue()) + }) + + When("service offering is not bindable", func() { BeforeEach(func() { - serviceDefinition = broker.ServiceDefinition{ - ID: "fa6334bc-5314-4b63-8a74-c0e4b638c950", - Name: "test-def", - Description: "test-def-desc", - DisplayName: "test-def-display-name", - ImageURL: "image-url", - DocumentationURL: "docs-url", - ProviderDisplayName: "provider-display-name", - SupportURL: "support-url", - Tags: []string{"Beta", "Tag"}, - Bindable: true, - PlanUpdateable: true, - Plans: nil, - } + serviceDefinition.Bindable = false }) - It("includes all metadata in the catalog", func() { + It("includes bindable: false", func() { catalogEntry := serviceDefinition.CatalogEntry() - - Expect(catalogEntry.Name).To(Equal("test-def")) - Expect(catalogEntry.Description).To(Equal("test-def-desc")) - Expect(catalogEntry.Metadata.DisplayName).To(Equal("test-def-display-name")) - Expect(catalogEntry.Metadata.ImageUrl).To(Equal("image-url")) - Expect(catalogEntry.Metadata.LongDescription).To(Equal("test-def-desc")) - Expect(catalogEntry.Metadata.DocumentationUrl).To(Equal("docs-url")) - Expect(catalogEntry.Metadata.ProviderDisplayName).To(Equal("provider-display-name")) - Expect(catalogEntry.Metadata.SupportUrl).To(Equal("support-url")) - Expect(catalogEntry.Tags).To(ConsistOf("Beta", "Tag")) - + Expect(catalogEntry.Bindable).To(BeFalse()) + }) + It("includes binding_retrievable: false", func() { + catalogEntry := serviceDefinition.CatalogEntry() + Expect(catalogEntry.BindingsRetrievable).To(BeFalse()) + }) + }) + When("service offering is bindable", func() { + BeforeEach(func() { + serviceDefinition.Bindable = true + }) + It("includes bindable: true", func() { + catalogEntry := serviceDefinition.CatalogEntry() + Expect(catalogEntry.Bindable).To(BeTrue()) + }) + It("includes binding_retrievable: true", func() { + catalogEntry := serviceDefinition.CatalogEntry() + Expect(catalogEntry.BindingsRetrievable).To(BeTrue()) }) })