diff --git a/dbservice/vcap.go b/dbservice/vcap.go index 40c42c3d5..f7684e830 100644 --- a/dbservice/vcap.go +++ b/dbservice/vcap.go @@ -100,15 +100,16 @@ func ParseVcapServices(vcapServicesData string) (VcapService, error) { return VcapService{}, fmt.Errorf("error unmarshalling VCAP_SERVICES: %s", err) } - for _, vcapArray := range vcapMap { - vcapService, err := findMySQLTag(vcapArray, "mysql") - if err != nil { - return VcapService{}, fmt.Errorf("error finding MySQL tag: %s", err) - } - return vcapService, nil + flatVCAPServices := []VcapService{} + for _, v := range vcapMap { + flatVCAPServices = append(flatVCAPServices, v...) } - return VcapService{}, fmt.Errorf("error parsing VCAP_SERVICES") + service, err := findMySQLTag(flatVCAPServices, "mysql") + if err != nil { + return VcapService{}, fmt.Errorf("error finding MySQL tag: %s", err) + } + return service, nil } // whether a given string array arr contains string key diff --git a/dbservice/vcap_test.go b/dbservice/vcap_test.go index a647e8227..b932c9d58 100644 --- a/dbservice/vcap_test.go +++ b/dbservice/vcap_test.go @@ -224,6 +224,79 @@ var _ = Describe("VCAP", func() { `) Expect(err).To(MatchError("error finding MySQL tag: the variable VCAP_SERVICES must have one VCAP service with a tag of 'mysql'. There are currently 0 VCAP services with the tag 'mysql'")) }) + + It("succeeds when VCAP_SERVICES has multiple lists but only one with a MySQL tag", func() { + _, err := ParseVcapServices(`{ + "google-cloudsql-mysql": [ + { + "binding_name": "testbinding", + "instance_name": "testinstance", + "name": "kf-binding-tt2-mystorage", + "label": "google-storage", + "tags": [ + "gcp", + "cloudsql" + ], + "plan": "nearline", + "credentials": { + "CaCert": "-truncated-", + "ClientCert": "-truncated-", + "ClientKey": "-truncated-", + "Email": "pcf-binding-testbind@test-gsb.iam.gserviceaccount.com", + "Name": "pcf-binding-testbind", + "Password": "PASSWORD", + "PrivateKeyData": "PRIVATEKEY", + "ProjectId": "test-gsb", + "Sha1Fingerprint": "aa3bade266136f733642ebdb4992b89eb05f83c4", + "UniqueId": "108868434450972082663", + "UriPrefix": "", + "Username": "newuseraccount", + "database_name": "service_broker", + "host": "127.0.0.1", + "instance_name": "pcf-sb-1-1561406852899716453", + "last_master_operation_id": "", + "region": "", + "uri": "mysql://newuseraccount:PASSWORD@127.0.0.1/service_broker?ssl_mode=required" + } + } + ], + "user-provided": [ + { + "binding_name": "testbinding", + "instance_name": "testinstance", + "name": "kf-binding-tt2-mystorage", + "label": "google-storage", + "tags": [ + "gcp", + "cloudsql", + "mysql" + ], + "plan": "nearline", + "credentials": { + "CaCert": "-truncated-", + "ClientCert": "-truncated-", + "ClientKey": "-truncated-", + "Email": "pcf-binding-testbind@test-gsb.iam.gserviceaccount.com", + "Name": "pcf-binding-testbind", + "Password": "PASSWORD", + "PrivateKeyData": "PRIVATEKEY", + "ProjectId": "test-gsb", + "Sha1Fingerprint": "aa3bade266136f733642ebdb4992b89eb05f83c4", + "UniqueId": "108868434450972082663", + "UriPrefix": "", + "Username": "newuseraccount", + "database_name": "service_broker", + "host": "127.0.0.1", + "instance_name": "pcf-sb-1-1561406852899716453", + "last_master_operation_id": "", + "region": "", + "uri": "mysql://newuseraccount:PASSWORD@127.0.0.1/service_broker?ssl_mode=required" + } + } + ] +}`) + Expect(err).NotTo(HaveOccurred()) + }) }) Describe("setting database credentials", func() { diff --git a/pkg/providers/tf/bind_test.go b/pkg/providers/tf/bind_test.go index 536ceccd2..2c212032c 100644 --- a/pkg/providers/tf/bind_test.go +++ b/pkg/providers/tf/bind_test.go @@ -8,8 +8,6 @@ import ( "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" - "github.com/cloudfoundry/cloud-service-broker/v2/internal/storage" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf" "github.com/cloudfoundry/cloud-service-broker/v2/pkg/providers/tf/executor" @@ -89,9 +87,9 @@ var _ = Describe("Bind", func() { Expect(actualWorkspace.Modules[0].Definition).To(Equal(fakeServiceDefinition.BindSettings.Template)) Expect(actualWorkspace.Modules[0].Definitions).To(Equal(fakeServiceDefinition.BindSettings.Templates)) Expect(actualWorkspace.Instances[0].Configuration).To(Equal(map[string]any{"username": "some-user"})) - Expect(actualWorkspace.Transformer.ParameterMappings).To(Equal([]workspace.ParameterMapping{})) - Expect(actualWorkspace.Transformer.ParametersToRemove).To(Equal([]string{})) - Expect(actualWorkspace.Transformer.ParametersToAdd).To(Equal([]workspace.ParameterMapping{})) + Expect(actualWorkspace.Transformer.ParameterMappings).To(BeZero()) + Expect(actualWorkspace.Transformer.ParametersToRemove).To(BeZero()) + Expect(actualWorkspace.Transformer.ParametersToAdd).To(BeZero()) By("checking that provision is marked as started") Expect(fakeDeploymentManager.MarkOperationStartedCallCount()).To(Equal(1)) diff --git a/pkg/providers/tf/deployment_manager_test.go b/pkg/providers/tf/deployment_manager_test.go index 8eb7fdb35..77df92be6 100644 --- a/pkg/providers/tf/deployment_manager_test.go +++ b/pkg/providers/tf/deployment_manager_test.go @@ -424,9 +424,8 @@ var _ = Describe("DeploymentManager", func() { By("checking that the modules and instances are updated, but the state remains the same") expectedWorkspace := &workspace.TerraformWorkspace{ Modules: []workspace.ModuleDefinition{{ - Name: "brokertemplate", - Definition: template, - Definitions: map[string]string{}, + Name: "brokertemplate", + Definition: template, }}, Instances: []workspace.ModuleInstance{{ ModuleName: "brokertemplate", @@ -435,11 +434,6 @@ var _ = Describe("DeploymentManager", func() { "resourceGroup": nil, }, }}, - Transformer: workspace.TfTransformer{ - ParameterMappings: []workspace.ParameterMapping{}, - ParametersToRemove: []string{}, - ParametersToAdd: []workspace.ParameterMapping{}, - }, State: []byte(terraformState), } Expect(actualTerraformDeployment.Workspace).To(Equal(expectedWorkspace)) @@ -506,9 +500,8 @@ var _ = Describe("DeploymentManager", func() { By("checking that the modules and instances are updated, but the state remains the same") expectedWorkspace := &workspace.TerraformWorkspace{ Modules: []workspace.ModuleDefinition{{ - Name: "brokertemplate", - Definition: template, - Definitions: map[string]string{}, + Name: "brokertemplate", + Definition: template, }}, Instances: []workspace.ModuleInstance{{ ModuleName: "brokertemplate", @@ -517,11 +510,6 @@ var _ = Describe("DeploymentManager", func() { "resourceGroup": nil, }, }}, - Transformer: workspace.TfTransformer{ - ParameterMappings: []workspace.ParameterMapping{}, - ParametersToRemove: []string{}, - ParametersToAdd: []workspace.ParameterMapping{}, - }, State: []byte(terraformState), } Expect(actualTerraformDeployment.Workspace).To(Equal(expectedWorkspace)) diff --git a/pkg/providers/tf/provision_test.go b/pkg/providers/tf/provision_test.go index 75abe6ed8..1884bc146 100644 --- a/pkg/providers/tf/provision_test.go +++ b/pkg/providers/tf/provision_test.go @@ -79,9 +79,9 @@ var _ = Describe("Provision", func() { Expect(actualWorkspace.Modules[0].Definition).To(Equal(fakeServiceDefinition.ProvisionSettings.Template)) Expect(actualWorkspace.Modules[0].Definitions).To(Equal(fakeServiceDefinition.ProvisionSettings.Templates)) Expect(actualWorkspace.Instances[0].Configuration).To(Equal(map[string]any{"username": "some-user"})) - Expect(actualWorkspace.Transformer.ParameterMappings).To(Equal([]workspace.ParameterMapping{})) - Expect(actualWorkspace.Transformer.ParametersToRemove).To(Equal([]string{})) - Expect(actualWorkspace.Transformer.ParametersToAdd).To(Equal([]workspace.ParameterMapping{})) + Expect(actualWorkspace.Transformer.ParameterMappings).To(BeZero()) + Expect(actualWorkspace.Transformer.ParametersToRemove).To(BeZero()) + Expect(actualWorkspace.Transformer.ParametersToAdd).To(BeZero()) By("checking that provision is marked as started") Expect(fakeDeploymentManager.MarkOperationStartedCallCount()).To(Equal(1)) diff --git a/pkg/providers/tf/workspace/workspace.go b/pkg/providers/tf/workspace/workspace.go index b03d486e3..a93f7be4b 100644 --- a/pkg/providers/tf/workspace/workspace.go +++ b/pkg/providers/tf/workspace/workspace.go @@ -56,8 +56,11 @@ func NewWorkspace(templateVars map[string]any, parametersToRemove []string, parametersToAdd []ParameterMapping) (*TerraformWorkspace, error) { - terraformTemplatesCopy := make(map[string]string) - maps.Copy(terraformTemplatesCopy, terraformTemplates) + var terraformTemplatesCopy map[string]string + if len(terraformTemplates) > 0 { + terraformTemplatesCopy = make(map[string]string) + maps.Copy(terraformTemplatesCopy, terraformTemplates) + } tfModule := ModuleDefinition{ Name: "brokertemplate", @@ -75,12 +78,23 @@ func NewWorkspace(templateVars map[string]any, limitedConfig[name] = templateVars[name] } - parameterMappingsCopy := make([]ParameterMapping, len(importParameterMappings)) - copy(parameterMappingsCopy, importParameterMappings) - parametersToRemoveCopy := make([]string, len(parametersToRemove)) - copy(parametersToRemoveCopy, parametersToRemove) - parametersToAddCopy := make([]ParameterMapping, len(parametersToAdd)) - copy(parametersToAddCopy, parametersToAdd) + var parameterMappingsCopy []ParameterMapping + if len(importParameterMappings) > 0 { + parameterMappingsCopy = make([]ParameterMapping, len(importParameterMappings)) + copy(parameterMappingsCopy, importParameterMappings) + } + + var parametersToRemoveCopy []string + if len(parametersToRemove) > 0 { + parametersToRemoveCopy = make([]string, len(parametersToRemove)) + copy(parametersToRemoveCopy, parametersToRemove) + } + + var parametersToAddCopy []ParameterMapping + if len(parametersToAdd) > 0 { + parametersToAddCopy = make([]ParameterMapping, len(parametersToAdd)) + copy(parametersToAddCopy, parametersToAdd) + } workspace := TerraformWorkspace{ Modules: []ModuleDefinition{tfModule}, diff --git a/pkg/providers/tf/workspace/workspace_test.go b/pkg/providers/tf/workspace/workspace_test.go index c8f3fea3d..6a4b47aee 100644 --- a/pkg/providers/tf/workspace/workspace_test.go +++ b/pkg/providers/tf/workspace/workspace_test.go @@ -370,4 +370,76 @@ func TestNewWorkspace(t *testing.T) { t.Error("Expected NewWorkspace to create deep copy of ParametersToAdd") } }) + t.Run("returns empty map for empty templateVars", func(t *testing.T) { + ws, err := NewWorkspace(nil, "", nil, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + got := ws.Instances[0].Configuration + expect := make(map[string]any) + if !reflect.DeepEqual(expect, got) { + t.Errorf("expected %v, got %v", expect, got) + } + }) + + t.Run("returns zero-value for empty terraformTemplate", func(t *testing.T) { + ws, err := NewWorkspace(nil, "", nil, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + got := ws.Modules[0].Definition + var expect string + if !reflect.DeepEqual(expect, got) { + t.Errorf("expected %v, got %v", expect, got) + } + }) + + t.Run("returns zero-value for empty terraformTemplates", func(t *testing.T) { + ws, err := NewWorkspace(nil, "", nil, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + got := ws.Modules[0].Definitions + var expect map[string]string + if !reflect.DeepEqual(expect, got) { + t.Errorf("expected %v, got %v", expect, got) + } + }) + + t.Run("returns zero-value for empty importParameterMappings", func(t *testing.T) { + ws, err := NewWorkspace(nil, "", nil, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + got := ws.Transformer.ParameterMappings + var expect []ParameterMapping + if !reflect.DeepEqual(expect, got) { + t.Errorf("expected %v, got %v", expect, got) + } + }) + + t.Run("returns zero-value for empty importParametersToRemove", func(t *testing.T) { + ws, err := NewWorkspace(nil, "", nil, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + got := ws.Transformer.ParametersToRemove + var expect []string + if !reflect.DeepEqual(expect, got) { + t.Errorf("expected %v, got %v", expect, got) + } + }) + + t.Run("returns zero-value for empty importParametersToAdd", func(t *testing.T) { + ws, err := NewWorkspace(nil, "", nil, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + got := ws.Transformer.ParametersToAdd + var expect []ParameterMapping + if !reflect.DeepEqual(expect, got) { + t.Errorf("expected %v, got %v", expect, got) + } + }) + }