Skip to content

Commit

Permalink
chore: func NewWorkspace returns proper zero-values for empty functio…
Browse files Browse the repository at this point in the history
…n parameters (#1115)

* refactor: func NewWorkspace returns proper zero-values for empty function parameters

* test: use workspace.transformer zero-values

---------

Co-authored-by: ifindlay-cci <84311346+ifindlay-cci@users.noreply.github.com>
  • Loading branch information
kanngiesser and ifindlay-cci authored Oct 9, 2024
1 parent f0a172c commit 60a315d
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 32 deletions.
8 changes: 3 additions & 5 deletions pkg/providers/tf/bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
Expand Down
20 changes: 4 additions & 16 deletions pkg/providers/tf/deployment_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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))
Expand Down Expand Up @@ -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",
Expand All @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions pkg/providers/tf/provision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
30 changes: 22 additions & 8 deletions pkg/providers/tf/workspace/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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},
Expand Down
72 changes: 72 additions & 0 deletions pkg/providers/tf/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})

}

0 comments on commit 60a315d

Please sign in to comment.