Skip to content

Commit

Permalink
Fix: lexicographical sorting of configuration items (#518)
Browse files Browse the repository at this point in the history 
* Fix: lexicographical sorting of configuration items

* fix crash
  • Loading branch information
@TomerHeber
TomerHeber authored Oct 25, 2022
1 parent e898a81 commit 94e53c1
Show file tree
Hide file tree
Showing 2 changed files with 173 additions and 35 deletions.
115 changes: 85 additions & 30 deletions env0/resource_environment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ func resourceEnvironment() *schema.Resource {
Type: schema.TypeString,
Description: "the type the variable must be of",
Optional: true,
Default: "string",
},
"schema_enum": {
Type: schema.TypeList,
Expand Down Expand Up @@ -277,43 +278,97 @@ func setEnvironmentSchema(d *schema.ResourceData, environment client.Environment
return nil
}

func createVariable(configurationVariable *client.ConfigurationVariable) interface{} {
variable := make(map[string]interface{})

variable["name"] = configurationVariable.Name
variable["value"] = configurationVariable.Value

if configurationVariable.Type == nil || *configurationVariable.Type == 0 {
variable["type"] = "environment"
} else {
variable["type"] = "terraform"
}

if configurationVariable.Description != "" {
variable["description"] = configurationVariable.Description
}

if configurationVariable.Regex != "" {
variable["regex"] = configurationVariable.Regex
}

if configurationVariable.IsSensitive != nil {
variable["is_sensitive"] = configurationVariable.IsSensitive
}

if configurationVariable.IsReadOnly != nil {
variable["is_read_only"] = configurationVariable.IsReadOnly
}

if configurationVariable.IsRequired != nil {
variable["is_required"] = configurationVariable.IsRequired
}

if configurationVariable.Schema != nil {
variable["schema_type"] = configurationVariable.Schema.Type
variable["schema_enum"] = configurationVariable.Schema.Enum
variable["schema_format"] = configurationVariable.Schema.Format
}

return variable
}

func setEnvironmentConfigurationSchema(d *schema.ResourceData, configurationVariables []client.ConfigurationVariable) {
var variables []interface{}
ivariables, ok := d.GetOk("configuration")
if !ok {
return
}

for _, configurationVariable := range configurationVariables {
variable := make(map[string]interface{})
variable["name"] = configurationVariable.Name
variable["value"] = configurationVariable.Value
if configurationVariable.Type == nil || *configurationVariable.Type == 0 {
variable["type"] = "environment"
} else {
variable["type"] = "terraform"
}
if configurationVariable.Description != "" {
variable["description"] = configurationVariable.Description
}
if configurationVariable.Regex != "" {
variable["regex"] = configurationVariable.Regex
}
if configurationVariable.IsSensitive != nil {
variable["is_sensitive"] = configurationVariable.IsSensitive
}
if configurationVariable.IsReadOnly != nil {
variable["is_read_only"] = configurationVariable.IsReadOnly
if ivariables == nil {
ivariables = make([]interface{}, 0)
}

variables := ivariables.([]interface{})

newVariables := make([]interface{}, 0)

// The goal is to maintain existing state order as much as possible. (The backend response order may vary from state).
for _, ivariable := range variables {
variable := ivariable.(map[string]interface{})
variableName := variable["name"].(string)

for _, configurationVariable := range configurationVariables {
if configurationVariable.Name == variableName {
newVariables = append(newVariables, createVariable(&configurationVariable))
break
}
}
if configurationVariable.IsRequired != nil {
variable["is_required"] = configurationVariable.IsRequired
}

// Check for drifts: add new configuration variables received from the backend.
for _, configurationVariable := range configurationVariables {
found := false

for _, ivariable := range variables {
variable := ivariable.(map[string]interface{})
variableName := variable["name"].(string)
if configurationVariable.Name == variableName {
found = true
break
}
}
if configurationVariable.Schema != nil {
variable["schema_type"] = configurationVariable.Schema.Type
variable["schema_enum"] = configurationVariable.Schema.Enum
variable["schema_format"] = configurationVariable.Schema.Format

if !found {
log.Printf("[WARN] Drift Detected for configuration: %s", configurationVariable.Name)
newVariables = append(newVariables, createVariable(&configurationVariable))
}
variables = append(variables, variable)
}

if variables != nil {
d.Set("configuration", variables)
if len(newVariables) > 0 {
d.Set("configuration", newVariables)
} else {
d.Set("configuration", nil)
}
}

Expand Down
93 changes: 88 additions & 5 deletions env0/resource_environment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func TestUnitEnvironmentResource(t *testing.T) {
resource.TestCheckResourceAttr(accessor, "revision", environment.LatestDeploymentLog.BlueprintRevision),
resource.TestCheckResourceAttr(accessor, "configuration.0.name", configurationVariables.Name),
resource.TestCheckResourceAttr(accessor, "configuration.0.value", configurationVariables.Value),
resource.TestCheckResourceAttr(accessor, "configuration.0.schema_type", ""),
resource.TestCheckResourceAttr(accessor, "configuration.0.schema_type", "string"),
resource.TestCheckNoResourceAttr(accessor, "configuration.0.schema_enum"),
),
},
Expand All @@ -415,7 +415,7 @@ func TestUnitEnvironmentResource(t *testing.T) {
mock.EXPECT().Template(environment.LatestDeploymentLog.BlueprintId).Times(1).Return(template, nil)
mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1).Return(environment, nil)

mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id).Times(1).Return(client.ConfigurationChanges{}, nil) // read after create -> on update -> read after update
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id).Times(1).Return(client.ConfigurationChanges{configurationVariables}, nil) // read after create -> on update -> read after update
mock.EXPECT().Environment(environment.Id).Times(1).Return(environment, nil)

mock.EXPECT().EnvironmentDestroy(environment.Id).Times(1)
Expand Down Expand Up @@ -486,7 +486,88 @@ func TestUnitEnvironmentResource(t *testing.T) {
mock.EXPECT().Template(environment.LatestDeploymentLog.BlueprintId).Times(1).Return(template, nil)
mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1).Return(environment, nil)

mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id).Times(1).Return(client.ConfigurationChanges{}, nil) // read after create -> on update -> read after update
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id).Times(1).Return(client.ConfigurationChanges{configurationVariables}, nil) // read after create -> on update -> read after update
mock.EXPECT().Environment(environment.Id).Times(1).Return(environment, nil)

mock.EXPECT().EnvironmentDestroy(environment.Id).Times(1)
})
})

