Skip to content

Commit

Permalink
Fix: drift in value of env0_configuration_variable in every plan (#937)
Browse files Browse the repository at this point in the history
* Fix: drift in value of env0_configuration_variable in every plan

* fix invalid memory error

* fix data_source_code_variables

* Fix unit test

* fix test
  • Loading branch information
TomerHeber authored Aug 21, 2024
1 parent cff9b13 commit 4de999d
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 15 deletions.
2 changes: 1 addition & 1 deletion client/configuration_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (c *ConfigurationVariableSchema) ResourceDataSliceStructValueWrite(values m

type ConfigurationVariable struct {
ScopeId string `json:"scopeId,omitempty"`
Value string `json:"value"`
Value string `json:"value" tfschema:"-"`
OrganizationId string `json:"organizationId,omitempty"`
UserId string `json:"userId,omitempty"`
IsSensitive *bool `json:"isSensitive,omitempty"`
Expand Down
4 changes: 4 additions & 0 deletions env0/data_configuration_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ func dataConfigurationVariableRead(ctx context.Context, d *schema.ResourceData,
return diag.Errorf("schema resource data serialization failed: %v", err)
}

if variable.IsSensitive == nil || !*variable.IsSensitive {
d.Set("value", variable.Value)
}

d.Set("enum", variable.Schema.Enum)

if variable.Schema.Format != client.Text {
Expand Down
12 changes: 11 additions & 1 deletion env0/data_source_code_variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,20 @@ func dataSourceCodeVariablesRead(ctx context.Context, d *schema.ResourceData, me
return diag.Errorf("failed to extract variables from repository: %v", err)
}

if err := writeResourceDataSlice(variables, "variables", d); err != nil {
ivalues, err := writeResourceDataGetSliceValues(variables, "variables", d)
if err != nil {
return diag.Errorf("schema slice resource data serialization failed: %v", err)
}

for i, ivalue := range ivalues {
if variables[i].IsSensitive == nil || !*variables[i].IsSensitive {
ivariable := ivalue.(map[string]interface{})
ivariable["value"] = variables[i].Value
}
}

d.Set("variables", ivalues)

d.SetId(templateId)

return nil
Expand Down
12 changes: 9 additions & 3 deletions env0/resource_configuration_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func getConfigurationVariableCreateParams(d *schema.ResourceData) (*client.Confi
func resourceConfigurationVariableCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
params, err := getConfigurationVariableCreateParams(d)
if err != nil {
return diag.Errorf(err.Error())
return diag.FromErr(err)
}

apiClient := meta.(client.ApiClientInterface)
Expand All @@ -195,7 +195,9 @@ func getEnum(d *schema.ResourceData, selectedValue string) ([]string, error) {
if enumValue == nil {
return nil, fmt.Errorf("an empty enum value is not allowed (at index %d)", i)
}

actualEnumValues = append(actualEnumValues, enumValue.(string))

if enumValue == selectedValue {
valueExists = true
}
Expand Down Expand Up @@ -223,13 +225,17 @@ func resourceConfigurationVariableRead(ctx context.Context, d *schema.ResourceDa
return diag.Errorf("schema resource data serialization failed: %v", err)
}

if variable.IsSensitive == nil || !*variable.IsSensitive {
d.Set("value", variable.Value)
}

return nil
}

func resourceConfigurationVariableUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
params, err := getConfigurationVariableCreateParams(d)
if err != nil {
return diag.Errorf(err.Error())
return diag.FromErr(err)
}

apiClient := meta.(client.ApiClientInterface)
Expand Down Expand Up @@ -280,7 +286,7 @@ func resourceConfigurationVariableImport(ctx context.Context, d *schema.Resource
var scopeName string

if variable.Scope == client.ScopeTemplate {
scopeName = strings.ToLower(fmt.Sprintf("%s_id", templateScope))
scopeName = strings.ToLower(templateScope + "_id")
} else {
scopeName = strings.ToLower(fmt.Sprintf("%s_id", variable.Scope))
}
Expand Down
47 changes: 44 additions & 3 deletions env0/resource_configuration_variable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/env0/terraform-provider-env0/client"
"github.com/env0/terraform-provider-env0/client/http"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"go.uber.org/mock/gomock"
)
Expand Down Expand Up @@ -50,6 +51,7 @@ func TestUnitConfigurationVariableResource(t *testing.T) {
IsRequired: *configVar.IsRequired,
IsReadOnly: *configVar.IsReadOnly,
}

t.Run("Create", func(t *testing.T) {
createTestCase := resource.TestCase{
Steps: []resource.TestStep{
Expand All @@ -74,6 +76,47 @@ func TestUnitConfigurationVariableResource(t *testing.T) {
})
})

t.Run("Create sensitive", func(t *testing.T) {
createSenstiveConfig := client.ConfigurationVariableCreateParams{
Name: "name",
Value: "value",
IsSensitive: true,
Scope: client.ScopeGlobal,
}

sensitiveConfig := client.ConfigurationVariable{
Id: uuid.NewString(),
Name: createSenstiveConfig.Name,
Value: "*",
IsSensitive: boolPtr(true),
Scope: client.ScopeGlobal,
}

createTestCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"name": createSenstiveConfig.Name,
"value": createSenstiveConfig.Value,
"is_sensitive": true,
}),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(accessor, "id", sensitiveConfig.Id),
resource.TestCheckResourceAttr(accessor, "name", createSenstiveConfig.Name),
resource.TestCheckResourceAttr(accessor, "value", createSenstiveConfig.Value),
resource.TestCheckResourceAttr(accessor, "is_sensitive", strconv.FormatBool(true)),
),
},
},
}

runUnitTest(t, createTestCase, func(mock *client.MockApiClientInterface) {
mock.EXPECT().ConfigurationVariableCreate(createSenstiveConfig).Times(1).Return(sensitiveConfig, nil)
mock.EXPECT().ConfigurationVariablesById(sensitiveConfig.Id).Times(1).Return(sensitiveConfig, nil)
mock.EXPECT().ConfigurationVariableDelete(sensitiveConfig.Id).Times(1).Return(nil)
})
})

t.Run("Create Two with readonly", func(t *testing.T) {
// https://github.com/env0/terraform-provider-env0/issues/215
// we want to create two variables, one org level with read only and another in lower level and see we can still manage both - double apply and destroy
Expand Down Expand Up @@ -286,7 +329,6 @@ resource "{{.resourceType}}" "{{.projResourceName}}" {

for _, format := range []client.Format{client.HCL, client.JSON} {
t.Run("Create "+string(format)+" Variable", func(t *testing.T) {

expectedVariable := `{
A = "A"
B = "B"
Expand Down Expand Up @@ -627,8 +669,8 @@ resource "%s" "test" {
IsRequired: *configVarImport.IsRequired,
IsReadOnly: *configVarImport.IsReadOnly,
}
t.Run("import by name", func(t *testing.T) {

t.Run("import by name", func(t *testing.T) {
createTestCaseForImport := resource.TestCase{
Steps: []resource.TestStep{
{
Expand All @@ -653,7 +695,6 @@ resource "%s" "test" {
})

t.Run("import by id", func(t *testing.T) {

createTestCaseForImport := resource.TestCase{
Steps: []resource.TestStep{
{
Expand Down
6 changes: 0 additions & 6 deletions env0/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ func TestWriteCustomResourceData(t *testing.T) {
Name: "name0",
Description: "desc0",
ScopeId: "scope0",
Value: "value0",
OrganizationId: "organization0",
UserId: "user0",
IsSensitive: boolPtr(true),
Expand All @@ -196,7 +195,6 @@ func TestWriteCustomResourceData(t *testing.T) {
assert.Equal(t, configurationVariable.Name, d.Get("name"))
assert.Equal(t, configurationVariable.Description, d.Get("description"))
assert.Equal(t, "terraform", d.Get("type"))
assert.Equal(t, configurationVariable.Value, d.Get("value"))
assert.Equal(t, string(configurationVariable.Scope), d.Get("scope"))
assert.Equal(t, *configurationVariable.IsReadOnly, d.Get("is_read_only"))
assert.Equal(t, *configurationVariable.IsRequired, d.Get("is_required"))
Expand Down Expand Up @@ -287,15 +285,13 @@ func TestWriteResourceDataSliceVariablesConfigurationVariable(t *testing.T) {
Id: "id0",
Name: "name0",
Description: "desc0",
Value: "v1",
Schema: &schema1,
}

var2 := client.ConfigurationVariable{
Id: "id1",
Name: "name1",
Description: "desc1",
Value: "v2",
Schema: &schema2,
}

Expand All @@ -305,8 +301,6 @@ func TestWriteResourceDataSliceVariablesConfigurationVariable(t *testing.T) {

assert.Equal(t, var1.Name, d.Get("variables.0.name"))
assert.Equal(t, var2.Name, d.Get("variables.1.name"))
assert.Equal(t, var1.Value, d.Get("variables.0.value"))
assert.Equal(t, var2.Value, d.Get("variables.1.value"))
assert.Equal(t, string(var1.Schema.Format), d.Get("variables.0.format"))
assert.Equal(t, string(var2.Schema.Format), d.Get("variables.1.format"))
}
Expand Down
2 changes: 1 addition & 1 deletion tests/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func main() {
} else {
success, err := runTest(testName, destroyMode != "NO_DESTROY")
if !success {
log.Fatalf("Halting due to test '%s' failure: %s\n", testName, err.Error())
log.Fatalf("Halting due to test '%s' failure: %s\n", testName, err)
}
}

Expand Down

0 comments on commit 4de999d

Please sign in to comment.