diff --git a/brokerapi/broker/bind_test.go b/brokerapi/broker/bind_test.go index 48cf19cd8..f2d46db67 100644 --- a/brokerapi/broker/bind_test.go +++ b/brokerapi/broker/bind_test.go @@ -15,7 +15,6 @@ import ( "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/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" @@ -61,8 +60,6 @@ var _ = Describe("Bind", func() { PlanGUID: planID, SpaceGUID: spaceID, OrganizationGUID: orgID, - OperationType: models.ProvisionOperationType, - OperationGUID: "provision-operation-GUID", Outputs: map[string]any{"fakeInstanceOutput": "fakeInstanceValue"}, }, nil) diff --git a/brokerapi/broker/deprovision.go b/brokerapi/broker/deprovision.go index db91b1d3d..eb8a0003b 100644 --- a/brokerapi/broker/deprovision.go +++ b/brokerapi/broker/deprovision.go @@ -104,8 +104,6 @@ func (broker *ServiceBroker) Deprovision(ctx context.Context, instanceID string, OperationData: *operationID, } - instance.OperationType = models.DeprovisionOperationType - instance.OperationGUID = *operationID if err := broker.store.StoreServiceInstanceDetails(instance); err != nil { return response, fmt.Errorf("error saving instance details to database: %s. WARNING: this instance will remain visible in cf. Contact your operator for cleanup", err) } diff --git a/brokerapi/broker/deprovision_test.go b/brokerapi/broker/deprovision_test.go index 40dcad0ec..a543ad67d 100644 --- a/brokerapi/broker/deprovision_test.go +++ b/brokerapi/broker/deprovision_test.go @@ -7,7 +7,6 @@ import ( "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/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" @@ -79,8 +78,10 @@ var _ = Describe("Deprovision", func() { GUID: instanceToDeleteID, SpaceGUID: "test-space", OrganizationGUID: "test-org", - OperationType: "provision", - OperationGUID: operationID, + }, nil) + fakeStorage.GetTerraformDeploymentReturns(storage.TerraformDeployment{ + ID: instanceToDeleteID, + LastOperationType: "provision", }, nil) serviceBroker = must(broker.New(brokerConfig, fakeStorage, utils.NewLogger("brokers-test"))) @@ -110,8 +111,6 @@ var _ = Describe("Deprovision", func() { Expect(fakeStorage.StoreServiceInstanceDetailsCallCount()).To(Equal(1)) actualSIDetails := fakeStorage.StoreServiceInstanceDetailsArgsForCall(0) Expect(actualSIDetails.GUID).To(Equal(instanceToDeleteID)) - Expect(actualSIDetails.OperationType).To(Equal(models.DeprovisionOperationType)) - Expect(actualSIDetails.OperationGUID).To(Equal(operationID)) }) When("provider deprovision errors", func() { @@ -139,11 +138,14 @@ var _ = Describe("Deprovision", func() { When("offering does not exists", func() { BeforeEach(func() { fakeStorage.GetServiceInstanceDetailsReturns(storage.ServiceInstanceDetails{ - ServiceGUID: "non-existent-offering", - PlanGUID: planID, - GUID: instanceToDeleteID, - OperationType: "provision", - OperationGUID: "opGUID", + ServiceGUID: "non-existent-offering", + PlanGUID: planID, + GUID: instanceToDeleteID, + }, nil) + + fakeStorage.GetTerraformDeploymentReturns(storage.TerraformDeployment{ + LastOperationType: "provision", + ID: instanceToDeleteID, }, nil) }) diff --git a/brokerapi/broker/last_instance_operation.go b/brokerapi/broker/last_instance_operation.go index 640a17f52..a737891a3 100644 --- a/brokerapi/broker/last_instance_operation.go +++ b/brokerapi/broker/last_instance_operation.go @@ -4,13 +4,13 @@ import ( "context" "fmt" - "code.cloudfoundry.org/lager/v3" - "github.com/pivotal-cf/brokerapi/v11/domain" "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" + "code.cloudfoundry.org/lager/v3" "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker" "github.com/cloudfoundry/cloud-service-broker/v2/utils/correlation" + "github.com/pivotal-cf/brokerapi/v11/domain" ) // LastOperation fetches last operation state for a service instance. @@ -24,9 +24,21 @@ func (broker *ServiceBroker) LastOperation(ctx context.Context, instanceID strin "operation_data": details.OperationData, }) + // make sure that instance actually exists + // Technically according to OSBAPI we should return HTTP 410 Gone only if the operation is a Deprovision, + // and for Provision or Update we should return an HTTP 404 Not Found, but we do not do this, and the + // pivotal-cf/brokerapi/apiresponses package does not provide the required error to do this. + exists, err := broker.store.ExistsServiceInstanceDetails(instanceID) + switch { + case err != nil: + return domain.LastOperation{}, fmt.Errorf("database error checking for existing instance: %s", err) + case !exists: + return domain.LastOperation{}, apiresponses.ErrInstanceDoesNotExist + } + instance, err := broker.store.GetServiceInstanceDetails(instanceID) if err != nil { - return domain.LastOperation{}, apiresponses.ErrInstanceDoesNotExist + return domain.LastOperation{}, fmt.Errorf("error getting service instance details: %w", err) } _, serviceProvider, err := broker.getDefinitionAndProvider(instance.ServiceGUID) @@ -34,9 +46,7 @@ func (broker *ServiceBroker) LastOperation(ctx context.Context, instanceID strin return domain.LastOperation{}, err } - lastOperationType := instance.OperationType - - done, message, err := serviceProvider.PollInstance(ctx, instance.GUID) + done, message, lastOperationType, err := serviceProvider.PollInstance(ctx, instance.GUID) if err != nil { return domain.LastOperation{State: domain.Failed, Description: err.Error()}, nil } @@ -84,11 +94,14 @@ func (broker *ServiceBroker) updateStateOnOperationCompletion(ctx context.Contex } details.Outputs = outs - details.OperationGUID = "" - details.OperationType = models.ClearOperationType if err := broker.store.StoreServiceInstanceDetails(details); err != nil { return fmt.Errorf("error saving instance details to database %v", err) } + err = service.ClearOperationType(ctx, instanceID) + if err != nil { + return fmt.Errorf("error clearing operation type from database %v", err) + } + return nil } diff --git a/brokerapi/broker/last_instance_operation_test.go b/brokerapi/broker/last_instance_operation_test.go index 5d1e949d5..78ce90e72 100644 --- a/brokerapi/broker/last_instance_operation_test.go +++ b/brokerapi/broker/last_instance_operation_test.go @@ -42,7 +42,7 @@ var _ = Describe("LastInstanceOperation", func() { fakeServiceProvider = &pkgBrokerFakes.FakeServiceProvider{} expectedTFOutput = storage.JSONObject{"output": "value"} fakeServiceProvider.GetTerraformOutputsReturns(expectedTFOutput, nil) - fakeServiceProvider.PollInstanceReturns(true, "operation complete", nil) + fakeServiceProvider.PollInstanceReturns(true, "operation complete", models.ProvisionOperationType, nil) providerBuilder := func(logger lager.Logger, store pkgBroker.ServiceProviderStorage) pkgBroker.ServiceProvider { return fakeServiceProvider @@ -66,16 +66,17 @@ var _ = Describe("LastInstanceOperation", func() { } fakeStorage = &brokerfakes.FakeStorage{} + fakeStorage.ExistsServiceInstanceDetailsReturns(true, nil) fakeStorage.GetServiceInstanceDetailsReturns( storage.ServiceInstanceDetails{ GUID: instanceID, - OperationType: models.ProvisionOperationType, - OperationGUID: operationID, PlanGUID: planID, ServiceGUID: offeringID, SpaceGUID: spaceID, OrganizationGUID: orgID, - }, nil) + }, + nil, + ) serviceBroker = must(broker.New(brokerConfig, fakeStorage, utils.NewLogger("brokers-test"))) @@ -89,7 +90,7 @@ var _ = Describe("LastInstanceOperation", func() { Describe("operation complete", func() { Describe("provision", func() { BeforeEach(func() { - fakeServiceProvider.PollInstanceReturns(true, "operation complete", nil) + fakeServiceProvider.PollInstanceReturns(true, "operation complete", models.ProvisionOperationType, nil) }) It("should complete provision", func() { @@ -114,12 +115,16 @@ var _ = Describe("LastInstanceOperation", func() { actualSIDetails := fakeStorage.StoreServiceInstanceDetailsArgsForCall(0) Expect(actualSIDetails.GUID).To(Equal(instanceID)) Expect(actualSIDetails.Outputs).To(Equal(expectedTFOutput)) - Expect(actualSIDetails.OperationGUID).To(BeEmpty()) - Expect(actualSIDetails.OperationType).To(BeEmpty()) Expect(actualSIDetails.ServiceGUID).To(Equal(offeringID)) Expect(actualSIDetails.PlanGUID).To(Equal(planID)) Expect(actualSIDetails.SpaceGUID).To(Equal(spaceID)) Expect(actualSIDetails.OrganizationGUID).To(Equal(orgID)) + + By("validating that the operation type is cleared") + Expect(fakeServiceProvider.ClearOperationTypeCallCount()).To(Equal(1)) + actualContext, actualInstanceID = fakeServiceProvider.ClearOperationTypeArgsForCall(0) + Expect(actualContext.Value(middlewares.OriginatingIdentityKey)).To(Equal(expectedHeader)) + Expect(actualInstanceID).To(Equal(instanceID)) }) }) @@ -128,15 +133,13 @@ var _ = Describe("LastInstanceOperation", func() { fakeStorage.GetServiceInstanceDetailsReturns( storage.ServiceInstanceDetails{ GUID: instanceID, - OperationType: models.DeprovisionOperationType, - OperationGUID: operationID, PlanGUID: planID, ServiceGUID: offeringID, SpaceGUID: spaceID, OrganizationGUID: orgID, }, nil) - fakeServiceProvider.PollInstanceReturns(true, "operation complete", nil) + fakeServiceProvider.PollInstanceReturns(true, "operation complete", models.DeprovisionOperationType, nil) }) It("should complete deprovision", func() { @@ -179,7 +182,7 @@ var _ = Describe("LastInstanceOperation", func() { Describe("operation in progress", func() { BeforeEach(func() { - fakeServiceProvider.PollInstanceReturns(false, "operation in progress still", nil) + fakeServiceProvider.PollInstanceReturns(false, "operation in progress still", models.ProvisionOperationType, nil) }) It("should not update the service instance", func() { @@ -204,7 +207,7 @@ var _ = Describe("LastInstanceOperation", func() { Describe("operation failed", func() { BeforeEach(func() { - fakeServiceProvider.PollInstanceReturns(false, "there was an error", errors.New("some error happened")) + fakeServiceProvider.PollInstanceReturns(false, "there was an error", models.ProvisionOperationType, errors.New("some error happened")) }) It("should set operation to failed", func() { @@ -228,6 +231,17 @@ var _ = Describe("LastInstanceOperation", func() { }) Describe("storage errors", func() { + Context("storage error when checking whether SI exists", func() { + BeforeEach(func() { + fakeStorage.ExistsServiceInstanceDetailsReturns(false, errors.New("failed to check whether SI exists")) + }) + + It("should error", func() { + _, err := serviceBroker.LastOperation(context.TODO(), instanceID, pollDetails) + Expect(err).To(MatchError("database error checking for existing instance: failed to check whether SI exists")) + }) + }) + Context("storage errors when getting SI details", func() { BeforeEach(func() { fakeStorage.GetServiceInstanceDetailsReturns(storage.ServiceInstanceDetails{}, errors.New("failed to get SI details")) @@ -235,7 +249,7 @@ var _ = Describe("LastInstanceOperation", func() { It("should error", func() { _, err := serviceBroker.LastOperation(context.TODO(), instanceID, pollDetails) - Expect(err).To(MatchError("instance does not exist")) + Expect(err).To(MatchError("error getting service instance details: failed to get SI details")) }) }) @@ -256,11 +270,11 @@ var _ = Describe("LastInstanceOperation", func() { BeforeEach(func() { fakeStorage.GetServiceInstanceDetailsReturns( storage.ServiceInstanceDetails{ - GUID: instanceID, - OperationType: models.DeprovisionOperationType, - ServiceGUID: offeringID, + GUID: instanceID, + ServiceGUID: offeringID, }, nil) fakeStorage.DeleteServiceInstanceDetailsReturns(errors.New("failed to delete SI details")) + fakeServiceProvider.PollInstanceReturns(true, "operation complete", models.DeprovisionOperationType, nil) }) It("should error", func() { @@ -275,11 +289,11 @@ var _ = Describe("LastInstanceOperation", func() { BeforeEach(func() { fakeStorage.GetServiceInstanceDetailsReturns( storage.ServiceInstanceDetails{ - GUID: instanceID, - OperationType: models.DeprovisionOperationType, - ServiceGUID: offeringID, + GUID: instanceID, + ServiceGUID: offeringID, }, nil) fakeStorage.DeleteProvisionRequestDetailsReturns(errors.New("failed to delete provision params")) + fakeServiceProvider.PollInstanceReturns(true, "operation complete", models.DeprovisionOperationType, nil) }) It("should error", func() { @@ -291,16 +305,16 @@ var _ = Describe("LastInstanceOperation", func() { }) }) - Describe("Service provider error", func() { + Describe("service provider error", func() { Context("service provider errors when deleting provider instance data", func() { BeforeEach(func() { fakeStorage.GetServiceInstanceDetailsReturns( storage.ServiceInstanceDetails{ - GUID: instanceID, - OperationType: models.DeprovisionOperationType, - ServiceGUID: offeringID, + GUID: instanceID, + ServiceGUID: offeringID, }, nil) fakeServiceProvider.DeleteInstanceDataReturns(errors.New("failed to delete provider instance data")) + fakeServiceProvider.PollInstanceReturns(true, "operation complete", models.DeprovisionOperationType, nil) }) It("should error", func() { @@ -314,7 +328,7 @@ var _ = Describe("LastInstanceOperation", func() { Describe("service instance does not exist", func() { BeforeEach(func() { - fakeStorage.GetServiceInstanceDetailsReturns(storage.ServiceInstanceDetails{}, errors.New("does not exist")) + fakeStorage.ExistsServiceInstanceDetailsReturns(false, nil) }) It("should return HTTP 410 as per OSBAPI spec", func() { diff --git a/brokerapi/broker/provision.go b/brokerapi/broker/provision.go index be3104966..6404994ce 100644 --- a/brokerapi/broker/provision.go +++ b/brokerapi/broker/provision.go @@ -23,6 +23,7 @@ import ( "github.com/pivotal-cf/brokerapi/v11/domain/apiresponses" "github.com/cloudfoundry/cloud-service-broker/v2/internal/paramparser" + "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" "github.com/cloudfoundry/cloud-service-broker/v2/utils/correlation" "github.com/cloudfoundry/cloud-service-broker/v2/utils/request" ) @@ -81,18 +82,19 @@ func (broker *ServiceBroker) Provision(ctx context.Context, instanceID string, d return domain.ProvisionedServiceSpec{}, err } - // get instance details - instanceDetails, err := serviceProvider.Provision(ctx, vars) + err = serviceProvider.Provision(ctx, vars) if err != nil { return domain.ProvisionedServiceSpec{}, err } // save instance details - instanceDetails.ServiceGUID = parsedDetails.ServiceID - instanceDetails.GUID = instanceID - instanceDetails.PlanGUID = parsedDetails.PlanID - instanceDetails.SpaceGUID = parsedDetails.SpaceGUID - instanceDetails.OrganizationGUID = parsedDetails.OrganizationGUID + instanceDetails := storage.ServiceInstanceDetails{ + ServiceGUID: parsedDetails.ServiceID, + GUID: instanceID, + PlanGUID: parsedDetails.PlanID, + SpaceGUID: parsedDetails.SpaceGUID, + OrganizationGUID: parsedDetails.OrganizationGUID, + } if err := broker.store.StoreServiceInstanceDetails(instanceDetails); err != nil { return domain.ProvisionedServiceSpec{}, fmt.Errorf("error saving instance details to database: %s. WARNING: this instance cannot be deprovisioned through cf. Contact your operator for cleanup", err) @@ -104,5 +106,7 @@ func (broker *ServiceBroker) Provision(ctx context.Context, instanceID string, d return domain.ProvisionedServiceSpec{}, fmt.Errorf("error saving provision request details to database: %s. Services relying on async provisioning will not be able to complete provisioning", err) } - return domain.ProvisionedServiceSpec{IsAsync: true, DashboardURL: "", OperationData: instanceDetails.OperationGUID}, nil + operationID := generateTFInstanceID(instanceDetails.GUID) + + return domain.ProvisionedServiceSpec{IsAsync: true, DashboardURL: "", OperationData: operationID}, nil } diff --git a/brokerapi/broker/provision_test.go b/brokerapi/broker/provision_test.go index 3f06b9d11..668eb2b34 100644 --- a/brokerapi/broker/provision_test.go +++ b/brokerapi/broker/provision_test.go @@ -16,7 +16,6 @@ import ( "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/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" @@ -32,7 +31,7 @@ var _ = Describe("Provision", func() { planID = "test-plan-id" offeringID = "test-service-id" newInstanceID = "test-instance-id" - operationID = "test-operation-id" + operationID = "tf:test-instance-id:" ) var ( @@ -45,10 +44,7 @@ var _ = Describe("Provision", func() { BeforeEach(func() { fakeServiceProvider = &pkgBrokerFakes.FakeServiceProvider{} - fakeServiceProvider.ProvisionReturns(storage.ServiceInstanceDetails{ - OperationType: models.ProvisionOperationType, - OperationGUID: operationID, - }, nil) + fakeServiceProvider.ProvisionReturns(nil) providerBuilder := func(logger lager.Logger, store pkgBroker.ServiceProviderStorage) pkgBroker.ServiceProvider { return fakeServiceProvider @@ -255,7 +251,7 @@ var _ = Describe("Provision", func() { When("provider provision errors", func() { BeforeEach(func() { - fakeServiceProvider.ProvisionReturns(storage.ServiceInstanceDetails{}, errors.New("cannot provision right now")) + fakeServiceProvider.ProvisionReturns(errors.New("cannot provision right now")) }) It("should error", func() { diff --git a/brokerapi/broker/unbind_test.go b/brokerapi/broker/unbind_test.go index c59b2b1bb..86979739b 100644 --- a/brokerapi/broker/unbind_test.go +++ b/brokerapi/broker/unbind_test.go @@ -8,7 +8,6 @@ import ( "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/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" @@ -57,8 +56,6 @@ var _ = Describe("Unbind", func() { PlanGUID: planID, SpaceGUID: spaceID, OrganizationGUID: orgID, - OperationType: models.ProvisionOperationType, - OperationGUID: "provision-operation-GUID", }, nil) fakeStorage.GetBindRequestDetailsReturns(storage.JSONObject{"foo": "bar"}, nil) @@ -114,8 +111,6 @@ var _ = Describe("Unbind", func() { PlanGUID: planID, SpaceGUID: spaceID, OrganizationGUID: orgID, - OperationType: models.ProvisionOperationType, - OperationGUID: "provision-operation-GUID", }, nil) fakeStorage.GetBindRequestDetailsReturns(storage.JSONObject{"foo": "bar"}, nil) }) diff --git a/brokerapi/broker/update.go b/brokerapi/broker/update.go index 3960b96bf..fcbab5fc0 100644 --- a/brokerapi/broker/update.go +++ b/brokerapi/broker/update.go @@ -173,7 +173,7 @@ func (broker *ServiceBroker) doUpdate(ctx context.Context, serviceProvider broke return domain.UpdateServiceSpec{}, fmt.Errorf("tofu version check failed: %s", err.Error()) } - instanceDetails, err := serviceProvider.Update(ctx, vars) + err = serviceProvider.Update(ctx, vars) if err != nil { return domain.UpdateServiceSpec{}, err } @@ -183,8 +183,6 @@ func (broker *ServiceBroker) doUpdate(ctx context.Context, serviceProvider broke instance.PlanGUID = parsedDetails.PlanID } - instance.OperationType = instanceDetails.OperationType - instance.OperationGUID = instanceDetails.OperationID if err := broker.store.StoreServiceInstanceDetails(instance); err != nil { return domain.UpdateServiceSpec{}, fmt.Errorf("error saving instance details to database: %s. WARNING: this instance cannot be deprovisioned through cf. Contact your operator for cleanup", err) } @@ -197,7 +195,7 @@ func (broker *ServiceBroker) doUpdate(ctx context.Context, serviceProvider broke return domain.UpdateServiceSpec{ IsAsync: true, DashboardURL: "", - OperationData: instanceDetails.OperationID, + OperationData: generateTFInstanceID(instance.GUID), }, nil } diff --git a/brokerapi/broker/update_test.go b/brokerapi/broker/update_test.go index 135f028a4..e7d4098f4 100644 --- a/brokerapi/broker/update_test.go +++ b/brokerapi/broker/update_test.go @@ -35,7 +35,7 @@ var _ = Describe("Update", func() { offeringID = "test-service-id" newPlanID = "new-test-plan-id" instanceID = "test-instance-id" - updateOperationID = "update-operation-id" + updateOperationID = "tf:test-instance-id:" ) var ( @@ -129,8 +129,11 @@ var _ = Describe("Update", func() { PlanGUID: originalPlanID, SpaceGUID: spaceID, OrganizationGUID: orgID, - OperationType: models.ProvisionOperationType, - OperationGUID: "provision-operation-GUID", + }, nil) + fakeStorage.GetTerraformDeploymentReturns(storage.TerraformDeployment{ + ID: updateOperationID, + LastOperationType: models.ProvisionOperationType, + LastOperationState: "GOOG", }, nil) serviceBroker = must(broker.New(brokerConfig, fakeStorage, utils.NewLogger("brokers-test"))) @@ -157,10 +160,8 @@ var _ = Describe("Update", func() { Describe("update", func() { When("no plan or parameter changes are requested", func() { BeforeEach(func() { - fakeServiceProvider.UpdateReturns(models.ServiceInstanceDetails{ - OperationType: models.UpdateOperationType, - OperationID: updateOperationID, - }, nil) + fakeServiceProvider.UpdateReturns(nil) + fakeServiceProvider.PollInstanceReturns(true, "a message", models.UpdateOperationType, nil) }) It("should complete changing the instance operation type", func() { @@ -196,10 +197,7 @@ var _ = Describe("Update", func() { When("plan change is requested", func() { BeforeEach(func() { - fakeServiceProvider.UpdateReturns(models.ServiceInstanceDetails{ - OperationType: models.UpdateOperationType, - OperationID: updateOperationID, - }, nil) + fakeServiceProvider.UpdateReturns(nil) }) It("should do update async and not change planID", func() { @@ -230,10 +228,7 @@ var _ = Describe("Update", func() { When("parameter change is requested", func() { BeforeEach(func() { - fakeServiceProvider.UpdateReturns(models.ServiceInstanceDetails{ - OperationType: models.UpdateOperationType, - OperationID: updateOperationID, - }, nil) + fakeServiceProvider.UpdateReturns(nil) updateDetails = domain.UpdateDetails{ ServiceID: offeringID, @@ -286,7 +281,7 @@ var _ = Describe("Update", func() { When("provider update errors", func() { BeforeEach(func() { - fakeServiceProvider.UpdateReturns(models.ServiceInstanceDetails{}, errors.New("cannot update right now")) + fakeServiceProvider.UpdateReturns(errors.New("cannot update right now")) }) It("should error and not update the provision variables", func() { @@ -799,8 +794,6 @@ func expectOperationTypeToBeUpdated( ) { Expect(fakeStorage.StoreServiceInstanceDetailsCallCount()).To(Equal(1)) actualServiceInstanceDetails := fakeStorage.StoreServiceInstanceDetailsArgsForCall(0) - Expect(actualServiceInstanceDetails.OperationType).To(Equal("update")) - Expect(actualServiceInstanceDetails.OperationGUID).To(Equal(operationGUID)) Expect(actualServiceInstanceDetails.GUID).To(Equal(instanceID)) Expect(actualServiceInstanceDetails.ServiceGUID).To(Equal(offeringID)) Expect(actualServiceInstanceDetails.PlanGUID).To(Equal(newPlanID)) diff --git a/dbservice/migrations.go b/dbservice/migrations.go index 30a7dc3c7..7b7fcd084 100644 --- a/dbservice/migrations.go +++ b/dbservice/migrations.go @@ -23,7 +23,7 @@ import ( "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" ) -const numMigrations = 16 +const numMigrations = 17 // RunMigrations runs schema migrations on the provided service broker database to get it up to date func RunMigrations(db *gorm.DB) error { @@ -128,6 +128,19 @@ func RunMigrations(db *gorm.DB) error { return autoMigrateTables(db, &models.BindRequestDetailsV1{}) } + migrations[16] = func() error { + if err := db.Migrator().DropColumn(&models.ServiceInstanceDetailsV4{}, "operation_type"); err != nil { + return err + } + if err := db.Migrator().DropColumn(&models.ServiceInstanceDetailsV4{}, "operation_id"); err != nil { + return err + } + if err := db.Migrator().DropColumn(&models.ServiceInstanceDetailsV4{}, "location"); err != nil { + return err + } + return db.Migrator().DropColumn(&models.ServiceInstanceDetailsV4{}, "url") + } + var lastMigrationNumber = -1 // if we've run any migrations before, we should have a migrations table, so find the last one we ran diff --git a/dbservice/models/db.go b/dbservice/models/db.go index bc38d787e..0b5ee494d 100644 --- a/dbservice/models/db.go +++ b/dbservice/models/db.go @@ -34,7 +34,7 @@ const ( type ServiceBindingCredentials ServiceBindingCredentialsV2 // ServiceInstanceDetails holds information about provisioned services. -type ServiceInstanceDetails ServiceInstanceDetailsV3 +type ServiceInstanceDetails ServiceInstanceDetailsV4 // ProvisionRequestDetails holds user-defined properties passed to a call // to provision a service. diff --git a/dbservice/models/historical_db.go b/dbservice/models/historical_db.go index 55d6dd8a7..58bb2a7b9 100644 --- a/dbservice/models/historical_db.go +++ b/dbservice/models/historical_db.go @@ -162,6 +162,29 @@ func (ServiceInstanceDetailsV3) TableName() string { return "service_instance_details" } +// ServiceInstanceDetailsV4 holds information about provisioned services. +type ServiceInstanceDetailsV4 struct { + ID string `gorm:"primary_key;type:varchar(255);not null"` + CreatedAt time.Time + UpdatedAt time.Time + DeletedAt *time.Time + + Name string + OtherDetails []byte `gorm:"type:blob"` + + ServiceID string + PlanID string + SpaceGUID string + OrganizationGUID string +} + +// TableName returns a consistent table name for +// gorm so multiple structs from different versions of the database all operate +// on the same table. +func (ServiceInstanceDetailsV4) TableName() string { + return "service_instance_details" +} + // ProvisionRequestDetailsV1 holds user-defined properties passed to a call // to provision a service. type ProvisionRequestDetailsV1 struct { diff --git a/docs/configuration.md b/docs/configuration.md index 80df62af8..a58e7f609 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -78,6 +78,7 @@ no longer marked as primary. ## Broker Service Configuration Broker service configuration values: + | Environment Variable | Config File Value | Type | Description | |----------------------|------|-------------|------------------| | SECURITY_USER_NAME * | api.user | string |

Broker authentication username

| @@ -86,9 +87,18 @@ Broker service configuration values: | TLS_CERT_CHAIN | api.certCaBundle | string |

File path to a pem encoded certificate chain

| | TLS_PRIVATE_KEY | api.tlsKey | string |

File path to a pem encoded private key

| + +## Debugging +Values for debugging: + +| Environment Variable | Config File Value | Type | Description | +|----------------------------------------|---------------------------|------|-------------------------------------------------------------------------------------------------| +| CSB_DEBUG_LEAVE_WORKSPACE_DIR | debug.leave_workspace_dir | bool | Disables the cleanup of workspace directories, so you can inspect the files and run tf commands | + ## Feature flags Configuration Feature flags can be toggled through the following configuration values. See also [source code occurences of "toggles.Features.Toggle"](https://github.com/cloudfoundry/cloud-service-broker/search?q=toggles.Features.Toggle&type=code) + | Environment Variable | Config File Value | Type | Description | Default | |----------------------|------|-------------|------------------|----------| | GSB_COMPATIBILITY_ENABLE_BUILTIN_BROKERPAKS * | compatibility.enable_builtin_brokerpaks | Boolean |

Load brokerpaks that are built-in to the software.

| "true" | diff --git a/internal/storage/service_instance_details.go b/internal/storage/service_instance_details.go index 78af047e9..f2c1c0e9d 100644 --- a/internal/storage/service_instance_details.go +++ b/internal/storage/service_instance_details.go @@ -10,15 +10,11 @@ import ( type ServiceInstanceDetails struct { GUID string Name string - Location string - URL string Outputs JSONObject ServiceGUID string PlanGUID string SpaceGUID string OrganizationGUID string - OperationType string - OperationGUID string } func (s *Storage) StoreServiceInstanceDetails(d ServiceInstanceDetails) error { @@ -33,15 +29,11 @@ func (s *Storage) StoreServiceInstanceDetails(d ServiceInstanceDetails) error { } m.Name = d.Name - m.Location = d.Location - m.URL = d.URL m.OtherDetails = encoded m.ServiceID = d.ServiceGUID m.PlanID = d.PlanGUID m.SpaceGUID = d.SpaceGUID m.OrganizationGUID = d.OrganizationGUID - m.OperationType = d.OperationType - m.OperationID = d.OperationGUID switch m.ID { case "": @@ -88,15 +80,11 @@ func (s *Storage) GetServiceInstanceDetails(guid string) (ServiceInstanceDetails return ServiceInstanceDetails{ GUID: guid, Name: receiver.Name, - Location: receiver.Location, - URL: receiver.URL, Outputs: decoded, ServiceGUID: receiver.ServiceID, PlanGUID: receiver.PlanID, SpaceGUID: receiver.SpaceGUID, OrganizationGUID: receiver.OrganizationGUID, - OperationType: receiver.OperationType, - OperationGUID: receiver.OperationID, }, nil } diff --git a/internal/storage/service_instance_details_test.go b/internal/storage/service_instance_details_test.go index 4bc717b8b..83a15bd07 100644 --- a/internal/storage/service_instance_details_test.go +++ b/internal/storage/service_instance_details_test.go @@ -15,15 +15,11 @@ var _ = Describe("ServiceInstanceDetails", func() { err := store.StoreServiceInstanceDetails(storage.ServiceInstanceDetails{ GUID: "fake-guid", Name: "fake-name", - Location: "fake-location", - URL: "fake-url", Outputs: map[string]any{"foo": "bar"}, ServiceGUID: "fake-service-guid", PlanGUID: "fake-plan-guid", SpaceGUID: "fake-space-guid", OrganizationGUID: "fake-org-guid", - OperationType: "fake-operation-type", - OperationGUID: "fake-operation-guid", }) Expect(err).NotTo(HaveOccurred()) @@ -31,15 +27,11 @@ var _ = Describe("ServiceInstanceDetails", func() { Expect(db.Find(&receiver).Error).NotTo(HaveOccurred()) Expect(receiver.ID).To(Equal("fake-guid")) Expect(receiver.Name).To(Equal("fake-name")) - Expect(receiver.Location).To(Equal("fake-location")) - Expect(receiver.URL).To(Equal("fake-url")) Expect(receiver.OtherDetails).To(Equal([]byte(`{"encrypted":{"foo":"bar"}}`))) Expect(receiver.ServiceID).To(Equal("fake-service-guid")) Expect(receiver.PlanID).To(Equal("fake-plan-guid")) Expect(receiver.SpaceGUID).To(Equal("fake-space-guid")) Expect(receiver.OrganizationGUID).To(Equal("fake-org-guid")) - Expect(receiver.OperationType).To(Equal("fake-operation-type")) - Expect(receiver.OperationID).To(Equal("fake-operation-guid")) }) When("encoding fails", func() { @@ -60,15 +52,11 @@ var _ = Describe("ServiceInstanceDetails", func() { err := store.StoreServiceInstanceDetails(storage.ServiceInstanceDetails{ GUID: "fake-id-1", Name: "fake-name", - Location: "fake-location", - URL: "fake-url", Outputs: map[string]any{"foo": "bar"}, ServiceGUID: "fake-service-guid", PlanGUID: "fake-plan-guid", SpaceGUID: "fake-space-guid", OrganizationGUID: "fake-org-guid", - OperationType: "fake-operation-type", - OperationGUID: "fake-operation-guid", }) Expect(err).NotTo(HaveOccurred()) @@ -76,15 +64,11 @@ var _ = Describe("ServiceInstanceDetails", func() { Expect(db.Where(`id = "fake-id-1"`).Find(&receiver).Error).NotTo(HaveOccurred()) Expect(receiver.ID).To(Equal("fake-id-1")) Expect(receiver.Name).To(Equal("fake-name")) - Expect(receiver.Location).To(Equal("fake-location")) - Expect(receiver.URL).To(Equal("fake-url")) Expect(receiver.OtherDetails).To(Equal([]byte(`{"encrypted":{"foo":"bar"}}`))) Expect(receiver.ServiceID).To(Equal("fake-service-guid")) Expect(receiver.PlanID).To(Equal("fake-plan-guid")) Expect(receiver.SpaceGUID).To(Equal("fake-space-guid")) Expect(receiver.OrganizationGUID).To(Equal("fake-org-guid")) - Expect(receiver.OperationType).To(Equal("fake-operation-type")) - Expect(receiver.OperationID).To(Equal("fake-operation-guid")) }) }) }) @@ -99,15 +83,11 @@ var _ = Describe("ServiceInstanceDetails", func() { Expect(err).NotTo(HaveOccurred()) Expect(r.GUID).To(Equal("fake-id-2")) - Expect(r.Location).To(Equal("fake-location-2")) - Expect(r.URL).To(Equal("fake-url-2")) Expect(r.Outputs).To(Equal(storage.JSONObject{"decrypted": map[string]any{"foo": "bar-2"}})) Expect(r.ServiceGUID).To(Equal("fake-service-id-2")) Expect(r.PlanGUID).To(Equal("fake-plan-id-2")) Expect(r.SpaceGUID).To(Equal("fake-space-guid-2")) Expect(r.OrganizationGUID).To(Equal("fake-org-guid-2")) - Expect(r.OperationType).To(Equal("fake-operation-type-2")) - Expect(r.OperationGUID).To(Equal("fake-operation-id-2")) }) When("decoding fails", func() { @@ -204,40 +184,28 @@ func addFakeServiceInstanceDetails() { Expect(db.Create(&models.ServiceInstanceDetails{ ID: "fake-id-1", Name: "fake-name-1", - Location: "fake-location-1", - URL: "fake-url-1", OtherDetails: []byte(`{"foo":"bar-1"}`), ServiceID: "fake-service-id-1", PlanID: "fake-plan-id-1", SpaceGUID: "fake-space-guid-1", OrganizationGUID: "fake-org-guid-1", - OperationType: "fake-operation-type-1", - OperationID: "fake-operation-id-1", }).Error).NotTo(HaveOccurred()) Expect(db.Create(&models.ServiceInstanceDetails{ ID: "fake-id-2", Name: "fake-name-2", - Location: "fake-location-2", - URL: "fake-url-2", OtherDetails: []byte(`{"foo":"bar-2"}`), ServiceID: "fake-service-id-2", PlanID: "fake-plan-id-2", SpaceGUID: "fake-space-guid-2", OrganizationGUID: "fake-org-guid-2", - OperationType: "fake-operation-type-2", - OperationID: "fake-operation-id-2", }).Error).NotTo(HaveOccurred()) Expect(db.Create(&models.ServiceInstanceDetails{ ID: "fake-id-3", Name: "fake-name-3", - Location: "fake-location-3", - URL: "fake-url-3", OtherDetails: []byte(`{"foo":"bar-3"}`), ServiceID: "fake-service-id-3", PlanID: "fake-plan-id-3", SpaceGUID: "fake-space-guid-3", OrganizationGUID: "fake-org-guid-3", - OperationType: "fake-operation-type-3", - OperationID: "fake-operation-id-3", }).Error).NotTo(HaveOccurred()) } diff --git a/pkg/broker/brokerfakes/fake_service_provider.go b/pkg/broker/brokerfakes/fake_service_provider.go index a965087eb..53f9f9f56 100644 --- a/pkg/broker/brokerfakes/fake_service_provider.go +++ b/pkg/broker/brokerfakes/fake_service_provider.go @@ -5,7 +5,6 @@ import ( "context" "sync" - "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/varcontext" @@ -49,6 +48,18 @@ type FakeServiceProvider struct { checkUpgradeAvailableReturnsOnCall map[int]struct { result1 error } + ClearOperationTypeStub func(context.Context, string) error + clearOperationTypeMutex sync.RWMutex + clearOperationTypeArgsForCall []struct { + arg1 context.Context + arg2 string + } + clearOperationTypeReturns struct { + result1 error + } + clearOperationTypeReturnsOnCall map[int]struct { + result1 error + } DeleteBindingDataStub func(context.Context, string, string) error deleteBindingDataMutex sync.RWMutex deleteBindingDataArgsForCall []struct { @@ -119,7 +130,7 @@ type FakeServiceProvider struct { result1 storage.JSONObject result2 error } - PollInstanceStub func(context.Context, string) (bool, string, error) + PollInstanceStub func(context.Context, string) (bool, string, string, error) pollInstanceMutex sync.RWMutex pollInstanceArgsForCall []struct { arg1 context.Context @@ -128,26 +139,26 @@ type FakeServiceProvider struct { pollInstanceReturns struct { result1 bool result2 string - result3 error + result3 string + result4 error } pollInstanceReturnsOnCall map[int]struct { result1 bool result2 string - result3 error + result3 string + result4 error } - ProvisionStub func(context.Context, *varcontext.VarContext) (storage.ServiceInstanceDetails, error) + ProvisionStub func(context.Context, *varcontext.VarContext) error provisionMutex sync.RWMutex provisionArgsForCall []struct { arg1 context.Context arg2 *varcontext.VarContext } provisionReturns struct { - result1 storage.ServiceInstanceDetails - result2 error + result1 error } provisionReturnsOnCall map[int]struct { - result1 storage.ServiceInstanceDetails - result2 error + result1 error } UnbindStub func(context.Context, string, string, *varcontext.VarContext) error unbindMutex sync.RWMutex @@ -163,19 +174,17 @@ type FakeServiceProvider struct { unbindReturnsOnCall map[int]struct { result1 error } - UpdateStub func(context.Context, *varcontext.VarContext) (models.ServiceInstanceDetails, error) + UpdateStub func(context.Context, *varcontext.VarContext) error updateMutex sync.RWMutex updateArgsForCall []struct { arg1 context.Context arg2 *varcontext.VarContext } updateReturns struct { - result1 models.ServiceInstanceDetails - result2 error + result1 error } updateReturnsOnCall map[int]struct { - result1 models.ServiceInstanceDetails - result2 error + result1 error } UpgradeBindingsStub func(context.Context, *varcontext.VarContext, []*varcontext.VarContext) error upgradeBindingsMutex sync.RWMutex @@ -396,6 +405,68 @@ func (fake *FakeServiceProvider) CheckUpgradeAvailableReturnsOnCall(i int, resul }{result1} } +func (fake *FakeServiceProvider) ClearOperationType(arg1 context.Context, arg2 string) error { + fake.clearOperationTypeMutex.Lock() + ret, specificReturn := fake.clearOperationTypeReturnsOnCall[len(fake.clearOperationTypeArgsForCall)] + fake.clearOperationTypeArgsForCall = append(fake.clearOperationTypeArgsForCall, struct { + arg1 context.Context + arg2 string + }{arg1, arg2}) + stub := fake.ClearOperationTypeStub + fakeReturns := fake.clearOperationTypeReturns + fake.recordInvocation("ClearOperationType", []interface{}{arg1, arg2}) + fake.clearOperationTypeMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeServiceProvider) ClearOperationTypeCallCount() int { + fake.clearOperationTypeMutex.RLock() + defer fake.clearOperationTypeMutex.RUnlock() + return len(fake.clearOperationTypeArgsForCall) +} + +func (fake *FakeServiceProvider) ClearOperationTypeCalls(stub func(context.Context, string) error) { + fake.clearOperationTypeMutex.Lock() + defer fake.clearOperationTypeMutex.Unlock() + fake.ClearOperationTypeStub = stub +} + +func (fake *FakeServiceProvider) ClearOperationTypeArgsForCall(i int) (context.Context, string) { + fake.clearOperationTypeMutex.RLock() + defer fake.clearOperationTypeMutex.RUnlock() + argsForCall := fake.clearOperationTypeArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeServiceProvider) ClearOperationTypeReturns(result1 error) { + fake.clearOperationTypeMutex.Lock() + defer fake.clearOperationTypeMutex.Unlock() + fake.ClearOperationTypeStub = nil + fake.clearOperationTypeReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeServiceProvider) ClearOperationTypeReturnsOnCall(i int, result1 error) { + fake.clearOperationTypeMutex.Lock() + defer fake.clearOperationTypeMutex.Unlock() + fake.ClearOperationTypeStub = nil + if fake.clearOperationTypeReturnsOnCall == nil { + fake.clearOperationTypeReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.clearOperationTypeReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeServiceProvider) DeleteBindingData(arg1 context.Context, arg2 string, arg3 string) error { fake.deleteBindingDataMutex.Lock() ret, specificReturn := fake.deleteBindingDataReturnsOnCall[len(fake.deleteBindingDataArgsForCall)] @@ -724,7 +795,7 @@ func (fake *FakeServiceProvider) GetTerraformOutputsReturnsOnCall(i int, result1 }{result1, result2} } -func (fake *FakeServiceProvider) PollInstance(arg1 context.Context, arg2 string) (bool, string, error) { +func (fake *FakeServiceProvider) PollInstance(arg1 context.Context, arg2 string) (bool, string, string, error) { fake.pollInstanceMutex.Lock() ret, specificReturn := fake.pollInstanceReturnsOnCall[len(fake.pollInstanceArgsForCall)] fake.pollInstanceArgsForCall = append(fake.pollInstanceArgsForCall, struct { @@ -739,9 +810,9 @@ func (fake *FakeServiceProvider) PollInstance(arg1 context.Context, arg2 string) return stub(arg1, arg2) } if specificReturn { - return ret.result1, ret.result2, ret.result3 + return ret.result1, ret.result2, ret.result3, ret.result4 } - return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3, fakeReturns.result4 } func (fake *FakeServiceProvider) PollInstanceCallCount() int { @@ -750,7 +821,7 @@ func (fake *FakeServiceProvider) PollInstanceCallCount() int { return len(fake.pollInstanceArgsForCall) } -func (fake *FakeServiceProvider) PollInstanceCalls(stub func(context.Context, string) (bool, string, error)) { +func (fake *FakeServiceProvider) PollInstanceCalls(stub func(context.Context, string) (bool, string, string, error)) { fake.pollInstanceMutex.Lock() defer fake.pollInstanceMutex.Unlock() fake.PollInstanceStub = stub @@ -763,18 +834,19 @@ func (fake *FakeServiceProvider) PollInstanceArgsForCall(i int) (context.Context return argsForCall.arg1, argsForCall.arg2 } -func (fake *FakeServiceProvider) PollInstanceReturns(result1 bool, result2 string, result3 error) { +func (fake *FakeServiceProvider) PollInstanceReturns(result1 bool, result2 string, result3 string, result4 error) { fake.pollInstanceMutex.Lock() defer fake.pollInstanceMutex.Unlock() fake.PollInstanceStub = nil fake.pollInstanceReturns = struct { result1 bool result2 string - result3 error - }{result1, result2, result3} + result3 string + result4 error + }{result1, result2, result3, result4} } -func (fake *FakeServiceProvider) PollInstanceReturnsOnCall(i int, result1 bool, result2 string, result3 error) { +func (fake *FakeServiceProvider) PollInstanceReturnsOnCall(i int, result1 bool, result2 string, result3 string, result4 error) { fake.pollInstanceMutex.Lock() defer fake.pollInstanceMutex.Unlock() fake.PollInstanceStub = nil @@ -782,17 +854,19 @@ func (fake *FakeServiceProvider) PollInstanceReturnsOnCall(i int, result1 bool, fake.pollInstanceReturnsOnCall = make(map[int]struct { result1 bool result2 string - result3 error + result3 string + result4 error }) } fake.pollInstanceReturnsOnCall[i] = struct { result1 bool result2 string - result3 error - }{result1, result2, result3} + result3 string + result4 error + }{result1, result2, result3, result4} } -func (fake *FakeServiceProvider) Provision(arg1 context.Context, arg2 *varcontext.VarContext) (storage.ServiceInstanceDetails, error) { +func (fake *FakeServiceProvider) Provision(arg1 context.Context, arg2 *varcontext.VarContext) error { fake.provisionMutex.Lock() ret, specificReturn := fake.provisionReturnsOnCall[len(fake.provisionArgsForCall)] fake.provisionArgsForCall = append(fake.provisionArgsForCall, struct { @@ -807,9 +881,9 @@ func (fake *FakeServiceProvider) Provision(arg1 context.Context, arg2 *varcontex return stub(arg1, arg2) } if specificReturn { - return ret.result1, ret.result2 + return ret.result1 } - return fakeReturns.result1, fakeReturns.result2 + return fakeReturns.result1 } func (fake *FakeServiceProvider) ProvisionCallCount() int { @@ -818,7 +892,7 @@ func (fake *FakeServiceProvider) ProvisionCallCount() int { return len(fake.provisionArgsForCall) } -func (fake *FakeServiceProvider) ProvisionCalls(stub func(context.Context, *varcontext.VarContext) (storage.ServiceInstanceDetails, error)) { +func (fake *FakeServiceProvider) ProvisionCalls(stub func(context.Context, *varcontext.VarContext) error) { fake.provisionMutex.Lock() defer fake.provisionMutex.Unlock() fake.ProvisionStub = stub @@ -831,30 +905,27 @@ func (fake *FakeServiceProvider) ProvisionArgsForCall(i int) (context.Context, * return argsForCall.arg1, argsForCall.arg2 } -func (fake *FakeServiceProvider) ProvisionReturns(result1 storage.ServiceInstanceDetails, result2 error) { +func (fake *FakeServiceProvider) ProvisionReturns(result1 error) { fake.provisionMutex.Lock() defer fake.provisionMutex.Unlock() fake.ProvisionStub = nil fake.provisionReturns = struct { - result1 storage.ServiceInstanceDetails - result2 error - }{result1, result2} + result1 error + }{result1} } -func (fake *FakeServiceProvider) ProvisionReturnsOnCall(i int, result1 storage.ServiceInstanceDetails, result2 error) { +func (fake *FakeServiceProvider) ProvisionReturnsOnCall(i int, result1 error) { fake.provisionMutex.Lock() defer fake.provisionMutex.Unlock() fake.ProvisionStub = nil if fake.provisionReturnsOnCall == nil { fake.provisionReturnsOnCall = make(map[int]struct { - result1 storage.ServiceInstanceDetails - result2 error + result1 error }) } fake.provisionReturnsOnCall[i] = struct { - result1 storage.ServiceInstanceDetails - result2 error - }{result1, result2} + result1 error + }{result1} } func (fake *FakeServiceProvider) Unbind(arg1 context.Context, arg2 string, arg3 string, arg4 *varcontext.VarContext) error { @@ -921,7 +992,7 @@ func (fake *FakeServiceProvider) UnbindReturnsOnCall(i int, result1 error) { }{result1} } -func (fake *FakeServiceProvider) Update(arg1 context.Context, arg2 *varcontext.VarContext) (models.ServiceInstanceDetails, error) { +func (fake *FakeServiceProvider) Update(arg1 context.Context, arg2 *varcontext.VarContext) error { fake.updateMutex.Lock() ret, specificReturn := fake.updateReturnsOnCall[len(fake.updateArgsForCall)] fake.updateArgsForCall = append(fake.updateArgsForCall, struct { @@ -936,9 +1007,9 @@ func (fake *FakeServiceProvider) Update(arg1 context.Context, arg2 *varcontext.V return stub(arg1, arg2) } if specificReturn { - return ret.result1, ret.result2 + return ret.result1 } - return fakeReturns.result1, fakeReturns.result2 + return fakeReturns.result1 } func (fake *FakeServiceProvider) UpdateCallCount() int { @@ -947,7 +1018,7 @@ func (fake *FakeServiceProvider) UpdateCallCount() int { return len(fake.updateArgsForCall) } -func (fake *FakeServiceProvider) UpdateCalls(stub func(context.Context, *varcontext.VarContext) (models.ServiceInstanceDetails, error)) { +func (fake *FakeServiceProvider) UpdateCalls(stub func(context.Context, *varcontext.VarContext) error) { fake.updateMutex.Lock() defer fake.updateMutex.Unlock() fake.UpdateStub = stub @@ -960,30 +1031,27 @@ func (fake *FakeServiceProvider) UpdateArgsForCall(i int) (context.Context, *var return argsForCall.arg1, argsForCall.arg2 } -func (fake *FakeServiceProvider) UpdateReturns(result1 models.ServiceInstanceDetails, result2 error) { +func (fake *FakeServiceProvider) UpdateReturns(result1 error) { fake.updateMutex.Lock() defer fake.updateMutex.Unlock() fake.UpdateStub = nil fake.updateReturns = struct { - result1 models.ServiceInstanceDetails - result2 error - }{result1, result2} + result1 error + }{result1} } -func (fake *FakeServiceProvider) UpdateReturnsOnCall(i int, result1 models.ServiceInstanceDetails, result2 error) { +func (fake *FakeServiceProvider) UpdateReturnsOnCall(i int, result1 error) { fake.updateMutex.Lock() defer fake.updateMutex.Unlock() fake.UpdateStub = nil if fake.updateReturnsOnCall == nil { fake.updateReturnsOnCall = make(map[int]struct { - result1 models.ServiceInstanceDetails - result2 error + result1 error }) } fake.updateReturnsOnCall[i] = struct { - result1 models.ServiceInstanceDetails - result2 error - }{result1, result2} + result1 error + }{result1} } func (fake *FakeServiceProvider) UpgradeBindings(arg1 context.Context, arg2 *varcontext.VarContext, arg3 []*varcontext.VarContext) error { @@ -1128,6 +1196,8 @@ func (fake *FakeServiceProvider) Invocations() map[string][][]interface{} { defer fake.checkOperationConstraintsMutex.RUnlock() fake.checkUpgradeAvailableMutex.RLock() defer fake.checkUpgradeAvailableMutex.RUnlock() + fake.clearOperationTypeMutex.RLock() + defer fake.clearOperationTypeMutex.RUnlock() fake.deleteBindingDataMutex.RLock() defer fake.deleteBindingDataMutex.RUnlock() fake.deleteInstanceDataMutex.RLock() diff --git a/pkg/broker/service_provider.go b/pkg/broker/service_provider.go index f28cc0758..60532ceb4 100644 --- a/pkg/broker/service_provider.go +++ b/pkg/broker/service_provider.go @@ -18,7 +18,6 @@ import ( "context" "sync" - "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/varcontext" ) @@ -33,10 +32,10 @@ import ( type ServiceProvider interface { // Provision creates the necessary resources that an instance of this service // needs to operate. - Provision(ctx context.Context, provisionContext *varcontext.VarContext) (storage.ServiceInstanceDetails, error) + Provision(ctx context.Context, provisionContext *varcontext.VarContext) error // Update makes necessary updates to resources so they match new desired configuration - Update(ctx context.Context, updateContext *varcontext.VarContext) (models.ServiceInstanceDetails, error) + Update(ctx context.Context, updateContext *varcontext.VarContext) error UpgradeInstance(ctx context.Context, instanceContext *varcontext.VarContext) (*sync.WaitGroup, error) UpgradeBindings(ctx context.Context, instanceContext *varcontext.VarContext, bindingContexts []*varcontext.VarContext) error @@ -57,7 +56,7 @@ type ServiceProvider interface { // If no error and no operationId are returned, then the deprovision is expected to have been completed successfully. Deprovision(ctx context.Context, instanceGUID string, vc *varcontext.VarContext) (*string, error) - PollInstance(ctx context.Context, instanceGUID string) (bool, string, error) + PollInstance(ctx context.Context, instanceGUID string) (bool, string, string, error) GetTerraformOutputs(ctx context.Context, instanceGUID string) (storage.JSONObject, error) @@ -65,6 +64,8 @@ type ServiceProvider interface { DeleteBindingData(ctx context.Context, instanceGUID, bindingID string) error + ClearOperationType(ctx context.Context, instanceGUID string) error + CheckUpgradeAvailable(deploymentID string) error CheckOperationConstraints(deploymentID string, operationType string) error diff --git a/pkg/providers/tf/bind.go b/pkg/providers/tf/bind.go index 286c1053d..076473c20 100644 --- a/pkg/providers/tf/bind.go +++ b/pkg/providers/tf/bind.go @@ -18,11 +18,13 @@ func (provider *TerraformProvider) Bind(ctx context.Context, bindContext *varcon "context": bindContext.ToMap(), }) - tfID, err := provider.create(ctx, bindContext, provider.serviceDefinition.BindSettings, models.BindOperationType) + err := provider.create(ctx, bindContext, provider.serviceDefinition.BindSettings, models.BindOperationType) if err != nil { return nil, fmt.Errorf("error from provider bind: %w", err) } + tfID := bindContext.GetString("tf_id") + if err := provider.Wait(ctx, tfID); err != nil { return nil, fmt.Errorf("error waiting for result: %w", err) } diff --git a/pkg/providers/tf/bind_test.go b/pkg/providers/tf/bind_test.go index 2217542e2..536ceccd2 100644 --- a/pkg/providers/tf/bind_test.go +++ b/pkg/providers/tf/bind_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/workspace/workspacefakes" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/workspace" @@ -69,7 +70,7 @@ var _ = Describe("Bind", func() { It("returns the binding output", func() { fakeDeploymentManager.CreateAndSaveDeploymentReturns(deployment, nil) fakeDeploymentManager.GetTerraformDeploymentReturns(deployment, nil) - fakeDeploymentManager.OperationStatusReturns(true, "operation succeeded", nil) + fakeDeploymentManager.OperationStatusReturns(true, "operation succeeded", models.BindOperationType, nil) fakeInvokerBuilder.VersionedTerraformInvokerReturns(fakeDefaultInvoker) fakeDefaultInvoker.ApplyReturns(nil) fakeTerraformWorkspace.OutputsReturns(map[string]any{"username": "some-user"}, nil) @@ -153,7 +154,7 @@ var _ = Describe("Bind", func() { It("returns the error in last operation, if tofu apply fails", func() { fakeDeploymentManager.CreateAndSaveDeploymentReturns(deployment, nil) - fakeDeploymentManager.OperationStatusReturns(true, "operation failed", fmt.Errorf("tofu apply failed")) + fakeDeploymentManager.OperationStatusReturns(true, "operation failed", models.BindOperationType, fmt.Errorf("tofu apply failed")) fakeDeploymentManager.MarkOperationFinishedReturns(errors.New("couldnt do this now")) fakeInvokerBuilder.VersionedTerraformInvokerReturns(fakeDefaultInvoker) fakeDefaultInvoker.ApplyReturns(errors.New("some TF issue happened")) diff --git a/pkg/providers/tf/clear_operation_type.go b/pkg/providers/tf/clear_operation_type.go new file mode 100644 index 000000000..e5edfc923 --- /dev/null +++ b/pkg/providers/tf/clear_operation_type.go @@ -0,0 +1,18 @@ +package tf + +import ( + "context" + + "code.cloudfoundry.org/lager/v3" + "github.com/cloudfoundry/cloud-service-broker/v2/utils/correlation" +) + +func (provider *TerraformProvider) ClearOperationType(ctx context.Context, instanceGUID string) error { + provider.logger.Debug("ClearOperationType", correlation.ID(ctx), lager.Data{ + "instance": instanceGUID, + }) + + tfID := generateTfID(instanceGUID, "") + + return provider.ResetOperationType(tfID) +} diff --git a/pkg/providers/tf/deployment_manager.go b/pkg/providers/tf/deployment_manager.go index 3a3f6c19b..55ae07e47 100644 --- a/pkg/providers/tf/deployment_manager.go +++ b/pkg/providers/tf/deployment_manager.go @@ -6,6 +6,7 @@ import ( "code.cloudfoundry.org/lager/v3" + "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/featureflags" @@ -86,19 +87,19 @@ func (d *DeploymentManager) MarkOperationFinished(deployment *storage.TerraformD return d.store.RemoveLockFile(deployment.ID) } -func (d *DeploymentManager) OperationStatus(deploymentID string) (bool, string, error) { +func (d *DeploymentManager) OperationStatus(deploymentID string) (bool, string, string, error) { deployment, err := d.store.GetTerraformDeployment(deploymentID) if err != nil { - return true, "", err + return true, "", "", err } switch deployment.LastOperationState { case Succeeded: - return true, deployment.LastOperationMessage, nil + return true, deployment.LastOperationMessage, deployment.LastOperationType, nil case Failed: - return true, deployment.LastOperationMessage, errors.New(deployment.LastOperationMessage) + return true, deployment.LastOperationMessage, deployment.LastOperationType, errors.New(deployment.LastOperationMessage) default: - return false, deployment.LastOperationMessage, nil + return false, deployment.LastOperationMessage, deployment.LastOperationType, nil } } @@ -157,3 +158,13 @@ func (d *DeploymentManager) GetBindingDeployments(deploymentID string) ([]storag } return bindingDeployments, nil } + +func (d *DeploymentManager) ResetOperationType(deploymentID string) error { + deployment, err := d.store.GetTerraformDeployment(deploymentID) + if err != nil { + return err + } + + deployment.LastOperationType = models.ClearOperationType + return d.store.StoreTerraformDeployment(deployment) +} diff --git a/pkg/providers/tf/deployment_manager_test.go b/pkg/providers/tf/deployment_manager_test.go index 30c8d5eb5..7f929c41e 100644 --- a/pkg/providers/tf/deployment_manager_test.go +++ b/pkg/providers/tf/deployment_manager_test.go @@ -5,6 +5,7 @@ import ( "code.cloudfoundry.org/lager/v3/lagertest" + "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker/brokerfakes" @@ -257,10 +258,11 @@ var _ = Describe("DeploymentManager", func() { } fakeStore.GetTerraformDeploymentReturns(existingDeployment, nil) - completed, lastOpMessage, err := deploymentManager.OperationStatus(existingDeploymentID) + completed, lastOpMessage, operationType, err := deploymentManager.OperationStatus(existingDeploymentID) Expect(err).NotTo(HaveOccurred()) Expect(completed).To(BeTrue()) + Expect(operationType).To(Equal("update")) Expect(lastOpMessage).To(Equal("great update")) }) }) @@ -275,10 +277,11 @@ var _ = Describe("DeploymentManager", func() { } fakeStore.GetTerraformDeploymentReturns(existingDeployment, nil) - completed, lastOpMessage, err := deploymentManager.OperationStatus(existingDeploymentID) + completed, lastOpMessage, operationType, err := deploymentManager.OperationStatus(existingDeploymentID) Expect(err).To(MatchError("not so great update")) Expect(completed).To(BeTrue()) + Expect(operationType).To(Equal("update")) Expect(lastOpMessage).To(Equal("not so great update")) }) }) @@ -292,10 +295,11 @@ var _ = Describe("DeploymentManager", func() { } fakeStore.GetTerraformDeploymentReturns(existingDeployment, nil) - completed, lastOpMessage, err := deploymentManager.OperationStatus(existingDeploymentID) + completed, lastOpMessage, operationType, err := deploymentManager.OperationStatus(existingDeploymentID) Expect(err).NotTo(HaveOccurred()) Expect(completed).To(BeFalse()) + Expect(operationType).To(Equal("update")) Expect(lastOpMessage).To(Equal("still doing stuff")) }) }) @@ -303,7 +307,7 @@ var _ = Describe("DeploymentManager", func() { It("fails, when it errors getting the deployment", func() { fakeStore.GetTerraformDeploymentReturns(storage.TerraformDeployment{}, errors.New("cant get it now")) - _, _, err := deploymentManager.OperationStatus(existingDeploymentID) + _, _, _, err := deploymentManager.OperationStatus(existingDeploymentID) Expect(err).To(MatchError("cant get it now")) }) @@ -604,6 +608,36 @@ var _ = Describe("DeploymentManager", func() { }) }) + Describe("ResetOperationType", func() { + var ( + fakeStore brokerfakes.FakeServiceProviderStorage + deploymentManager *tf.DeploymentManager + existingDeployment storage.TerraformDeployment + ) + + const existingDeploymentID = "tf:instance:binding" + + BeforeEach(func() { + fakeStore = brokerfakes.FakeServiceProviderStorage{} + deploymentManager = tf.NewDeploymentManager(&fakeStore, lagertest.NewTestLogger("test")) + existingDeployment = storage.TerraformDeployment{ + ID: existingDeploymentID, + LastOperationType: "validation", + } + }) + + It("it clears the operation type", func() { + fakeStore.GetTerraformDeploymentReturns(existingDeployment, nil) + + err := deploymentManager.ResetOperationType(existingDeploymentID) + + Expect(err).NotTo(HaveOccurred()) + Expect(fakeStore.StoreTerraformDeploymentCallCount()).To(Equal(1)) + actualDeployment := fakeStore.StoreTerraformDeploymentArgsForCall(0) + Expect(actualDeployment.LastOperationType).To(Equal(models.ClearOperationType)) + }) + }) + Describe("GetBindingDeployments", func() { var ( fakeStore brokerfakes.FakeServiceProviderStorage diff --git a/pkg/providers/tf/poll_instance.go b/pkg/providers/tf/poll_instance.go index fb01548d0..8d0a39d5e 100644 --- a/pkg/providers/tf/poll_instance.go +++ b/pkg/providers/tf/poll_instance.go @@ -5,6 +5,6 @@ import ( ) // PollInstance returns the instance status of the backing job. -func (provider *TerraformProvider) PollInstance(_ context.Context, instanceGUID string) (bool, string, error) { +func (provider *TerraformProvider) PollInstance(_ context.Context, instanceGUID string) (bool, string, string, error) { return provider.OperationStatus(generateTfID(instanceGUID, "")) } diff --git a/pkg/providers/tf/poll_instance_test.go b/pkg/providers/tf/poll_instance_test.go index 7e593fd2a..104fdf9dc 100644 --- a/pkg/providers/tf/poll_instance_test.go +++ b/pkg/providers/tf/poll_instance_test.go @@ -17,13 +17,14 @@ var _ = Describe("PollInstance", func() { fakeInvokerBuilder := &tffakes.FakeTerraformInvokerBuilder{} fakeLogger := utils.NewLogger("test") - fakeDeploymentManager.OperationStatusReturns(true, "LO message", nil) + fakeDeploymentManager.OperationStatusReturns(true, "LO message", "update", nil) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, tf.TfServiceDefinitionV1{}, fakeDeploymentManager) - finished, message, err := provider.PollInstance(context.TODO(), "instance-guid") + finished, message, operationType, err := provider.PollInstance(context.TODO(), "instance-guid") Expect(err).NotTo(HaveOccurred()) Expect(finished).To(BeTrue()) + Expect(operationType).To(Equal("update")) Expect(message).To(Equal("LO message")) Expect(fakeDeploymentManager.OperationStatusCallCount()).To(Equal(1)) diff --git a/pkg/providers/tf/provider.go b/pkg/providers/tf/provider.go index 6e2eaeae7..9a828a083 100644 --- a/pkg/providers/tf/provider.go +++ b/pkg/providers/tf/provider.go @@ -68,25 +68,25 @@ func (provider *TerraformProvider) VersionedInvoker(version *version.Version) in return provider.VersionedTerraformInvoker(version) } -func (provider *TerraformProvider) create(ctx context.Context, vars *varcontext.VarContext, action TfServiceDefinitionV1Action, operationType string) (string, error) { +func (provider *TerraformProvider) create(ctx context.Context, vars *varcontext.VarContext, action TfServiceDefinitionV1Action, operationType string) error { tfID := vars.GetString("tf_id") if err := vars.Error(); err != nil { - return "", err + return err } newWorkspace, err := workspace.NewWorkspace(vars.ToMap(), action.Template, action.Templates, []workspace.ParameterMapping{}, []string{}, []workspace.ParameterMapping{}) if err != nil { - return tfID, fmt.Errorf("error creating workspace: %w", err) + return fmt.Errorf("error creating workspace: %w", err) } deployment, err := provider.CreateAndSaveDeployment(tfID, newWorkspace) if err != nil { provider.logger.Error("deployment create failed", err) - return tfID, fmt.Errorf("deployment create failed: %w", err) + return fmt.Errorf("deployment create failed: %w", err) } if err := provider.MarkOperationStarted(&deployment, operationType); err != nil { - return tfID, fmt.Errorf("error marking job started: %w", err) + return fmt.Errorf("error marking job started: %w", err) } go func() { @@ -102,7 +102,7 @@ func (provider *TerraformProvider) create(ctx context.Context, vars *varcontext. } }() - return tfID, nil + return nil } func (provider *TerraformProvider) destroy(ctx context.Context, deploymentID string, templateVars map[string]any, operationType string) error { @@ -129,6 +129,7 @@ func (provider *TerraformProvider) destroy(ctx context.Context, deploymentID str tfWorkspace.Instances[0].Configuration = limitedConfig + fmt.Println("provider.MarkOperationStarted") if err := provider.MarkOperationStarted(&deployment, operationType); err != nil { return err } @@ -148,7 +149,7 @@ func (provider *TerraformProvider) Wait(ctx context.Context, id string) error { return nil case <-time.After(1 * time.Second): - isDone, _, err := provider.OperationStatus(id) + isDone, _, _, err := provider.OperationStatus(id) if isDone { return err } @@ -163,8 +164,9 @@ type DeploymentManagerInterface interface { CreateAndSaveDeployment(deploymentID string, workspace *workspace.TerraformWorkspace) (storage.TerraformDeployment, error) MarkOperationStarted(deployment *storage.TerraformDeployment, operationType string) error MarkOperationFinished(deployment *storage.TerraformDeployment, err error) error - OperationStatus(deploymentID string) (bool, string, error) + OperationStatus(deploymentID string) (bool, string, string, error) UpdateWorkspaceHCL(deploymentID string, serviceDefinitionAction TfServiceDefinitionV1Action, templateVars map[string]any) error GetBindingDeployments(deploymentID string) ([]storage.TerraformDeployment, error) DeleteTerraformDeployment(deploymentID string) error + ResetOperationType(deploymentID string) error } diff --git a/pkg/providers/tf/provision.go b/pkg/providers/tf/provision.go index d4c499462..1bdb7a545 100644 --- a/pkg/providers/tf/provision.go +++ b/pkg/providers/tf/provision.go @@ -8,7 +8,6 @@ import ( "code.cloudfoundry.org/lager/v3" "github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models" "github.com/cloudfoundry/cloud-service-broker/v2/internal/steps" - "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/invoker" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/workspace" @@ -25,42 +24,36 @@ type ImportResource struct { // Provision creates the necessary resources that an instance of this service // needs to operate. -func (provider *TerraformProvider) Provision(ctx context.Context, provisionContext *varcontext.VarContext) (storage.ServiceInstanceDetails, error) { +func (provider *TerraformProvider) Provision(ctx context.Context, provisionContext *varcontext.VarContext) error { provider.logger.Debug("terraform-provision", correlation.ID(ctx), lager.Data{ "context": provisionContext.ToMap(), }) - var ( - tfID string - err error - ) + var err error switch provider.serviceDefinition.ProvisionSettings.IsTfImport(provisionContext) { case true: - tfID, err = provider.importCreate(ctx, provisionContext, provider.serviceDefinition.ProvisionSettings) + err = provider.importCreate(ctx, provisionContext, provider.serviceDefinition.ProvisionSettings) default: - tfID, err = provider.create(ctx, provisionContext, provider.serviceDefinition.ProvisionSettings, models.ProvisionOperationType) + err = provider.create(ctx, provisionContext, provider.serviceDefinition.ProvisionSettings, models.ProvisionOperationType) } if err != nil { - return storage.ServiceInstanceDetails{}, err + return err } - return storage.ServiceInstanceDetails{ - OperationGUID: tfID, - OperationType: models.ProvisionOperationType, - }, nil + return nil } -func (provider *TerraformProvider) importCreate(ctx context.Context, vars *varcontext.VarContext, action TfServiceDefinitionV1Action) (string, error) { +func (provider *TerraformProvider) importCreate(ctx context.Context, vars *varcontext.VarContext, action TfServiceDefinitionV1Action) error { varsMap := vars.ToMap() importParams, err := validateImportParams(action.ImportVariables, varsMap) if err != nil { - return "", err + return err } tfID := vars.GetString("tf_id") if err := vars.Error(); err != nil { - return "", err + return err } newWorkspace, err := workspace.NewWorkspace( @@ -71,17 +64,17 @@ func (provider *TerraformProvider) importCreate(ctx context.Context, vars *varco action.ImportParametersToDelete, evaluateParametersToAdd(action.ImportParametersToAdd)) if err != nil { - return tfID, fmt.Errorf("error creating workspace: %w", err) + return fmt.Errorf("error creating workspace: %w", err) } deployment, err := provider.CreateAndSaveDeployment(tfID, newWorkspace) if err != nil { provider.logger.Error("deployment create failed", err) - return tfID, fmt.Errorf("deployment create failed: %w", err) + return fmt.Errorf("deployment create failed: %w", err) } if err := provider.MarkOperationStarted(&deployment, models.ProvisionOperationType); err != nil { - return tfID, fmt.Errorf("error marking job started: %w", err) + return fmt.Errorf("error marking job started: %w", err) } go func() { @@ -122,7 +115,7 @@ func (provider *TerraformProvider) importCreate(ctx context.Context, vars *varco } }() - return tfID, nil + return nil } func validateImportParams(importVariables []broker.ImportVariable, varsMap map[string]any) ([]ImportResource, error) { diff --git a/pkg/providers/tf/provision_test.go b/pkg/providers/tf/provision_test.go index 8aaef029d..75abe6ed8 100644 --- a/pkg/providers/tf/provision_test.go +++ b/pkg/providers/tf/provision_test.go @@ -67,11 +67,9 @@ var _ = Describe("Provision", func() { fakeInvokerBuilder.VersionedTerraformInvokerReturns(fakeDefaultInvoker) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - actualInstanceDetails, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).NotTo(HaveOccurred()) - Expect(actualInstanceDetails.OperationGUID).To(Equal(expectedTfID)) - Expect(actualInstanceDetails.OperationType).To(Equal("provision")) By("checking the new saved deployment") Expect(fakeDeploymentManager.CreateAndSaveDeploymentCallCount()).To(Equal(1)) @@ -104,7 +102,7 @@ var _ = Describe("Provision", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err = provider.Provision(context.TODO(), provisionContext) + err = provider.Provision(context.TODO(), provisionContext) Expect(err.Error()).To(ContainSubstring(`missing value for key "tf_id"`)) }) @@ -119,7 +117,7 @@ var _ = Describe("Provision", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).To(MatchError(`error creating workspace: :1,17-17: Invalid block definition; Either a quoted string block label or an opening brace ("{") is expected here.`)) }) @@ -128,7 +126,7 @@ var _ = Describe("Provision", func() { fakeDeploymentManager.CreateAndSaveDeploymentReturns(storage.TerraformDeployment{}, errors.New("cant save now")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).To(MatchError("deployment create failed: cant save now")) }) @@ -138,7 +136,7 @@ var _ = Describe("Provision", func() { fakeDeploymentManager.MarkOperationStartedReturns(errors.New("couldnt do this now")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).To(MatchError("error marking job started: couldnt do this now")) }) @@ -149,7 +147,7 @@ var _ = Describe("Provision", func() { fakeDefaultInvoker.ApplyReturns(errors.New("some TF issue happened")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).NotTo(HaveOccurred()) By("checking TF apply has been called") @@ -206,11 +204,9 @@ var _ = Describe("Provision", func() { fakeInvokerBuilder.VersionedTerraformInvokerReturns(fakeDefaultInvoker) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - actualInstanceDetails, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).NotTo(HaveOccurred()) - Expect(actualInstanceDetails.OperationGUID).To(Equal(expectedTfID)) - Expect(actualInstanceDetails.OperationType).To(Equal("provision")) By("checking the new saved deployment") Expect(fakeDeploymentManager.CreateAndSaveDeploymentCallCount()).To(Equal(1)) @@ -249,7 +245,7 @@ var _ = Describe("Provision", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err = provider.Provision(context.TODO(), pc) + err = provider.Provision(context.TODO(), pc) Expect(err).To(MatchError("must provide values for all import parameters: import_input")) }) @@ -260,7 +256,7 @@ var _ = Describe("Provision", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err = provider.Provision(context.TODO(), pc) + err = provider.Provision(context.TODO(), pc) Expect(err.Error()).To(ContainSubstring(`missing value for key "tf_id"`)) }) @@ -298,7 +294,7 @@ var _ = Describe("Provision", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, sd, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err.Error()).To(ContainSubstring(`error creating workspace: :1,17-17: Invalid block definition; Either a quoted string block label or an opening brace ("{") is expected here.`)) }) @@ -307,7 +303,7 @@ var _ = Describe("Provision", func() { fakeDeploymentManager.CreateAndSaveDeploymentReturns(storage.TerraformDeployment{}, errors.New("cant save now")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).To(MatchError("deployment create failed: cant save now")) }) @@ -317,7 +313,7 @@ var _ = Describe("Provision", func() { fakeDeploymentManager.MarkOperationStartedReturns(errors.New("couldnt do this now")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).To(MatchError("error marking job started: couldnt do this now")) }) @@ -329,7 +325,7 @@ var _ = Describe("Provision", func() { fakeDefaultInvoker.ImportReturns(errors.New("some TF import issue happened")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).NotTo(HaveOccurred()) By("checking TF import has been called") @@ -347,7 +343,7 @@ var _ = Describe("Provision", func() { fakeDefaultInvoker.ShowReturns("", errors.New("some TF show issue happened")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).NotTo(HaveOccurred()) By("checking TF show has been called") @@ -364,7 +360,7 @@ var _ = Describe("Provision", func() { fakeDefaultInvoker.PlanReturns(executor.ExecutionOutput{}, errors.New("some TF plan issue happened")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).NotTo(HaveOccurred()) By("checking TF plan has been called") @@ -380,7 +376,7 @@ var _ = Describe("Provision", func() { fakeDefaultInvoker.ApplyReturns(errors.New("some TF apply issue happened")) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Provision(context.TODO(), provisionContext) + err := provider.Provision(context.TODO(), provisionContext) Expect(err).NotTo(HaveOccurred()) By("checking TF apply has been called") diff --git a/pkg/providers/tf/tffakes/fake_deployment_manager_interface.go b/pkg/providers/tf/tffakes/fake_deployment_manager_interface.go index 60a2a8c6a..9a2ea75a0 100644 --- a/pkg/providers/tf/tffakes/fake_deployment_manager_interface.go +++ b/pkg/providers/tf/tffakes/fake_deployment_manager_interface.go @@ -85,7 +85,7 @@ type FakeDeploymentManagerInterface struct { markOperationStartedReturnsOnCall map[int]struct { result1 error } - OperationStatusStub func(string) (bool, string, error) + OperationStatusStub func(string) (bool, string, string, error) operationStatusMutex sync.RWMutex operationStatusArgsForCall []struct { arg1 string @@ -93,12 +93,25 @@ type FakeDeploymentManagerInterface struct { operationStatusReturns struct { result1 bool result2 string - result3 error + result3 string + result4 error } operationStatusReturnsOnCall map[int]struct { result1 bool result2 string - result3 error + result3 string + result4 error + } + ResetOperationTypeStub func(string) error + resetOperationTypeMutex sync.RWMutex + resetOperationTypeArgsForCall []struct { + arg1 string + } + resetOperationTypeReturns struct { + result1 error + } + resetOperationTypeReturnsOnCall map[int]struct { + result1 error } UpdateWorkspaceHCLStub func(string, tf.TfServiceDefinitionV1Action, map[string]any) error updateWorkspaceHCLMutex sync.RWMutex @@ -495,7 +508,7 @@ func (fake *FakeDeploymentManagerInterface) MarkOperationStartedReturnsOnCall(i }{result1} } -func (fake *FakeDeploymentManagerInterface) OperationStatus(arg1 string) (bool, string, error) { +func (fake *FakeDeploymentManagerInterface) OperationStatus(arg1 string) (bool, string, string, error) { fake.operationStatusMutex.Lock() ret, specificReturn := fake.operationStatusReturnsOnCall[len(fake.operationStatusArgsForCall)] fake.operationStatusArgsForCall = append(fake.operationStatusArgsForCall, struct { @@ -509,9 +522,9 @@ func (fake *FakeDeploymentManagerInterface) OperationStatus(arg1 string) (bool, return stub(arg1) } if specificReturn { - return ret.result1, ret.result2, ret.result3 + return ret.result1, ret.result2, ret.result3, ret.result4 } - return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3, fakeReturns.result4 } func (fake *FakeDeploymentManagerInterface) OperationStatusCallCount() int { @@ -520,7 +533,7 @@ func (fake *FakeDeploymentManagerInterface) OperationStatusCallCount() int { return len(fake.operationStatusArgsForCall) } -func (fake *FakeDeploymentManagerInterface) OperationStatusCalls(stub func(string) (bool, string, error)) { +func (fake *FakeDeploymentManagerInterface) OperationStatusCalls(stub func(string) (bool, string, string, error)) { fake.operationStatusMutex.Lock() defer fake.operationStatusMutex.Unlock() fake.OperationStatusStub = stub @@ -533,18 +546,19 @@ func (fake *FakeDeploymentManagerInterface) OperationStatusArgsForCall(i int) st return argsForCall.arg1 } -func (fake *FakeDeploymentManagerInterface) OperationStatusReturns(result1 bool, result2 string, result3 error) { +func (fake *FakeDeploymentManagerInterface) OperationStatusReturns(result1 bool, result2 string, result3 string, result4 error) { fake.operationStatusMutex.Lock() defer fake.operationStatusMutex.Unlock() fake.OperationStatusStub = nil fake.operationStatusReturns = struct { result1 bool result2 string - result3 error - }{result1, result2, result3} + result3 string + result4 error + }{result1, result2, result3, result4} } -func (fake *FakeDeploymentManagerInterface) OperationStatusReturnsOnCall(i int, result1 bool, result2 string, result3 error) { +func (fake *FakeDeploymentManagerInterface) OperationStatusReturnsOnCall(i int, result1 bool, result2 string, result3 string, result4 error) { fake.operationStatusMutex.Lock() defer fake.operationStatusMutex.Unlock() fake.OperationStatusStub = nil @@ -552,14 +566,77 @@ func (fake *FakeDeploymentManagerInterface) OperationStatusReturnsOnCall(i int, fake.operationStatusReturnsOnCall = make(map[int]struct { result1 bool result2 string - result3 error + result3 string + result4 error }) } fake.operationStatusReturnsOnCall[i] = struct { result1 bool result2 string - result3 error - }{result1, result2, result3} + result3 string + result4 error + }{result1, result2, result3, result4} +} + +func (fake *FakeDeploymentManagerInterface) ResetOperationType(arg1 string) error { + fake.resetOperationTypeMutex.Lock() + ret, specificReturn := fake.resetOperationTypeReturnsOnCall[len(fake.resetOperationTypeArgsForCall)] + fake.resetOperationTypeArgsForCall = append(fake.resetOperationTypeArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ResetOperationTypeStub + fakeReturns := fake.resetOperationTypeReturns + fake.recordInvocation("ResetOperationType", []interface{}{arg1}) + fake.resetOperationTypeMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeDeploymentManagerInterface) ResetOperationTypeCallCount() int { + fake.resetOperationTypeMutex.RLock() + defer fake.resetOperationTypeMutex.RUnlock() + return len(fake.resetOperationTypeArgsForCall) +} + +func (fake *FakeDeploymentManagerInterface) ResetOperationTypeCalls(stub func(string) error) { + fake.resetOperationTypeMutex.Lock() + defer fake.resetOperationTypeMutex.Unlock() + fake.ResetOperationTypeStub = stub +} + +func (fake *FakeDeploymentManagerInterface) ResetOperationTypeArgsForCall(i int) string { + fake.resetOperationTypeMutex.RLock() + defer fake.resetOperationTypeMutex.RUnlock() + argsForCall := fake.resetOperationTypeArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeDeploymentManagerInterface) ResetOperationTypeReturns(result1 error) { + fake.resetOperationTypeMutex.Lock() + defer fake.resetOperationTypeMutex.Unlock() + fake.ResetOperationTypeStub = nil + fake.resetOperationTypeReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeDeploymentManagerInterface) ResetOperationTypeReturnsOnCall(i int, result1 error) { + fake.resetOperationTypeMutex.Lock() + defer fake.resetOperationTypeMutex.Unlock() + fake.ResetOperationTypeStub = nil + if fake.resetOperationTypeReturnsOnCall == nil { + fake.resetOperationTypeReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.resetOperationTypeReturnsOnCall[i] = struct { + result1 error + }{result1} } func (fake *FakeDeploymentManagerInterface) UpdateWorkspaceHCL(arg1 string, arg2 tf.TfServiceDefinitionV1Action, arg3 map[string]any) error { @@ -642,6 +719,8 @@ func (fake *FakeDeploymentManagerInterface) Invocations() map[string][][]interfa defer fake.markOperationStartedMutex.RUnlock() fake.operationStatusMutex.RLock() defer fake.operationStatusMutex.RUnlock() + fake.resetOperationTypeMutex.RLock() + defer fake.resetOperationTypeMutex.RUnlock() fake.updateWorkspaceHCLMutex.RLock() defer fake.updateWorkspaceHCLMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} diff --git a/pkg/providers/tf/unbind_test.go b/pkg/providers/tf/unbind_test.go index ed24c5bec..9919f197d 100644 --- a/pkg/providers/tf/unbind_test.go +++ b/pkg/providers/tf/unbind_test.go @@ -79,7 +79,7 @@ var _ = Describe("Unbind", func() { It("destroys the binding", func() { fakeInvokerBuilder.VersionedTerraformInvokerReturns(fakeDefaultInvoker) fakeDeploymentManager.GetTerraformDeploymentReturns(deployment, nil) - fakeDeploymentManager.OperationStatusReturns(true, "operation succeeded", nil) + fakeDeploymentManager.OperationStatusReturns(true, "operation succeeded", "bind", nil) provider := tf.NewTerraformProvider(executor.TFBinariesContext{DefaultTfVersion: version.Must(version.NewVersion("1"))}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) @@ -141,7 +141,7 @@ var _ = Describe("Unbind", func() { fakeDeploymentManager.MarkOperationStartedReturns(nil) fakeInvokerBuilder.VersionedTerraformInvokerReturns(fakeDefaultInvoker) fakeDefaultInvoker.DestroyReturns(errors.New(expectedError)) - fakeDeploymentManager.OperationStatusReturns(true, "", errors.New(expectedError)) + fakeDeploymentManager.OperationStatusReturns(true, "", "destroy", errors.New(expectedError)) provider := tf.NewTerraformProvider(executor.TFBinariesContext{DefaultTfVersion: version.Must(version.NewVersion("1"))}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) diff --git a/pkg/providers/tf/update.go b/pkg/providers/tf/update.go index d3295c3fc..87166ab6a 100644 --- a/pkg/providers/tf/update.go +++ b/pkg/providers/tf/update.go @@ -12,33 +12,33 @@ import ( ) // Update makes necessary updates to resources, so they match new desired configuration -func (provider *TerraformProvider) Update(ctx context.Context, updateContext *varcontext.VarContext) (models.ServiceInstanceDetails, error) { +func (provider *TerraformProvider) Update(ctx context.Context, updateContext *varcontext.VarContext) error { provider.logger.Debug("update", correlation.ID(ctx), lager.Data{ "context": updateContext.ToMap(), }) if provider.serviceDefinition.ProvisionSettings.IsTfImport(updateContext) { - return models.ServiceInstanceDetails{}, fmt.Errorf("cannot update to subsume plan\n\nFor OpsMan Tile users see documentation here: https://via.vmw.com/ENs4\n\nFor Open Source users deployed via 'cf push' see documentation here: https://via.vmw.com/ENw4") + return fmt.Errorf("cannot update to subsume plan\n\nFor OpsMan Tile users see documentation here: https://via.vmw.com/ENs4\n\nFor Open Source users deployed via 'cf push' see documentation here: https://via.vmw.com/ENw4") } tfID := updateContext.GetString("tf_id") if err := updateContext.Error(); err != nil { - return models.ServiceInstanceDetails{}, err + return err } if err := provider.UpdateWorkspaceHCL(tfID, provider.serviceDefinition.ProvisionSettings, updateContext.ToMap()); err != nil { - return models.ServiceInstanceDetails{}, err + return err } deployment, err := provider.GetTerraformDeployment(tfID) if err != nil { - return models.ServiceInstanceDetails{}, err + return err } workspace := deployment.Workspace if err := provider.MarkOperationStarted(&deployment, models.UpdateOperationType); err != nil { - return models.ServiceInstanceDetails{}, err + return err } go func() { @@ -52,8 +52,5 @@ func (provider *TerraformProvider) Update(ctx context.Context, updateContext *va _ = provider.MarkOperationFinished(&deployment, err) }() - return models.ServiceInstanceDetails{ - OperationID: tfID, - OperationType: models.UpdateOperationType, - }, nil + return nil } diff --git a/pkg/providers/tf/update_test.go b/pkg/providers/tf/update_test.go index b3d0377a6..74bd9e78f 100644 --- a/pkg/providers/tf/update_test.go +++ b/pkg/providers/tf/update_test.go @@ -63,7 +63,7 @@ var _ = Describe("Update", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{DefaultTfVersion: newVersion(tfVersion)}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Update(context.TODO(), varContext) + err := provider.Update(context.TODO(), varContext) Expect(err).NotTo(HaveOccurred()) Eventually(operationWasFinishedForDeployment(fakeDeploymentManager)).Should(Equal(deployment)) Expect(operationWasFinishedWithError(fakeDeploymentManager)()).To(BeNil()) @@ -81,7 +81,7 @@ var _ = Describe("Update", func() { fakeWorkspace.OutputsReturns(map[string]any{"status": "status from terraform"}, nil) provider := tf.NewTerraformProvider(executor.TFBinariesContext{DefaultTfVersion: newVersion(tfVersion)}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Update(context.TODO(), varContext) + err := provider.Update(context.TODO(), varContext) Expect(err).To(Succeed()) Eventually(operationWasFinishedForDeployment(fakeDeploymentManager)).Should(Equal(deployment)) Expect(operationWasFinishedWithError(fakeDeploymentManager)()).To(BeNil()) @@ -96,7 +96,7 @@ var _ = Describe("Update", func() { fakeDefaultInvoker.ApplyReturns(genericError) provider := tf.NewTerraformProvider(executor.TFBinariesContext{DefaultTfVersion: newVersion(tfVersion)}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Update(context.TODO(), varContext) + err := provider.Update(context.TODO(), varContext) Expect(err).To(Succeed()) Eventually(operationWasFinishedForDeployment(fakeDeploymentManager)).Should(Equal(deployment)) @@ -115,7 +115,7 @@ var _ = Describe("Update", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err = provider.Update(context.TODO(), varContext) + err = provider.Update(context.TODO(), varContext) Expect(err).To(MatchError("cannot update to subsume plan\n\nFor OpsMan Tile users see documentation here: https://via.vmw.com/ENs4\n\nFor Open Source users deployed via 'cf push' see documentation here: https://via.vmw.com/ENw4")) }) }) @@ -127,7 +127,7 @@ var _ = Describe("Update", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err = provider.Update(context.TODO(), varContext) + err = provider.Update(context.TODO(), varContext) Expect(err).To(MatchError(`1 error(s) occurred: missing value for key "tf_id"`)) }) }) @@ -138,7 +138,7 @@ var _ = Describe("Update", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Update(context.TODO(), varContext) + err := provider.Update(context.TODO(), varContext) Expect(err).To(MatchError(genericError)) }) }) @@ -148,7 +148,7 @@ var _ = Describe("Update", func() { fakeDeploymentManager.GetTerraformDeploymentReturns(storage.TerraformDeployment{}, genericError) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Update(context.TODO(), varContext) + err := provider.Update(context.TODO(), varContext) Expect(err).To(MatchError(genericError)) }) }) @@ -159,7 +159,7 @@ var _ = Describe("Update", func() { fakeDeploymentManager.MarkOperationStartedReturns(genericError) provider := tf.NewTerraformProvider(executor.TFBinariesContext{}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Update(context.TODO(), varContext) + err := provider.Update(context.TODO(), varContext) Expect(err).To(MatchError(genericError)) }) @@ -175,7 +175,7 @@ var _ = Describe("Update", func() { provider := tf.NewTerraformProvider(executor.TFBinariesContext{DefaultTfVersion: newVersion(tfVersion)}, fakeInvokerBuilder, fakeLogger, fakeServiceDefinition, fakeDeploymentManager) - _, err := provider.Update(context.TODO(), varContext) + err := provider.Update(context.TODO(), varContext) Expect(err).NotTo(HaveOccurred()) Eventually(operationWasFinishedForDeployment(fakeDeploymentManager)).Should(Equal(deployment)) diff --git a/pkg/providers/tf/workspace/workspace.go b/pkg/providers/tf/workspace/workspace.go index 2da28988f..a54073c87 100644 --- a/pkg/providers/tf/workspace/workspace.go +++ b/pkg/providers/tf/workspace/workspace.go @@ -25,18 +25,27 @@ import ( "strings" "sync" + "github.com/spf13/viper" + "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/command" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/executor" "github.com/hashicorp/go-version" ) -// DefaultInstanceName is the default name of an instance of a particular module. const ( - DefaultInstanceName = "instance" - binaryName = "tofu" + // DefaultInstanceName is the default name of an instance of a particular module. + DefaultInstanceName = "instance" + binaryName = "tofu" + leaveWorkspaceDirectoryConfigKey = "debug.leave_workspace_dir" + leaveWorkspaceDirectoryEnvVar = "CSB_DEBUG_LEAVE_WORKSPACE_DIR" ) +func init() { + viper.BindEnv(leaveWorkspaceDirectoryConfigKey, leaveWorkspaceDirectoryEnvVar) + viper.SetDefault(leaveWorkspaceDirectoryConfigKey, false) +} + // NewWorkspace creates a new TerraformWorkspace from a given template and variables to populate an instance of it. // The created instance will have the name specified by the DefaultInstanceName constant. func NewWorkspace(templateVars map[string]any, @@ -287,8 +296,10 @@ func (workspace *TerraformWorkspace) teardownFs() error { workspace.State = bytes - if err := os.RemoveAll(workspace.dir); err != nil { - return err + if !viper.GetBool(leaveWorkspaceDirectoryConfigKey) { + if err := os.RemoveAll(workspace.dir); err != nil { + return err + } } workspace.dir = ""