diff --git a/brokerapi/brokers/account_managers/service_account_manager.go b/brokerapi/brokers/account_managers/service_account_manager.go index d714e7bc6..c9d9b1ded 100644 --- a/brokerapi/brokers/account_managers/service_account_manager.go +++ b/brokerapi/brokers/account_managers/service_account_manager.go @@ -144,8 +144,16 @@ func (sam *ServiceAccountManager) DeleteCredentials(binding models.ServiceBindin } func (b *ServiceAccountManager) BuildInstanceCredentials(bindRecord models.ServiceBindingCredentials, instanceRecord models.ServiceInstanceDetails) (map[string]string, error) { - bindDetails := bindRecord.GetOtherDetails() - instanceDetails := instanceRecord.GetOtherDetails() + bindDetails, err := bindRecord.GetOtherDetails() + if err != nil { + return nil, err + } + + instanceDetails, err := instanceRecord.GetOtherDetails() + if err != nil { + return nil, err + } + return utils.MergeStringMaps(bindDetails, instanceDetails), nil } diff --git a/brokerapi/brokers/account_managers/sql_account_manager.go b/brokerapi/brokers/account_managers/sql_account_manager.go index 7edb3183f..fe560a2c3 100644 --- a/brokerapi/brokers/account_managers/sql_account_manager.go +++ b/brokerapi/brokers/account_managers/sql_account_manager.go @@ -180,8 +180,15 @@ func (sam *SqlAccountManager) pollOperationUntilDone(op *googlecloudsql.Operatio } func (b *SqlAccountManager) BuildInstanceCredentials(bindRecord models.ServiceBindingCredentials, instanceRecord models.ServiceInstanceDetails) (map[string]string, error) { - instanceDetails := instanceRecord.GetOtherDetails() - bindDetails := bindRecord.GetOtherDetails() + instanceDetails, err := instanceRecord.GetOtherDetails() + if err != nil { + return nil, err + } + + bindDetails, err := bindRecord.GetOtherDetails() + if err != nil { + return nil, err + } service, err := broker.GetServiceById(instanceRecord.ServiceId) if err != nil { diff --git a/brokerapi/brokers/broker_base/broker_base.go b/brokerapi/brokers/broker_base/broker_base.go index ae49e2ca5..bf942b9d8 100644 --- a/brokerapi/brokers/broker_base/broker_base.go +++ b/brokerapi/brokers/broker_base/broker_base.go @@ -15,6 +15,8 @@ package broker_base import ( + "errors" + "code.cloudfoundry.org/lager" "github.com/GoogleCloudPlatform/gcp-service-broker/brokerapi/brokers/models" @@ -68,5 +70,5 @@ func (b *BrokerBase) DeprovisionsAsync() bool { // determine if the workflow is a provision or deprovision flow based off the // type of the most recent operation. func (b *BrokerBase) LastOperationWasDelete(instanceId string) (bool, error) { - panic("Can't check last operation on a synchronous service") + return false, errors.New("can't check last operation on a synchronous service") } diff --git a/brokerapi/brokers/gcp_service_broker.go b/brokerapi/brokers/gcp_service_broker.go index 95170a0f2..c12d8bd1e 100755 --- a/brokerapi/brokers/gcp_service_broker.go +++ b/brokerapi/brokers/gcp_service_broker.go @@ -396,7 +396,7 @@ func (gcpBroker *GCPServiceBroker) lastOperationAsync(instanceId string, service // no error and we're done! Delete from the SB database if this was a delete flow and return success deleteFlow, err := service.LastOperationWasDelete(instanceId) if err != nil { - return brokerapi.LastOperation{State: brokerapi.Succeeded}, fmt.Errorf("Couldn't determine if provision or deprovision flow, this may leave orphaned resources, contact your operator for cleanup") + return brokerapi.LastOperation{State: brokerapi.Succeeded}, fmt.Errorf("Couldn't determine if provision or deprovision flow, this may leave orphaned resources, contact your operator for cleanup: %s", err) } if deleteFlow { err = db_service.DeleteServiceInstanceDetailsById(instanceId) diff --git a/brokerapi/brokers/models/db.go b/brokerapi/brokers/models/db.go index 66302390d..bc984f1f3 100644 --- a/brokerapi/brokers/models/db.go +++ b/brokerapi/brokers/models/db.go @@ -23,31 +23,26 @@ import ( type ServiceBindingCredentials ServiceBindingCredentialsV1 // GetOtherDetails returns an unmarshaled version of the OtherDetails field -// or panics. -func (sbc ServiceBindingCredentials) GetOtherDetails() map[string]string { +// or errors. +func (sbc ServiceBindingCredentials) GetOtherDetails() (map[string]string, error) { var creds map[string]string - if err := json.Unmarshal([]byte(sbc.OtherDetails), &creds); err != nil { - panic(err) - } - return creds + err := json.Unmarshal([]byte(sbc.OtherDetails), &creds) + return creds, err } -// ServiceInstanceDetails returns an unmarshaled version of the OtherDetails field -// or panics. +// ServiceInstanceDetails holds information about provisioned services type ServiceInstanceDetails ServiceInstanceDetailsV1 // GetOtherDetails returns an unmarshaled version of the OtherDetails field -// or panics. -func (si ServiceInstanceDetails) GetOtherDetails() map[string]string { +// or errors. +func (si ServiceInstanceDetails) GetOtherDetails() (map[string]string, error) { var instanceDetails map[string]string - // if the instance has access details saved - if si.OtherDetails != "" { - if err := json.Unmarshal([]byte(si.OtherDetails), &instanceDetails); err != nil { - panic(err) - } + if si.OtherDetails == "" { + return instanceDetails, nil } - return instanceDetails + err := json.Unmarshal([]byte(si.OtherDetails), &instanceDetails) + return instanceDetails, err } // ProvisionRequestDetails holds user-defined properties passed to a call diff --git a/integration_tests/async_integration_test.go b/integration_tests/async_integration_test.go index b58b28af2..2d639376c 100644 --- a/integration_tests/async_integration_test.go +++ b/integration_tests/async_integration_test.go @@ -87,11 +87,7 @@ var _ = Describe("AsyncIntegrationTests", func() { fakes.SetUpTestServices() brokerConfig, err = config.NewBrokerConfigFromEnv() - - if err != nil { - logger.Error("error", err) - panic(err.Error()) - } + Expect(err).NotTo(HaveOccurred()) instance_name = generateInstanceName(brokerConfig.ProjectId, "-") diff --git a/integration_tests/integration_test.go b/integration_tests/integration_test.go index a52b41c17..16f53c4be 100644 --- a/integration_tests/integration_test.go +++ b/integration_tests/integration_test.go @@ -60,7 +60,7 @@ type genericService struct { instanceId string serviceExistsFn func(bool) bool cleanupFn func() - serviceMetadataSavedFn func(string) bool + serviceMetadataSavedFn func(string) (bool, error) } type iamService struct { @@ -69,7 +69,7 @@ type iamService struct { planId string } -func getAndUnmarshalInstanceDetails(instanceId string) map[string]string { +func getAndUnmarshalInstanceDetails(instanceId string) (map[string]string, error) { instanceRecord, _ := db_service.GetServiceInstanceDetailsById(instanceId) return instanceRecord.GetOtherDetails() } @@ -98,7 +98,9 @@ func testGenericService(brokerConfig *config.BrokerConfig, gcpBroker *GCPService if params.serviceExistsFn != nil { Expect(params.serviceExistsFn(true)).To(BeTrue()) } - Expect(params.serviceMetadataSavedFn(params.instanceId)).To(BeTrue()) + metadataSaved, err := params.serviceMetadataSavedFn(params.instanceId) + Expect(err).NotTo(HaveOccurred()) + Expect(metadataSaved).To(BeTrue()) // // Bind @@ -168,10 +170,10 @@ func testIamBasedService(brokerConfig *config.BrokerConfig, gcpBroker *GCPServic instanceId: "iam-instance", bindingId: "iam-instance", rawBindingParams: json.RawMessage{}, - serviceMetadataSavedFn: func(instanceId string) bool { + serviceMetadataSavedFn: func(instanceId string) (bool, error) { // Metadata should be empty, there is no additional information required - instanceDetails := getAndUnmarshalInstanceDetails(instanceId) - return len(instanceDetails) == 0 + instanceDetails, err := getAndUnmarshalInstanceDetails(instanceId) + return len(instanceDetails) == 0, err }, } @@ -288,9 +290,9 @@ var _ = Describe("LiveIntegrationTests", func() { return err == nil }, - serviceMetadataSavedFn: func(instanceId string) bool { - instanceDetails := getAndUnmarshalInstanceDetails(instanceId) - return instanceDetails["dataset_id"] != "" + serviceMetadataSavedFn: func(instanceId string) (bool, error) { + instanceDetails, err := getAndUnmarshalInstanceDetails(instanceId) + return instanceDetails["dataset_id"] != "", err }, cleanupFn: func() { err := service.Datasets.Delete(brokerConfig.ProjectId, instance_name).Do() @@ -331,9 +333,9 @@ var _ = Describe("LiveIntegrationTests", func() { return err == nil && len(instances) == 1 && instances[0].Name == bigtableInstanceName }, - serviceMetadataSavedFn: func(instanceId string) bool { - instanceDetails := getAndUnmarshalInstanceDetails(instanceId) - return instanceDetails["instance_id"] != "" + serviceMetadataSavedFn: func(instanceId string) (bool, error) { + instanceDetails, err := getAndUnmarshalInstanceDetails(instanceId) + return instanceDetails["instance_id"] != "", err }, cleanupFn: func() { err := service.DeleteInstance(ctx, bigtableInstanceName) @@ -363,9 +365,9 @@ var _ = Describe("LiveIntegrationTests", func() { return err == nil }, - serviceMetadataSavedFn: func(instanceId string) bool { - instanceDetails := getAndUnmarshalInstanceDetails(instanceId) - return instanceDetails["bucket_name"] != "" + serviceMetadataSavedFn: func(instanceId string) (bool, error) { + instanceDetails, err := getAndUnmarshalInstanceDetails(instanceId) + return instanceDetails["bucket_name"] != "", err }, cleanupFn: func() { bucket := service.Bucket(instance_name) @@ -396,9 +398,9 @@ var _ = Describe("LiveIntegrationTests", func() { exists, err := topic.Exists(context.Background()) return exists && err == nil }, - serviceMetadataSavedFn: func(instanceId string) bool { - instanceDetails := getAndUnmarshalInstanceDetails(instanceId) - return instanceDetails["topic_name"] != "" + serviceMetadataSavedFn: func(instanceId string) (bool, error) { + instanceDetails, err := getAndUnmarshalInstanceDetails(instanceId) + return instanceDetails["topic_name"] != "", err }, cleanupFn: func() { err := topic.Delete(context.Background())