// Tests use-cases where the response returned by the backend varies from the order of the state.
t.Run("Create unordered configuration variables", func(t *testing.T) {
environment := client.Environment{
Id: "id0",
Name: "my-environment",
ProjectId: "project-id",
LatestDeploymentLog: client.DeploymentLog{
BlueprintId: "template-id",
BlueprintRevision: "revision",
},
}

configurationVariable1 := client.ConfigurationVariable{
Value: "my env var value",
Name: "my env var",
Schema: &client.ConfigurationVariableSchema{
Type: "string",
},
}

configurationVariable2 := client.ConfigurationVariable{
Value: "a",
Name: "b",
Schema: &client.ConfigurationVariableSchema{
Type: "string",
},
}

environmentResource := fmt.Sprintf(`
resource "%s" "%s" {
name = "%s"
project_id = "%s"
template_id = "%s"
revision = "%s"
force_destroy = true
configuration {
name = "%s"
value = "%s"
}
configuration {
name = "%s"
value = "%s"
}
}`,
resourceType, resourceName, environment.Name,
environment.ProjectId, environment.LatestDeploymentLog.BlueprintId,
environment.LatestDeploymentLog.BlueprintRevision, configurationVariable1.Name,
configurationVariable1.Value, configurationVariable2.Name,
configurationVariable2.Value,
)

testCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: environmentResource,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(accessor, "id", environment.Id),
resource.TestCheckResourceAttr(accessor, "name", environment.Name),
resource.TestCheckResourceAttr(accessor, "project_id", environment.ProjectId),
resource.TestCheckResourceAttr(accessor, "template_id", environment.LatestDeploymentLog.BlueprintId),
resource.TestCheckResourceAttr(accessor, "revision", environment.LatestDeploymentLog.BlueprintRevision),
resource.TestCheckResourceAttr(accessor, "configuration.0.name", configurationVariable1.Name),
resource.TestCheckResourceAttr(accessor, "configuration.0.value", configurationVariable1.Value),
resource.TestCheckResourceAttr(accessor, "configuration.1.name", configurationVariable2.Name),
resource.TestCheckResourceAttr(accessor, "configuration.1.value", configurationVariable2.Value),
),
},
},
}

runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {
mock.EXPECT().Template(environment.LatestDeploymentLog.BlueprintId).Times(1).Return(template, nil)
mock.EXPECT().EnvironmentCreate(gomock.Any()).Times(1).Return(environment, nil)

mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id).Times(1).Return(client.ConfigurationChanges{configurationVariable2, configurationVariable1}, nil) // read after create -> on update -> read after update
mock.EXPECT().Environment(environment.Id).Times(1).Return(environment, nil)

mock.EXPECT().EnvironmentDestroy(environment.Id).Times(1)
Expand Down Expand Up @@ -599,10 +680,12 @@ func TestUnitEnvironmentResource(t *testing.T) {
mock.EXPECT().EnvironmentDeploy(environment.Id, gomock.Any()).Times(1).Return(client.EnvironmentDeployResponse{
Id: "deployment-id",
}, nil)
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, updatedEnvironment.Id).Times(4).Return(client.ConfigurationChanges{}, nil) // read after create -> on update -> read after update
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, updatedEnvironment.Id).Times(2).Return(client.ConfigurationChanges{}, nil) // read after create -> on update -> read after update
gomock.InOrder(
mock.EXPECT().Environment(gomock.Any()).Times(2).Return(environment, nil), // 1 after create, 1 before update
mock.EXPECT().Environment(gomock.Any()).Times(2).Return(environment, nil), // 1 after create, 1 before update
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, updatedEnvironment.Id).Times(1).Return(client.ConfigurationChanges{configurationVariables}, nil),
mock.EXPECT().Environment(gomock.Any()).Times(1).Return(updatedEnvironment, nil), // 1 after update
mock.EXPECT().ConfigurationVariablesByScope(client.ScopeEnvironment, updatedEnvironment.Id).Times(1).Return(client.ConfigurationChanges{configurationVariables}, nil),
)

mock.EXPECT().EnvironmentDestroy(environment.Id).Times(1)
Expand Down

0 comments on commit 94e53c1

Please sign in to comment.