From a000e9db475a20ab925317b74789140191e49d40 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Tue, 10 May 2022 14:20:24 -0500 Subject: [PATCH] Fix: env0_aws_credentials - apply after import will delete+create --- client/cloud_credentials.go | 8 +- client/errors.go | 7 ++ env0/credentials.go | 47 ++++++++ env0/data_notification.go | 2 +- env0/errors.go | 15 ++- env0/resource_agent_project_assignment.go | 4 +- env0/resource_api_key.go | 4 +- env0/resource_aws_credentials.go | 54 ++++++--- env0/resource_aws_credentials_test.go | 110 +++++++++++++++++- env0/resource_git_token.go | 4 +- env0/resource_module.go | 4 +- env0/resource_notification.go | 4 +- ...esource_notification_project_assignment.go | 2 +- env0/resource_organization_policy.go | 2 +- env0/utils.go | 12 ++ env0/utils_test.go | 18 +++ .../resources/env0_aws_credentials/import.sh | 2 + 17 files changed, 258 insertions(+), 41 deletions(-) create mode 100644 client/errors.go create mode 100644 env0/credentials.go create mode 100644 examples/resources/env0_aws_credentials/import.sh diff --git a/client/cloud_credentials.go b/client/cloud_credentials.go index 542bf1b7..26809f9b 100644 --- a/client/cloud_credentials.go +++ b/client/cloud_credentials.go @@ -1,9 +1,5 @@ package client -import ( - "fmt" -) - type AwsCredentialsType string type GcpCredentialsType string type AzureCredentialsType string @@ -37,7 +33,7 @@ type AwsCredentialsCreatePayload struct { } type AwsCredentialsValuePayload struct { - RoleArn string `json:"roleArn"` + RoleArn string `json:"roleArn" resource:"arn"` ExternalId string `json:"externalId"` AccessKeyId string `json:"accessKeyId"` SecretAccessKey string `json:"secretAccessKey"` @@ -89,7 +85,7 @@ func (client *ApiClient) CloudCredentials(id string) (Credentials, error) { } } - return Credentials{}, fmt.Errorf("CloudCredentials: [%s] not found ", id) + return Credentials{}, &NotFoundError{} } func (client *ApiClient) CloudCredentialsList() ([]Credentials, error) { diff --git a/client/errors.go b/client/errors.go new file mode 100644 index 00000000..58de083c --- /dev/null +++ b/client/errors.go @@ -0,0 +1,7 @@ +package client + +type NotFoundError struct{} + +func (e *NotFoundError) Error() string { + return "not found" +} diff --git a/env0/credentials.go b/env0/credentials.go new file mode 100644 index 00000000..19d5bad1 --- /dev/null +++ b/env0/credentials.go @@ -0,0 +1,47 @@ +package env0 + +import ( + "fmt" + "log" + "strings" + + "github.com/env0/terraform-provider-env0/client" + "github.com/google/uuid" +) + +func getCredentialsByName(name string, prefix string, meta interface{}) (client.Credentials, error) { + apiClient := meta.(client.ApiClientInterface) + + credentialsList, err := apiClient.CloudCredentialsList() + if err != nil { + return client.Credentials{}, err + } + + var foundCredentials []client.Credentials + for _, credentials := range credentialsList { + if credentials.Name == name && strings.HasPrefix(credentials.Type, prefix) { + foundCredentials = append(foundCredentials, credentials) + } + } + + if len(foundCredentials) == 0 { + return client.Credentials{}, fmt.Errorf("credentials with name %v not found", name) + } + + if len(foundCredentials) > 1 { + return client.Credentials{}, fmt.Errorf("found multiple credentials with name: %s. Use id instead or make sure credential names are unique %v", name, foundCredentials) + } + + return foundCredentials[0], nil +} + +func getCredentials(id string, prefix string, meta interface{}) (client.Credentials, error) { + _, err := uuid.Parse(id) + if err == nil { + log.Println("[INFO] Resolving credentials by id: ", id) + return meta.(client.ApiClientInterface).CloudCredentials(id) + } else { + log.Println("[INFO] Resolving credentials by name: ", id) + return getCredentialsByName(id, prefix, meta) + } +} diff --git a/env0/data_notification.go b/env0/data_notification.go index ff410a61..e16bbc5e 100644 --- a/env0/data_notification.go +++ b/env0/data_notification.go @@ -61,7 +61,7 @@ func dataNotificationRead(ctx context.Context, d *schema.ResourceData, meta inte } if err := writeResourceData(notification, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil diff --git a/env0/errors.go b/env0/errors.go index 43677b83..0b8aa1f2 100644 --- a/env0/errors.go +++ b/env0/errors.go @@ -3,13 +3,26 @@ package env0 import ( "log" + "github.com/env0/terraform-provider-env0/client" "github.com/env0/terraform-provider-env0/client/http" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) -func ResourceGetFailure(resourceName string, d *schema.ResourceData, err error) diag.Diagnostics { +func driftDetected(d *schema.ResourceData, err error) bool { if frerr, ok := err.(*http.FailedResponseError); ok && frerr.NotFound() { + return true + } + + if _, ok := err.(*client.NotFoundError); ok { + return true + } + + return false +} + +func ResourceGetFailure(resourceName string, d *schema.ResourceData, err error) diag.Diagnostics { + if driftDetected(d, err) { log.Printf("[WARN] Drift Detected: Terraform will remove %s from state", d.Id()) d.SetId("") return nil diff --git a/env0/resource_agent_project_assignment.go b/env0/resource_agent_project_assignment.go index 46309bfc..ffa88a46 100644 --- a/env0/resource_agent_project_assignment.go +++ b/env0/resource_agent_project_assignment.go @@ -118,7 +118,7 @@ func resourceAgentProjectAssignmentRead(ctx context.Context, d *schema.ResourceD } if err := writeResourceData(assignment, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil @@ -189,7 +189,7 @@ func resourceAgentProjectAssignmentImport(ctx context.Context, d *schema.Resourc } if err := writeResourceData(assignment, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return nil, fmt.Errorf("schema resource data serialization failed: %v", err) } return []*schema.ResourceData{d}, nil diff --git a/env0/resource_api_key.go b/env0/resource_api_key.go index 23d5ee69..069ff70a 100644 --- a/env0/resource_api_key.go +++ b/env0/resource_api_key.go @@ -60,7 +60,7 @@ func resourceApiKeyRead(ctx context.Context, d *schema.ResourceData, meta interf } if err := writeResourceData(apiKey, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil @@ -138,7 +138,7 @@ func resourceApiKeyImport(ctx context.Context, d *schema.ResourceData, meta inte } if err := writeResourceData(apiKey, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return nil, fmt.Errorf("schema resource data serialization failed: %v", err) } return []*schema.ResourceData{d}, nil diff --git a/env0/resource_aws_credentials.go b/env0/resource_aws_credentials.go index e3900146..1dc07801 100644 --- a/env0/resource_aws_credentials.go +++ b/env0/resource_aws_credentials.go @@ -2,6 +2,7 @@ package env0 import ( "context" + "fmt" "github.com/env0/terraform-provider-env0/client" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -14,6 +15,8 @@ func resourceAwsCredentials() *schema.Resource { ReadContext: resourceAwsCredentialsRead, DeleteContext: resourceAwsCredentialsDelete, + Importer: &schema.ResourceImporter{StateContext: resourceAwsCredentialsImport}, + Schema: map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -27,7 +30,6 @@ func resourceAwsCredentials() *schema.Resource { Optional: true, ForceNew: true, ConflictsWith: []string{"access_key_id"}, - ExactlyOneOf: []string{"access_key_id"}, }, "external_id": { Type: schema.TypeString, @@ -46,7 +48,6 @@ func resourceAwsCredentials() *schema.Resource { ForceNew: true, ConflictsWith: []string{"arn", "external_id"}, RequiredWith: []string{"secret_access_key"}, - ExactlyOneOf: []string{"arn"}, }, "secret_access_key": { Type: schema.TypeString, @@ -62,29 +63,32 @@ func resourceAwsCredentials() *schema.Resource { } func resourceAwsCredentialsCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + _, accessKeyExist := d.GetOk("access_key_id") + _, arnExist := d.GetOk("arn") + if !accessKeyExist && !arnExist { + // Due to "import" must be inforced here and not in the schema level. + // This fields are only available during creation (will not be returned in read or import). + return diag.Errorf("one of `access_key_id,arn` must be specified") + } + apiClient := meta.(client.ApiClientInterface) value := client.AwsCredentialsValuePayload{} - requestType := client.AwsAssumedRoleCredentialsType - if arn, ok := d.GetOk("arn"); ok { - value.RoleArn = arn.(string) - requestType = client.AwsAssumedRoleCredentialsType - } - if externalId, ok := d.GetOk("external_id"); ok { - value.ExternalId = externalId.(string) + if err := readResourceData(&value, d); err != nil { + return diag.Errorf("schema resource data deserialization failed: %v", err) } - if accessKeyId, ok := d.GetOk("access_key_id"); ok { - value.AccessKeyId = accessKeyId.(string) + + requestType := client.AwsAssumedRoleCredentialsType + if _, ok := d.GetOk("access_key_id"); ok { requestType = client.AwsAccessKeysCredentialsType } - if secretAccessKey, ok := d.GetOk("secret_access_key"); ok { - value.SecretAccessKey = secretAccessKey.(string) - } + request := client.AwsCredentialsCreatePayload{ Name: d.Get("name").(string), Value: value, Type: requestType, } + credentials, err := apiClient.AwsCredentialsCreate(request) if err != nil { return diag.Errorf("could not create credentials key: %v", err) @@ -98,11 +102,14 @@ func resourceAwsCredentialsCreate(ctx context.Context, d *schema.ResourceData, m func resourceAwsCredentialsRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { apiClient := meta.(client.ApiClientInterface) - id := d.Id() - _, err := apiClient.CloudCredentials(id) + credentials, err := apiClient.CloudCredentials(d.Id()) if err != nil { return ResourceGetFailure("aws credentials", d, err) } + + d.Set("name", credentials.Name) + d.SetId(credentials.Id) + return nil } @@ -116,3 +123,18 @@ func resourceAwsCredentialsDelete(ctx context.Context, d *schema.ResourceData, m } return nil } + +func resourceAwsCredentialsImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + credentials, err := getCredentials(d.Id(), "AWS_", meta) + if err != nil { + if _, ok := err.(*client.NotFoundError); ok { + return nil, fmt.Errorf("aws credentials resource with id %v not found", d.Id()) + } + return nil, err + } + + d.Set("name", credentials.Name) + d.SetId(credentials.Id) + + return []*schema.ResourceData{d}, nil +} diff --git a/env0/resource_aws_credentials_test.go b/env0/resource_aws_credentials_test.go index fd471756..1ed786f1 100644 --- a/env0/resource_aws_credentials_test.go +++ b/env0/resource_aws_credentials_test.go @@ -1,6 +1,7 @@ package env0 import ( + "fmt" "regexp" "testing" @@ -12,9 +13,9 @@ import ( ) func TestUnitAwsCredentialsResource(t *testing.T) { - resourceType := "env0_aws_credentials" resourceName := "test" + resourceNameImport := resourceType + "." + resourceName accessor := resourceAccessor(resourceType, resourceName) awsArnCredentialResource := map[string]interface{}{ @@ -48,14 +49,21 @@ func TestUnitAwsCredentialsResource(t *testing.T) { } returnValues := client.Credentials{ - Id: "id", + Id: "f595c4b6-0a24-4c22-89f7-7030045de30f", Name: "test", OrganizationId: "id", Type: "AWS_ASSUMED_ROLE_FOR_DEPLOYMENT", } + otherTypeReturnValues := client.Credentials{ + Id: "f595c4b6-0a24-4c22-89f7-7030045de30a", + Name: "test", + OrganizationId: "id", + Type: "GCP_....", + } + updateReturnValues := client.Credentials{ - Id: "id2", + Id: "f595c4b6-0a24-4c22-89f7-7030045de30c", Name: "update", OrganizationId: "id", Type: "AWS_ASSUMED_ROLE_FOR_DEPLOYMENT", @@ -69,7 +77,7 @@ func TestUnitAwsCredentialsResource(t *testing.T) { resource.TestCheckResourceAttr(accessor, "name", awsArnCredentialResource["name"].(string)), resource.TestCheckResourceAttr(accessor, "arn", awsArnCredentialResource["arn"].(string)), resource.TestCheckResourceAttr(accessor, "external_id", awsArnCredentialResource["external_id"].(string)), - resource.TestCheckResourceAttr(accessor, "id", "id"), + resource.TestCheckResourceAttr(accessor, "id", returnValues.Id), ), }, }, @@ -108,7 +116,7 @@ func TestUnitAwsCredentialsResource(t *testing.T) { Steps: []resource.TestStep{ { Config: resourceConfigCreate(resourceType, resourceName, mutuallyExclusiveErrorResource), - ExpectError: regexp.MustCompile("only one of `access_key_id,arn` can be specified"), + ExpectError: regexp.MustCompile(`"external_id": conflicts with access_key_id`), }, }, } @@ -184,4 +192,96 @@ func TestUnitAwsCredentialsResource(t *testing.T) { }) }) + t.Run("import by name", func(t *testing.T) { + testCase := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, awsArnCredentialResource), + }, + { + ResourceName: resourceNameImport, + ImportState: true, + ImportStateId: awsArnCredentialResource["name"].(string), + ImportStateVerify: false, + }, + }, + } + + runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) { + mock.EXPECT().AwsCredentialsCreate(awsArnCredCreatePayload).Times(1).Return(returnValues, nil) + mock.EXPECT().CloudCredentials(returnValues.Id).Times(2).Return(returnValues, nil) + mock.EXPECT().CloudCredentialsList().Times(1).Return([]client.Credentials{otherTypeReturnValues, returnValues}, nil) + mock.EXPECT().CloudCredentialsDelete(returnValues.Id).Times(1).Return(nil) + }) + }) + + t.Run("import by id", func(t *testing.T) { + testCase := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, awsArnCredentialResource), + }, + { + ResourceName: resourceNameImport, + ImportState: true, + ImportStateId: returnValues.Id, + ImportStateVerify: false, + }, + }, + } + + runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) { + mock.EXPECT().AwsCredentialsCreate(awsArnCredCreatePayload).Times(1).Return(returnValues, nil) + mock.EXPECT().CloudCredentials(returnValues.Id).Times(3).Return(returnValues, nil) + mock.EXPECT().CloudCredentialsDelete(returnValues.Id).Times(1).Return(nil) + }) + }) + + t.Run("import by id not found", func(t *testing.T) { + testCase := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, awsArnCredentialResource), + }, + { + ResourceName: resourceNameImport, + ImportState: true, + ImportStateId: otherTypeReturnValues.Id, + ImportStateVerify: true, + ExpectError: regexp.MustCompile(fmt.Sprintf("aws credentials resource with id %v not found", otherTypeReturnValues.Id)), + }, + }, + } + + runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) { + mock.EXPECT().AwsCredentialsCreate(awsArnCredCreatePayload).Times(1).Return(returnValues, nil) + mock.EXPECT().CloudCredentials(returnValues.Id).Times(1).Return(returnValues, nil) + mock.EXPECT().CloudCredentials(otherTypeReturnValues.Id).Times(1).Return(client.Credentials{}, &client.NotFoundError{}) + mock.EXPECT().CloudCredentialsDelete(returnValues.Id).Times(1).Return(nil) + }) + }) + + t.Run("import by name not found", func(t *testing.T) { + testCase := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, awsArnCredentialResource), + }, + { + ResourceName: resourceNameImport, + ImportState: true, + ImportStateId: awsArnCredentialResource["name"].(string), + ImportStateVerify: true, + ExpectError: regexp.MustCompile(fmt.Sprintf("credentials with name %v not found", awsArnCredentialResource["name"].(string))), + }, + }, + } + + runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) { + mock.EXPECT().AwsCredentialsCreate(awsArnCredCreatePayload).Times(1).Return(returnValues, nil) + mock.EXPECT().CloudCredentials(returnValues.Id).Times(1).Return(returnValues, nil) + mock.EXPECT().CloudCredentialsList().Times(1).Return([]client.Credentials{otherTypeReturnValues}, nil) + mock.EXPECT().CloudCredentialsDelete(returnValues.Id).Times(1).Return(nil) + }) + }) } diff --git a/env0/resource_git_token.go b/env0/resource_git_token.go index d93f2f61..8fa8b11c 100644 --- a/env0/resource_git_token.go +++ b/env0/resource_git_token.go @@ -64,7 +64,7 @@ func resourceGitTokenRead(ctx context.Context, d *schema.ResourceData, meta inte } if err := writeResourceData(gitToken, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil @@ -124,7 +124,7 @@ func resourceGitTokenImport(ctx context.Context, d *schema.ResourceData, meta in } if err := writeResourceData(gitToken, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return nil, fmt.Errorf("schema resource data serialization failed: %v", err) } return []*schema.ResourceData{d}, nil diff --git a/env0/resource_module.go b/env0/resource_module.go index 5d44808b..9712d338 100644 --- a/env0/resource_module.go +++ b/env0/resource_module.go @@ -117,7 +117,7 @@ func resourceModuleRead(ctx context.Context, d *schema.ResourceData, meta interf } if err := writeResourceData(module, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil @@ -192,7 +192,7 @@ func resourceModuleImport(ctx context.Context, d *schema.ResourceData, meta inte } if err := writeResourceData(module, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return nil, fmt.Errorf("schema resource data serialization failed: %v", err) } return []*schema.ResourceData{d}, nil diff --git a/env0/resource_notification.go b/env0/resource_notification.go index 78d44ad9..3d6aa50d 100644 --- a/env0/resource_notification.go +++ b/env0/resource_notification.go @@ -121,7 +121,7 @@ func resourceNotificationRead(ctx context.Context, d *schema.ResourceData, meta } if err := writeResourceData(notification, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil @@ -175,7 +175,7 @@ func resourceNotificationImport(ctx context.Context, d *schema.ResourceData, met } if err := writeResourceData(notification, d); err != nil { - return nil, err + return nil, fmt.Errorf("schema resource data serialization failed: %v", err) } return []*schema.ResourceData{d}, nil diff --git a/env0/resource_notification_project_assignment.go b/env0/resource_notification_project_assignment.go index 0d28aa33..1648f8b4 100644 --- a/env0/resource_notification_project_assignment.go +++ b/env0/resource_notification_project_assignment.go @@ -114,7 +114,7 @@ func resourceNotificationProjectAssignmentRead(ctx context.Context, d *schema.Re } if err := writeResourceData(assignment, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil diff --git a/env0/resource_organization_policy.go b/env0/resource_organization_policy.go index c1f8e138..86582f21 100644 --- a/env0/resource_organization_policy.go +++ b/env0/resource_organization_policy.go @@ -59,7 +59,7 @@ func resourceOrganizationPolicyRead(ctx context.Context, d *schema.ResourceData, } if err := writeResourceData(&organization, d); err != nil { - diag.Errorf("schema resource data serialization failed: %v", err) + return diag.Errorf("schema resource data serialization failed: %v", err) } return nil diff --git a/env0/utils.go b/env0/utils.go index fb39fc4e..d7a7d569 100644 --- a/env0/utils.go +++ b/env0/utils.go @@ -42,7 +42,13 @@ func readResourceData(i interface{}, d *schema.ResourceData) error { for i := 0; i < val.NumField(); i++ { fieldName := val.Type().Field(i).Name // Assumes golang is CamalCase and Terraform is snake_case. + // This behavior can be overrided be used in the 'resource' tag. fieldNameSC := toSnakeCase(fieldName) + if resFieldName, ok := val.Type().Field(i).Tag.Lookup("resource"); ok { + // 'resource' tag found. Override to tag value. + fieldNameSC = resFieldName + } + field := val.Field(i) fieldType := field.Type() @@ -101,7 +107,13 @@ func writeResourceData(i interface{}, d *schema.ResourceData) error { for i := 0; i < val.NumField(); i++ { fieldName := val.Type().Field(i).Name // Assumes golang is CamalCase and Terraform is snake_case. + // This behavior can be overrided be used in the 'resource' tag. fieldNameSC := toSnakeCase(fieldName) + if resFieldName, ok := val.Type().Field(i).Tag.Lookup("resource"); ok { + // 'resource' tag found. Override to tag value. + fieldNameSC = resFieldName + } + field := val.Field(i) fieldType := field.Type() diff --git a/env0/utils_test.go b/env0/utils_test.go index 3449d01d..e103bcb5 100644 --- a/env0/utils_test.go +++ b/env0/utils_test.go @@ -91,6 +91,24 @@ func TestReadResourceDataNotification(t *testing.T) { assert.Equal(t, expectedPayload, payload) } +func TestReadResourceDataWithTag(t *testing.T) { + d := schema.TestResourceDataRaw(t, resourceAwsCredentials().Schema, map[string]interface{}{ + "name": "name", + "arn": "tagged_arn", + "external_id": "external_id", + }) + + expectedPayload := client.AwsCredentialsValuePayload{ + RoleArn: "tagged_arn", + ExternalId: "external_id", + } + + var payload client.AwsCredentialsValuePayload + + assert.Nil(t, readResourceData(&payload, d)) + assert.Equal(t, expectedPayload, payload) +} + func TestWriteResourceDataNotification(t *testing.T) { d := schema.TestResourceDataRaw(t, resourceNotification().Schema, map[string]interface{}{}) diff --git a/examples/resources/env0_aws_credentials/import.sh b/examples/resources/env0_aws_credentials/import.sh new file mode 100644 index 00000000..f992cc7b --- /dev/null +++ b/examples/resources/env0_aws_credentials/import.sh @@ -0,0 +1,2 @@ +terraform import env0_aws_credentials.by_id d31a6b30-5f69-4d24-937c-22322754934e +terraform import env0_aws_credentials.by_name "credentials name" \ No newline at end of file