diff --git a/integrationtest/binding_cleanup_test.go b/integrationtest/binding_cleanup_test.go new file mode 100644 index 000000000..83f62c70c --- /dev/null +++ b/integrationtest/binding_cleanup_test.go @@ -0,0 +1,71 @@ +package integrationtest_test + +import ( + "encoding/json" + "fmt" + + "github.com/cloudfoundry/cloud-service-broker/dbservice/models" + "github.com/cloudfoundry/cloud-service-broker/integrationtest/packer" + "github.com/cloudfoundry/cloud-service-broker/internal/testdrive" + "github.com/cloudfoundry/cloud-service-broker/pkg/providers/tf/workspace" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Binding Cleanup", func() { + const ( + badServiceOfferingGUID = "81b4ebaa-cc08-11ee-bd34-0f8547e428e9" + badServicePlanGUID = "9ff671e2-cc08-11ee-bb95-3facf049ac9d" + goodServiceOfferingGUID = "7779a92a-cc0b-11ee-85c4-4b4aa590c58a" + goodServicePlanGUID = "911ce91e-cc0b-11ee-a5e8-33dbc3f841a1" + ) + + var ( + brokerpak string + broker *testdrive.Broker + ) + + BeforeEach(func() { + brokerpak = must(packer.BuildBrokerpak(csb, fixtures("binding-cleanup"))) + broker = must(testdrive.StartBroker(csb, brokerpak, database, testdrive.WithOutputs(GinkgoWriter, GinkgoWriter))) + + DeferCleanup(func() { + Expect(broker.Stop()).To(Succeed()) + cleanup(brokerpak) + }) + }) + + It("does not need to clean up after a binding failed cleanly", func() { + By("provisioning successfully") + instance := must(broker.Provision(badServiceOfferingGUID, badServicePlanGUID)) + + By("failing to bind") + binding, err := broker.CreateBinding(instance) + Expect(err).To(MatchError(ContainSubstring("error performing bind: error waiting for result: bind failed: Error: Missing required argument"))) + + By("seeing an HTTP 410 Gone error") + Expect(broker.DeleteBinding(instance, binding.GUID)).To(MatchError(ContainSubstring("unexpected status code 410"))) + }) + + // This test captures an incorrect behavior that we want to fix + It("fails to clean up after a corrupted binding", func() { + By("provisioning successfully") + instance := must(broker.Provision(goodServiceOfferingGUID, goodServicePlanGUID)) + + By("binding successfully") + binding := must(broker.CreateBinding(instance)) + + By("corrupting the state as if terraform had been killed") + invalidWorkspace := must(json.Marshal(workspace.TerraformWorkspace{State: []byte(`{"foo`)})) // Base64-encoded truncated JSON + Expect( + dbConn.Model(&models.TerraformDeployment{}). + Where("id = ?", fmt.Sprintf("tf:%s:%s", instance.GUID, binding.GUID)). + Update("workspace", invalidWorkspace). + Error, + ).To(Succeed()) + + By("failing to clean up the binding") + Expect(broker.DeleteBinding(instance, binding.GUID)).To(MatchError(ContainSubstring("failed to unbind: invalid workspace state unexpected end of JSON input"))) + }) +}) diff --git a/integrationtest/fixtures/binding-cleanup/fake-bad-bind.tf b/integrationtest/fixtures/binding-cleanup/fake-bad-bind.tf new file mode 100644 index 000000000..9de1f243c --- /dev/null +++ b/integrationtest/fixtures/binding-cleanup/fake-bad-bind.tf @@ -0,0 +1,4 @@ +// Fails as required "length" parameter is missing +resource "random_string" "random" {} + +output bind_output { value = random_string.random.result } \ No newline at end of file diff --git a/integrationtest/fixtures/binding-cleanup/fake-bad-service.yml b/integrationtest/fixtures/binding-cleanup/fake-bad-service.yml new file mode 100644 index 000000000..e390a1cc7 --- /dev/null +++ b/integrationtest/fixtures/binding-cleanup/fake-bad-service.yml @@ -0,0 +1,27 @@ +version: 1 +name: fake-bad-service +id: 81b4ebaa-cc08-11ee-bd34-0f8547e428e9 +description: description +display_name: Fake +image_url: https://example.com/icon.jpg +documentation_url: https://example.com +support_url: https://example.com/support.html +plans: +- name: standard + id: 9ff671e2-cc08-11ee-bb95-3facf049ac9d + description: Standard plan + display_name: Standard +provision: + template_refs: + main: fake-provision.tf + outputs: + - field_name: provision_output + type: string + details: provision output +bind: + template_refs: + main: fake-bad-bind.tf + outputs: + - field_name: bind_output + type: string + details: bind output diff --git a/integrationtest/fixtures/binding-cleanup/fake-good-bind.tf b/integrationtest/fixtures/binding-cleanup/fake-good-bind.tf new file mode 100644 index 000000000..fcbed15be --- /dev/null +++ b/integrationtest/fixtures/binding-cleanup/fake-good-bind.tf @@ -0,0 +1,5 @@ +resource "random_string" "random" { + length = 10 +} + +output bind_output { value = random_string.random.result } \ No newline at end of file diff --git a/integrationtest/fixtures/binding-cleanup/fake-good-service.yml b/integrationtest/fixtures/binding-cleanup/fake-good-service.yml new file mode 100644 index 000000000..0a6b29dea --- /dev/null +++ b/integrationtest/fixtures/binding-cleanup/fake-good-service.yml @@ -0,0 +1,27 @@ +version: 1 +name: fake-good-service +id: 7779a92a-cc0b-11ee-85c4-4b4aa590c58a +description: description +display_name: Fake +image_url: https://example.com/icon.jpg +documentation_url: https://example.com +support_url: https://example.com/support.html +plans: +- name: standard + id: 911ce91e-cc0b-11ee-a5e8-33dbc3f841a1 + description: Standard plan + display_name: Standard +provision: + template_refs: + main: fake-provision.tf + outputs: + - field_name: provision_output + type: string + details: provision output +bind: + template_refs: + main: fake-good-bind.tf + outputs: + - field_name: bind_output + type: string + details: bind output diff --git a/integrationtest/fixtures/binding-cleanup/fake-provision.tf b/integrationtest/fixtures/binding-cleanup/fake-provision.tf new file mode 100644 index 000000000..adbe4074b --- /dev/null +++ b/integrationtest/fixtures/binding-cleanup/fake-provision.tf @@ -0,0 +1,3 @@ +resource "random_uuid" "random" {} + +output provision_output { value = random_uuid.random.result } \ No newline at end of file diff --git a/integrationtest/fixtures/binding-cleanup/manifest.yml b/integrationtest/fixtures/binding-cleanup/manifest.yml new file mode 100644 index 000000000..a712de76d --- /dev/null +++ b/integrationtest/fixtures/binding-cleanup/manifest.yml @@ -0,0 +1,19 @@ +packversion: 1 +name: fake-brokerpak +version: 0.1.0 +metadata: + author: noone@nowhere.com +platforms: +- os: linux + arch: amd64 +- os: darwin + arch: amd64 +terraform_binaries: +- name: terraform + version: 1.5.7 + source: https://github.com/hashicorp/terraform/archive/v1.5.7.zip +- name: terraform-provider-random + version: 3.1.0 +service_definitions: +- fake-good-service.yml +- fake-bad-service.yml diff --git a/integrationtest/fixtures/provision-cleanup/fake-string-provision.tf b/integrationtest/fixtures/provision-cleanup/fake-string-provision.tf new file mode 100644 index 000000000..c53142595 --- /dev/null +++ b/integrationtest/fixtures/provision-cleanup/fake-string-provision.tf @@ -0,0 +1,4 @@ +// Fails as required "length" parameter is missing +resource "random_string" "random" {} + +output provision_output { value = random_string.random.result } \ No newline at end of file diff --git a/integrationtest/fixtures/provision-cleanup/fake-string-service.yml b/integrationtest/fixtures/provision-cleanup/fake-string-service.yml new file mode 100644 index 000000000..eb55fb8b8 --- /dev/null +++ b/integrationtest/fixtures/provision-cleanup/fake-string-service.yml @@ -0,0 +1,20 @@ +version: 1 +name: fake-string-service +id: cfeda8d0-cbf3-11ee-be53-73f17d1c612b +description: description +display_name: Fake +image_url: https://example.com/icon.jpg +documentation_url: https://example.com +support_url: https://example.com/support.html +plans: +- name: standard + id: d8fbab66-cbf3-11ee-ab90-d7299e1fcf96 + description: Standard plan + display_name: Standard +provision: + template_refs: + main: fake-string-provision.tf + outputs: + - field_name: provision_output + type: string + details: provision output \ No newline at end of file diff --git a/integrationtest/fixtures/provision-cleanup/manifest.yml b/integrationtest/fixtures/provision-cleanup/manifest.yml new file mode 100644 index 000000000..ddace4617 --- /dev/null +++ b/integrationtest/fixtures/provision-cleanup/manifest.yml @@ -0,0 +1,18 @@ +packversion: 1 +name: fake-brokerpak +version: 0.1.0 +metadata: + author: noone@nowhere.com +platforms: +- os: linux + arch: amd64 +- os: darwin + arch: amd64 +terraform_binaries: +- name: terraform + version: 1.5.7 + source: https://github.com/hashicorp/terraform/archive/v1.5.7.zip +- name: terraform-provider-random + version: 3.1.0 +service_definitions: +- fake-string-service.yml diff --git a/integrationtest/provision_cleanup_test.go b/integrationtest/provision_cleanup_test.go new file mode 100644 index 000000000..1102f25de --- /dev/null +++ b/integrationtest/provision_cleanup_test.go @@ -0,0 +1,65 @@ +package integrationtest_test + +import ( + "encoding/json" + "fmt" + + "github.com/cloudfoundry/cloud-service-broker/dbservice/models" + "github.com/cloudfoundry/cloud-service-broker/integrationtest/packer" + "github.com/cloudfoundry/cloud-service-broker/internal/testdrive" + "github.com/cloudfoundry/cloud-service-broker/pkg/providers/tf/workspace" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Provision Cleanup", func() { + const ( + serviceOfferingGUID = "cfeda8d0-cbf3-11ee-be53-73f17d1c612b" + servicePlanGUID = "d8fbab66-cbf3-11ee-ab90-d7299e1fcf96" + ) + + var ( + brokerpak string + broker *testdrive.Broker + ) + + BeforeEach(func() { + brokerpak = must(packer.BuildBrokerpak(csb, fixtures("provision-cleanup"))) + broker = must(testdrive.StartBroker(csb, brokerpak, database, testdrive.WithOutputs(GinkgoWriter, GinkgoWriter))) + + DeferCleanup(func() { + Expect(broker.Stop()).To(Succeed()) + cleanup(brokerpak) + }) + }) + + // This test captures an incorrect behavior that we want to fix + It("fails to clean up after a provision failed with empty state", func() { + By("failing to provision") + instance, err := broker.Provision(serviceOfferingGUID, servicePlanGUID) + Expect(err).To(MatchError("provision failed with state: failed")) + + By("failing to clean up") + Expect(broker.Deprovision(instance)).To(MatchError(ContainSubstring("failed to delete: workspace state not generated"))) + }) + + // This test captures an incorrect behavior that we want to fix + It("fails to clean up after a provision failed with corrupted state", func() { + By("failing to provision") + instance, err := broker.Provision(serviceOfferingGUID, servicePlanGUID) + Expect(err).To(MatchError("provision failed with state: failed")) + + By("corrupting the state as if terraform had been killed") + invalidWorkspace := must(json.Marshal(workspace.TerraformWorkspace{State: []byte(`{"foo`)})) // Base64-encoded truncated JSON + Expect( + dbConn.Model(&models.TerraformDeployment{}). + Where("id = ?", fmt.Sprintf("tf:%s:", instance.GUID)). + Update("workspace", invalidWorkspace). + Error, + ).To(Succeed()) + + By("failing to clean up") + Expect(broker.Deprovision(instance)).To(MatchError(ContainSubstring("failed to delete: invalid workspace state unexpected end of JSON input"))) + }) +}) diff --git a/internal/testdrive/broker_create_binding.go b/internal/testdrive/broker_create_binding.go index c85f95ac4..3d4a71375 100644 --- a/internal/testdrive/broker_create_binding.go +++ b/internal/testdrive/broker_create_binding.go @@ -35,9 +35,9 @@ func (b *Broker) CreateBinding(s ServiceInstance, opts ...CreateBindingOption) ( bindResponse = b.Client.Bind(s.GUID, cfg.guid, s.ServiceOfferingGUID, s.ServicePlanGUID, uuid.New(), cfg.params) switch { case bindResponse.Error != nil: - return ServiceBinding{}, bindResponse.Error + return ServiceBinding{GUID: cfg.guid}, bindResponse.Error case bindResponse.StatusCode != http.StatusCreated: - return ServiceBinding{}, &UnexpectedStatusError{StatusCode: bindResponse.StatusCode, ResponseBody: bindResponse.ResponseBody} + return ServiceBinding{GUID: cfg.guid}, &UnexpectedStatusError{StatusCode: bindResponse.StatusCode, ResponseBody: bindResponse.ResponseBody} default: return ServiceBinding{ GUID: cfg.guid, diff --git a/internal/testdrive/broker_provision.go b/internal/testdrive/broker_provision.go index 8bc5ed010..027e088f1 100644 --- a/internal/testdrive/broker_provision.go +++ b/internal/testdrive/broker_provision.go @@ -49,21 +49,19 @@ func (b *Broker) Provision(serviceOfferingGUID, servicePlanGUID string, opts ... case err != nil: return err case state != domain.Succeeded: - return fmt.Errorf("provison failed with state: %s", state) + return fmt.Errorf("provision failed with state: %s", state) default: return nil } }, ) - if err != nil { - return ServiceInstance{}, err - } + // If it fails, we still return the GUIDs for cleanup return ServiceInstance{ GUID: cfg.guid, ServicePlanGUID: servicePlanGUID, ServiceOfferingGUID: serviceOfferingGUID, - }, nil + }, err } func WithProvisionParams(params any) ProvisionOption {