diff --git a/client/api_client.go b/client/api_client.go index a1a0cf46..29990f95 100644 --- a/client/api_client.go +++ b/client/api_client.go @@ -12,7 +12,8 @@ type ApiClient struct { } type ApiClientInterface interface { - ConfigurationVariables(scope Scope, scopeId string) ([]ConfigurationVariable, error) + ConfigurationVariablesByScope(scope Scope, scopeId string) ([]ConfigurationVariable, error) + ConfigurationVariablesById(id string) (ConfigurationVariable, error) ConfigurationVariableCreate(params ConfigurationVariableCreateParams) (ConfigurationVariable, error) ConfigurationVariableUpdate(params ConfigurationVariableUpdateParams) (ConfigurationVariable, error) ConfigurationVariableDelete(id string) error diff --git a/client/api_client_mock.go b/client/api_client_mock.go index 6f6de967..a01e8402 100644 --- a/client/api_client_mock.go +++ b/client/api_client_mock.go @@ -182,9 +182,9 @@ func (mr *MockApiClientInterfaceMockRecorder) ConfigurationVariableUpdate(arg0 i } // ConfigurationVariables mocks base method. -func (m *MockApiClientInterface) ConfigurationVariables(arg0 Scope, arg1 string) ([]ConfigurationVariable, error) { +func (m *MockApiClientInterface) ConfigurationVariablesByScope(arg0 Scope, arg1 string) ([]ConfigurationVariable, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ConfigurationVariables", arg0, arg1) + ret := m.ctrl.Call(m, "ConfigurationVariablesByScope", arg0, arg1) ret0, _ := ret[0].([]ConfigurationVariable) ret1, _ := ret[1].(error) return ret0, ret1 @@ -193,7 +193,22 @@ func (m *MockApiClientInterface) ConfigurationVariables(arg0 Scope, arg1 string) // ConfigurationVariables indicates an expected call of ConfigurationVariables. func (mr *MockApiClientInterfaceMockRecorder) ConfigurationVariables(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigurationVariables", reflect.TypeOf((*MockApiClientInterface)(nil).ConfigurationVariables), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigurationVariablesByScope", reflect.TypeOf((*MockApiClientInterface)(nil).ConfigurationVariablesByScope), arg0, arg1) +} + +// ConfigurationVariablesById mocks base method. +func (m *MockApiClientInterface) ConfigurationVariablesById(arg0 string) (ConfigurationVariable, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConfigurationVariablesById", arg0) + ret0, _ := ret[0].(ConfigurationVariable) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ConfigurationVariablesById indicates an expected call of ConfigurationVariablesById. +func (mr *MockApiClientInterfaceMockRecorder) ConfigurationVariablesById(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigurationVariablesById", reflect.TypeOf((*MockApiClientInterface)(nil).ConfigurationVariablesById), arg0) } // Environment mocks base method. diff --git a/client/configuration_variable.go b/client/configuration_variable.go index 5b4b443f..f2894a99 100644 --- a/client/configuration_variable.go +++ b/client/configuration_variable.go @@ -4,7 +4,18 @@ import ( "errors" ) -func (self *ApiClient) ConfigurationVariables(scope Scope, scopeId string) ([]ConfigurationVariable, error) { +func (self *ApiClient) ConfigurationVariablesById(id string) (ConfigurationVariable, error) { + var result ConfigurationVariable + + err := self.http.Get("/configuration/"+id, nil, &result) + + if err != nil { + return ConfigurationVariable{}, err + } + return result, nil +} + +func (self *ApiClient) ConfigurationVariablesByScope(scope Scope, scopeId string) ([]ConfigurationVariable, error) { organizationId, err := self.organizationId() if err != nil { return nil, err diff --git a/client/configuration_variable_test.go b/client/configuration_variable_test.go index fbb405bf..1aab92d5 100644 --- a/client/configuration_variable_test.go +++ b/client/configuration_variable_test.go @@ -35,6 +35,24 @@ var _ = Describe("Configuration Variable", func() { IsRequired: &isRequired, } + Describe("ConfigurationVariablesById", func() { + id := "configurationId" + var found ConfigurationVariable + BeforeEach(func() { + httpCall = mockHttpClient.EXPECT(). + Get("/configuration/"+id, nil, gomock.Any()). + Do(func(path string, request interface{}, response *ConfigurationVariable) { + *response = mockConfigurationVariable + }) + + found, _ = apiClient.ConfigurationVariablesById(id) + }) + + It("Should return variable", func() { + Expect(found).To(Equal(mockConfigurationVariable)) + }) + }) + Describe("ConfigurationVariableCreate", func() { var createdConfigurationVariable ConfigurationVariable @@ -198,7 +216,7 @@ var _ = Describe("Configuration Variable", func() { }) }) - Describe("ConfigurationVariables", func() { + Describe("ConfigurationVariablesByScope", func() { var returnedVariables []ConfigurationVariable mockVariables := []ConfigurationVariable{mockConfigurationVariable} expectedParams := map[string]string{"organizationId": organizationId} @@ -211,7 +229,7 @@ var _ = Describe("Configuration Variable", func() { Do(func(path string, request interface{}, response *[]ConfigurationVariable) { *response = mockVariables }) - returnedVariables, _ = apiClient.ConfigurationVariables(ScopeGlobal, "") + returnedVariables, _ = apiClient.ConfigurationVariablesByScope(ScopeGlobal, "") }) It("Should send GET request with expected params", func() { @@ -239,7 +257,7 @@ var _ = Describe("Configuration Variable", func() { Do(func(path string, request interface{}, response *[]ConfigurationVariable) { *response = mockVariables }) - returnedVariables, _ = apiClient.ConfigurationVariables(Scope(scope), scopeId) + returnedVariables, _ = apiClient.ConfigurationVariablesByScope(Scope(scope), scopeId) httpCall.Times(1) }, Entry("Template Scope", string(ScopeTemplate), "blueprintId"), diff --git a/env0/data_configuration_variable.go b/env0/data_configuration_variable.go index d0a744b9..27dca0f7 100644 --- a/env0/data_configuration_variable.go +++ b/env0/data_configuration_variable.go @@ -185,13 +185,22 @@ func getScopeAndId(d *schema.ResourceData) (client.Scope, string) { func getConfigurationVariable(params ConfigurationVariableParams, meta interface{}) (client.ConfigurationVariable, diag.Diagnostics) { apiClient := meta.(client.ApiClientInterface) + id, idOk := params.Id, params.Id != "" + + if idOk { + variable, err := apiClient.ConfigurationVariablesById(id) + if err != nil { + return client.ConfigurationVariable{}, diag.Errorf("Could not query variable: %v", err) + } + return variable, nil + } + + variables, err := apiClient.ConfigurationVariablesByScope(params.Scope, params.ScopeId) - variables, err := apiClient.ConfigurationVariables(params.Scope, params.ScopeId) if err != nil { return client.ConfigurationVariable{}, diag.Errorf("Could not query variables: %v", err) } - id, idOk := params.Id, params.Id != "" name, nameOk := params.Name, params.Name != "" typeString, ok := params.configurationType, params.configurationType != "" type_ := -1 diff --git a/env0/data_configuration_variable_test.go b/env0/data_configuration_variable_test.go index 682bbd88..4cba2d13 100644 --- a/env0/data_configuration_variable_test.go +++ b/env0/data_configuration_variable_test.go @@ -62,6 +62,8 @@ func TestUnitConfigurationVariableData(t *testing.T) { }, }, func(mock *client.MockApiClientInterface) { + mock.EXPECT().ConfigurationVariablesById(configurationVariable.Id).AnyTimes(). + Return(configurationVariable, nil) mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").AnyTimes(). Return([]client.ConfigurationVariable{configurationVariable}, nil) }) @@ -107,6 +109,8 @@ func TestUnitConfigurationVariableData(t *testing.T) { }, }, func(mock *client.MockApiClientInterface) { + mock.EXPECT().ConfigurationVariablesById(configurationVariable.Id).AnyTimes(). + Return(configurationVariable, nil) mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").AnyTimes(). Return([]client.ConfigurationVariable{configurationVariable}, nil) }) @@ -117,7 +121,7 @@ func TestUnitConfigurationVariableData(t *testing.T) { resource.TestCase{ Steps: []resource.TestStep{ { - Config: dataSourceConfigCreate(resourceType, resourceName, map[string]interface{}{"id": configurationVariable.Id, "template_id": "template_id"}), + Config: dataSourceConfigCreate(resourceType, resourceName, map[string]interface{}{"template_id": "template_id", "name": configurationVariable.Name}), Check: checkResources, }, }, diff --git a/env0/resource_configuration_variable.go b/env0/resource_configuration_variable.go index 6b5a7919..63fd5f5e 100644 --- a/env0/resource_configuration_variable.go +++ b/env0/resource_configuration_variable.go @@ -198,36 +198,34 @@ func resourceConfigurationVariableRead(ctx context.Context, d *schema.ResourceDa apiClient := meta.(client.ApiClientInterface) id := d.Id() - scope, scopeId := whichScope(d) - variables, err := apiClient.ConfigurationVariables(scope, scopeId) + variable, err := apiClient.ConfigurationVariablesById(id) + if err != nil { return diag.Errorf("could not get configurationVariable: %v", err) } - for _, variable := range variables { - if variable.Id == id { - d.Set("name", variable.Name) - d.Set("description", variable.Description) - d.Set("value", variable.Value) - d.Set("is_sensitive", variable.IsSensitive) - if variable.Type != nil && *variable.Type == client.ConfigurationVariableTypeTerraform { - d.Set("type", "terraform") - } else { - d.Set("type", "environment") - } - if variable.Schema != nil { - if len(variable.Schema.Enum) > 0 { - d.Set("enum", variable.Schema.Enum) - } - if variable.Schema.Format != "" { - d.Set("format", variable.Schema.Format) - } - } + d.Set("name", variable.Name) + d.Set("description", variable.Description) + d.Set("value", variable.Value) + d.Set("is_sensitive", variable.IsSensitive) + d.Set("is_read_only", variable.IsReadonly) + d.Set("is_required", variable.IsRequired) + if variable.Type != nil && *variable.Type == client.ConfigurationVariableTypeTerraform { + d.Set("type", "terraform") + } else { + d.Set("type", "environment") + } + if variable.Schema != nil { + if len(variable.Schema.Enum) > 0 { + d.Set("enum", variable.Schema.Enum) + } - return nil + if variable.Schema.Format != "" { + d.Set("format", variable.Schema.Format) } } - return diag.Errorf("variable %s not found (under this scope): %v", id, err) + + return nil } func resourceConfigurationVariableUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { diff --git a/env0/resource_configuration_variable_test.go b/env0/resource_configuration_variable_test.go index f11a8f76..a7ec777a 100644 --- a/env0/resource_configuration_variable_test.go +++ b/env0/resource_configuration_variable_test.go @@ -1,11 +1,13 @@ package env0 import ( + "bytes" "errors" "fmt" "regexp" "strconv" "testing" + "text/template" "github.com/env0/terraform-provider-env0/client" "github.com/golang/mock/gomock" @@ -48,7 +50,6 @@ func TestUnitConfigurationVariableResource(t *testing.T) { IsReadonly: *configVar.IsReadonly, } t.Run("Create", func(t *testing.T) { - createTestCase := resource.TestCase{ Steps: []resource.TestStep{ { @@ -67,11 +68,112 @@ func TestUnitConfigurationVariableResource(t *testing.T) { runUnitTest(t, createTestCase, func(mock *client.MockApiClientInterface) { mock.EXPECT().ConfigurationVariableCreate(configurationVariableCreateParams).Times(1).Return(configVar, nil) - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Times(1).Return([]client.ConfigurationVariable{configVar}, nil) + mock.EXPECT().ConfigurationVariablesById(configVar.Id).Times(1).Return(configVar, nil) mock.EXPECT().ConfigurationVariableDelete(configVar.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 + orgResourceName := "org" + projResourceName := "project" + orgAccessor := resourceAccessor(resourceType, orgResourceName) + projAccessor := resourceAccessor(resourceType, projResourceName) + + orgVar := client.ConfigurationVariable{ + Id: "orgVariableId", + Name: "variable", + Value: "orgVariable", + IsReadonly: &isReadonly, + } + + orgConfigVariableCreateParams := client.ConfigurationVariableCreateParams{ + Name: orgVar.Name, + Value: orgVar.Value, + IsSensitive: false, + Scope: client.ScopeGlobal, + ScopeId: "", + Type: client.ConfigurationVariableTypeEnvironment, + EnumValues: nil, + Format: client.Text, + IsReadonly: *orgVar.IsReadonly, + } + + projVar := client.ConfigurationVariable{ + Id: "projectVariableId", + Name: orgVar.Name, + Value: "projVariable", + Scope: client.ScopeProject, + ScopeId: "projectId", + } + + projectConfigVariableCreateParams := client.ConfigurationVariableCreateParams{ + Name: projVar.Name, + Value: projVar.Value, + IsSensitive: false, + Scope: client.ScopeProject, + ScopeId: projVar.ScopeId, + Type: client.ConfigurationVariableTypeEnvironment, + EnumValues: nil, + Format: client.Text, + } + + data := map[string]interface{}{"projectId": projVar.ScopeId, "orgResourceName": orgResourceName, "projResourceName": projResourceName, "resourceType": resourceType, "variableName": orgVar.Name, "orgValue": orgVar.Value, "projValue": projVar.Value} + + tmpl, err := template.New("").Parse(` +resource "{{.resourceType}}" "{{.orgResourceName}}" { + name = "{{.variableName}}" + value = "{{.orgValue}}" + is_read_only = true +} + +resource "{{.resourceType}}" "{{.projResourceName}}" { + name = {{.resourceType}}.{{.orgResourceName}}.name + value = "{{.projValue}}" + project_id = "{{.projectId}}" +} +`) + if err != nil { + panic(err) + } + var tpl bytes.Buffer + + err = tmpl.Execute(&tpl, data) + if err != nil { + panic(err) + } + stepConfig := tpl.String() + + testStep := resource.TestStep{ + Config: stepConfig, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(orgAccessor, "id", orgVar.Id), + resource.TestCheckResourceAttr(orgAccessor, "name", orgVar.Name), + resource.TestCheckResourceAttr(orgAccessor, "value", orgVar.Value), + resource.TestCheckResourceAttr(orgAccessor, "is_read_only", strconv.FormatBool(*orgVar.IsReadonly)), + resource.TestCheckResourceAttr(projAccessor, "id", projVar.Id), + resource.TestCheckResourceAttr(projAccessor, "name", projVar.Name), + resource.TestCheckResourceAttr(projAccessor, "value", projVar.Value), + ), + } + createTestCase := resource.TestCase{ + Steps: []resource.TestStep{ + testStep, + testStep, + }, + } + + runUnitTest(t, createTestCase, func(mock *client.MockApiClientInterface) { + mock.EXPECT().ConfigurationVariableCreate(projectConfigVariableCreateParams).Times(1).Return(projVar, nil) + mock.EXPECT().ConfigurationVariableCreate(orgConfigVariableCreateParams).Times(1).Return(orgVar, nil) + mock.EXPECT().ConfigurationVariablesById(orgVar.Id).AnyTimes().Return(orgVar, nil) + mock.EXPECT().ConfigurationVariablesById(projVar.Id).AnyTimes().Return(projVar, nil) + mock.EXPECT().ConfigurationVariableDelete(orgVar.Id).Times(1).Return(nil) + mock.EXPECT().ConfigurationVariableDelete(projVar.Id).Times(1).Return(nil) + }) + }) + t.Run("Create Enum", func(t *testing.T) { schema := client.ConfigurationVariableSchema{ Type: "string", @@ -121,7 +223,7 @@ func TestUnitConfigurationVariableResource(t *testing.T) { Description: configVar.Description, Format: client.Text, }).Times(1).Return(configVar, nil) - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Times(1).Return([]client.ConfigurationVariable{configVar}, nil) + mock.EXPECT().ConfigurationVariablesById(configVar.Id).Times(1).Return(configVar, nil) mock.EXPECT().ConfigurationVariableDelete(configVar.Id).Times(1).Return(nil) }) }) @@ -194,7 +296,7 @@ resource "%s" "test" { Description: configVar.Description, Format: format, }).Times(1).Return(configVar, nil) - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Times(1).Return([]client.ConfigurationVariable{configVar}, nil) + mock.EXPECT().ConfigurationVariablesById(configVar.Id).Times(1).Return(configVar, nil) mock.EXPECT().ConfigurationVariableDelete(configVar.Id).Times(1).Return(nil) }) }) @@ -244,24 +346,7 @@ resource "%s" "test" { Steps: []resource.TestStep{ { Config: stepConfig, - ExpectError: regexp.MustCompile(`(Error: could not get configurationVariable: error)`), - }, - }, - } - - runUnitTest(t, createTestCase, func(mock *client.MockApiClientInterface) { - mock.EXPECT().ConfigurationVariableCreate(configurationVariableCreateParams).Times(1).Return(configVar, nil) - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Times(1).Return([]client.ConfigurationVariable{}, errors.New("error")) - mock.EXPECT().ConfigurationVariableDelete(configVar.Id).Times(1).Return(nil) - }) - }) - - t.Run("Read not found", func(t *testing.T) { - createTestCase := resource.TestCase{ - Steps: []resource.TestStep{ - { - Config: stepConfig, - ExpectError: regexp.MustCompile(`(Error: variable .+ not found)`), + ExpectError: regexp.MustCompile("could not get configurationVariable: error"), }, }, } @@ -269,7 +354,7 @@ resource "%s" "test" { runUnitTest(t, createTestCase, func(mock *client.MockApiClientInterface) { mock.EXPECT().ConfigurationVariableCreate( configurationVariableCreateParams).Times(1).Return(configVar, nil) - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Times(1).Return([]client.ConfigurationVariable{}, nil) + mock.EXPECT().ConfigurationVariablesById(configVar.Id).Times(1).Return(client.ConfigurationVariable{}, errors.New("error")) mock.EXPECT().ConfigurationVariableDelete(configVar.Id).Times(1).Return(nil) }) }) @@ -354,8 +439,8 @@ resource "%s" "test" { mock.EXPECT().ConfigurationVariableCreate(createParams).Times(1).Return(configVar, nil) gomock.InOrder( - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Return([]client.ConfigurationVariable{configVar}, nil).Times(2), - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Return([]client.ConfigurationVariable{newConfigVar}, nil), + mock.EXPECT().ConfigurationVariablesById(configVar.Id).Times(2).Return(configVar, nil), + mock.EXPECT().ConfigurationVariablesById(configVar.Id).Return(newConfigVar, nil), ) mock.EXPECT().ConfigurationVariableUpdate(client.ConfigurationVariableUpdateParams{CommonParams: updateParams, Id: newConfigVar.Id}).Times(1).Return(configVar, nil) mock.EXPECT().ConfigurationVariableDelete(configVar.Id).Times(1).Return(nil) @@ -404,7 +489,7 @@ resource "%s" "test" { runUnitTest(t, updateTestCase, func(mock *client.MockApiClientInterface) { mock.EXPECT().ConfigurationVariableCreate(configurationVariableCreateParams).Times(1).Return(configVar, nil) - mock.EXPECT().ConfigurationVariables(client.ScopeGlobal, "").Return([]client.ConfigurationVariable{configVar}, nil).Times(2) + mock.EXPECT().ConfigurationVariablesById(configVar.Id).Times(2).Return(configVar, nil) mock.EXPECT().ConfigurationVariableDelete(configVar.Id).Times(1).Return(nil) }) }) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index c7e7c9ea..6cb8a8f5 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -254,7 +254,7 @@ func resourceEnvironmentRead(ctx context.Context, d *schema.ResourceData, meta i if err != nil { return diag.Errorf("could not get environment: %v", err) } - environmentConfigurationVariables, err := apiClient.ConfigurationVariables(client.ScopeEnvironment, environment.Id) + environmentConfigurationVariables, err := apiClient.ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id) if err != nil { return diag.Errorf("could not get environment configuration variables: %v", err) } @@ -491,7 +491,7 @@ func getTTl(date string) client.TTL { } func getUpdateConfigurationVariables(configurationChanges client.ConfigurationChanges, environmentId string, apiClient client.ApiClientInterface) client.ConfigurationChanges { - existVariables, err := apiClient.ConfigurationVariables(client.ScopeEnvironment, environmentId) + existVariables, err := apiClient.ConfigurationVariablesByScope(client.ScopeEnvironment, environmentId) if err != nil { diag.Errorf("could not get environment configuration variables: %v", err) } @@ -640,7 +640,7 @@ func resourceEnvironmentImport(ctx context.Context, d *schema.ResourceData, meta } apiClient := meta.(client.ApiClientInterface) d.SetId(environment.Id) - environmentConfigurationVariables, err := apiClient.ConfigurationVariables(client.ScopeEnvironment, environment.Id) + environmentConfigurationVariables, err := apiClient.ConfigurationVariablesByScope(client.ScopeEnvironment, environment.Id) if err != nil { return nil, errors.New(fmt.Sprintf("could not get environment configuration variables: %v", err)) }