From 94e53c16f85701fdea1a6a5fbca721204570c1a7 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Tue, 25 Oct 2022 09:57:39 -0500 Subject: [PATCH] Fix: lexicographical sorting of configuration items (#518) * Fix: lexicographical sorting of configuration items * fix crash --- env0/resource_environment.go | 115 ++++++++++++++++++++++-------- env0/resource_environment_test.go | 93 ++++++++++++++++++++++-- 2 files changed, 173 insertions(+), 35 deletions(-) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index 354d2d91..16a2fcd2 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -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, @@ -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) } } diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index 8fdfbce7..2e4b7ba9 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -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"), ), }, @@ -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) @@ -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) @@ -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)