From 8375f9c112aadf0cd260229a927ae2922a6d5197 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Mon, 13 Mar 2023 07:52:19 -0500 Subject: [PATCH] Feat: stop sending external_id for env0_aws_credentials resource (#593) --- client/cloud_credentials.go | 1 - client/cloud_credentials_test.go | 3 +- env0/resource_aws_credentials.go | 13 ++------ env0/resource_aws_credentials_test.go | 30 ++----------------- env0/resource_cost_credentials.go | 8 ----- env0/resource_cost_credentials_test.go | 19 ++++-------- env0/utils_test.go | 8 ++--- .../env0_aws_credentials/resource.tf | 7 ++--- .../resource.tf | 7 ++--- tests/integration/006_aws_credentials/main.tf | 5 ++-- .../main.tf | 5 ++-- .../main.tf | 5 ++-- .../integration/024_cloud_credentials/main.tf | 5 ++-- 13 files changed, 29 insertions(+), 87 deletions(-) diff --git a/client/cloud_credentials.go b/client/cloud_credentials.go index c426197e..36aed0f2 100644 --- a/client/cloud_credentials.go +++ b/client/cloud_credentials.go @@ -50,7 +50,6 @@ type AwsCredentialsCreatePayload struct { type AwsCredentialsValuePayload struct { RoleArn string `json:"roleArn" tfschema:"arn"` - ExternalId string `json:"externalId,omitempty"` AccessKeyId string `json:"accessKeyId"` SecretAccessKey string `json:"secretAccessKey"` } diff --git a/client/cloud_credentials_test.go b/client/cloud_credentials_test.go index 2e6cd709..221076f7 100644 --- a/client/cloud_credentials_test.go +++ b/client/cloud_credentials_test.go @@ -80,8 +80,7 @@ var _ = Describe("CloudCredentials", func() { mockOrganizationIdCall(organizationId) payloadValue := AwsCredentialsValuePayload{ - RoleArn: "role", - ExternalId: "external", + RoleArn: "role", } httpCall = mockHttpClient.EXPECT(). diff --git a/env0/resource_aws_credentials.go b/env0/resource_aws_credentials.go index fcff5541..d312d72a 100644 --- a/env0/resource_aws_credentials.go +++ b/env0/resource_aws_credentials.go @@ -30,22 +30,13 @@ func resourceAwsCredentials() *schema.Resource { ForceNew: true, ConflictsWith: []string{"access_key_id"}, }, - "external_id": { - Type: schema.TypeString, - Description: "the aws role external id", - Optional: true, - Sensitive: true, - ForceNew: true, - ConflictsWith: []string{"access_key_id"}, - Deprecated: "field will be removed in the near future", - }, "access_key_id": { Type: schema.TypeString, Description: "the aws access key id", Optional: true, Sensitive: true, ForceNew: true, - ConflictsWith: []string{"arn", "external_id"}, + ConflictsWith: []string{"arn"}, RequiredWith: []string{"secret_access_key"}, }, "secret_access_key": { @@ -54,7 +45,7 @@ func resourceAwsCredentials() *schema.Resource { Optional: true, Sensitive: true, ForceNew: true, - ConflictsWith: []string{"arn", "external_id"}, + ConflictsWith: []string{"arn"}, RequiredWith: []string{"access_key_id"}, }, }, diff --git a/env0/resource_aws_credentials_test.go b/env0/resource_aws_credentials_test.go index d49e92d6..35e36e11 100644 --- a/env0/resource_aws_credentials_test.go +++ b/env0/resource_aws_credentials_test.go @@ -19,9 +19,8 @@ func TestUnitAwsCredentialsResource(t *testing.T) { accessor := resourceAccessor(resourceType, resourceName) awsArnCredentialResource := map[string]interface{}{ - "name": "test", - "arn": "11111", - "external_id": "22222", + "name": "test", + "arn": "11111", } updatedAwsAccessKeyCredentialResource := map[string]interface{}{ @@ -33,8 +32,7 @@ func TestUnitAwsCredentialsResource(t *testing.T) { awsArnCredCreatePayload := client.AwsCredentialsCreatePayload{ Name: awsArnCredentialResource["name"].(string), Value: client.AwsCredentialsValuePayload{ - RoleArn: awsArnCredentialResource["arn"].(string), - ExternalId: awsArnCredentialResource["external_id"].(string), + RoleArn: awsArnCredentialResource["arn"].(string), }, Type: client.AwsAssumedRoleCredentialsType, } @@ -76,7 +74,6 @@ func TestUnitAwsCredentialsResource(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( 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", returnValues.Id), ), }, @@ -90,7 +87,6 @@ func TestUnitAwsCredentialsResource(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( 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", returnValues.Id), ), }, @@ -106,21 +102,6 @@ func TestUnitAwsCredentialsResource(t *testing.T) { }, } - mutuallyExclusiveErrorResource := map[string]interface{}{ - "name": "update", - "arn": "11111", - "external_id": "22222", - "access_key_id": "some-key", - } - testCaseFormMutuallyExclusiveError := resource.TestCase{ - Steps: []resource.TestStep{ - { - Config: resourceConfigCreate(resourceType, resourceName, mutuallyExclusiveErrorResource), - ExpectError: regexp.MustCompile(`"external_id": conflicts with access_key_id`), - }, - }, - } - missingValidInputErrorResource := map[string]interface{}{ "name": "update", } @@ -156,11 +137,6 @@ func TestUnitAwsCredentialsResource(t *testing.T) { }) }) - t.Run("throw error when enter mutually exclusive values", func(t *testing.T) { - runUnitTest(t, testCaseFormMutuallyExclusiveError, func(mock *client.MockApiClientInterface) { - }) - }) - t.Run("throw error when don't enter any valid options", func(t *testing.T) { runUnitTest(t, testCaseFormMissingValidInputError, func(mock *client.MockApiClientInterface) { }) diff --git a/env0/resource_cost_credentials.go b/env0/resource_cost_credentials.go index 31fd6f17..86660797 100644 --- a/env0/resource_cost_credentials.go +++ b/env0/resource_cost_credentials.go @@ -23,14 +23,6 @@ func resourceCostCredentials(providerName string) *schema.Resource { ForceNew: true, Required: true, }, - "external_id": { - Type: schema.TypeString, - Description: "the aws role external id", - Sensitive: true, - ForceNew: true, - Optional: true, - Deprecated: "field will be removed in the near future", - }, } azureSchema := map[string]*schema.Schema{ diff --git a/env0/resource_cost_credentials_test.go b/env0/resource_cost_credentials_test.go index e233a397..c285b457 100644 --- a/env0/resource_cost_credentials_test.go +++ b/env0/resource_cost_credentials_test.go @@ -17,22 +17,19 @@ func TestUnitAwsCostCredentialsResource(t *testing.T) { accessor := resourceAccessor(resourceType, resourceName) awsCredentialResource := map[string]interface{}{ - "name": "test", - "arn": "11111", - "external_id": "22222", + "name": "test", + "arn": "11111", } updatedAwsCredentialResource := map[string]interface{}{ - "name": "update", - "arn": "33333", - "external_id": "44444", + "name": "update", + "arn": "33333", } awsCredCreatePayload := client.AwsCredentialsCreatePayload{ Name: awsCredentialResource["name"].(string), Value: client.AwsCredentialsValuePayload{ - RoleArn: awsCredentialResource["arn"].(string), - ExternalId: awsCredentialResource["external_id"].(string), + RoleArn: awsCredentialResource["arn"].(string), }, Type: client.AwsCostCredentialsType, } @@ -40,8 +37,7 @@ func TestUnitAwsCostCredentialsResource(t *testing.T) { updateAwsCredCreatePayload := client.AwsCredentialsCreatePayload{ Name: updatedAwsCredentialResource["name"].(string), Value: client.AwsCredentialsValuePayload{ - RoleArn: updatedAwsCredentialResource["arn"].(string), - ExternalId: updatedAwsCredentialResource["external_id"].(string), + RoleArn: updatedAwsCredentialResource["arn"].(string), }, Type: client.AwsCostCredentialsType, } @@ -67,7 +63,6 @@ func TestUnitAwsCostCredentialsResource(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(accessor, "name", awsCredentialResource["name"].(string)), resource.TestCheckResourceAttr(accessor, "arn", awsCredentialResource["arn"].(string)), - resource.TestCheckResourceAttr(accessor, "external_id", awsCredentialResource["external_id"].(string)), resource.TestCheckResourceAttr(accessor, "id", "id"), ), }, @@ -81,7 +76,6 @@ func TestUnitAwsCostCredentialsResource(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(accessor, "name", awsCredentialResource["name"].(string)), resource.TestCheckResourceAttr(accessor, "arn", awsCredentialResource["arn"].(string)), - resource.TestCheckResourceAttr(accessor, "external_id", awsCredentialResource["external_id"].(string)), resource.TestCheckResourceAttr(accessor, "id", returnValues.Id), ), }, @@ -90,7 +84,6 @@ func TestUnitAwsCostCredentialsResource(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(accessor, "name", updatedAwsCredentialResource["name"].(string)), resource.TestCheckResourceAttr(accessor, "arn", updatedAwsCredentialResource["arn"].(string)), - resource.TestCheckResourceAttr(accessor, "external_id", updatedAwsCredentialResource["external_id"].(string)), resource.TestCheckResourceAttr(accessor, "id", updateReturnValues.Id), ), }, diff --git a/env0/utils_test.go b/env0/utils_test.go index 4c1d0569..7ba631f3 100644 --- a/env0/utils_test.go +++ b/env0/utils_test.go @@ -96,14 +96,12 @@ func TestReadResourceDataNotification(t *testing.T) { func TestReadResourceDataWithTag(t *testing.T) { d := schema.TestResourceDataRaw(t, resourceAwsCredentials().Schema, map[string]interface{}{ - "name": "name", - "arn": "tagged_arn", - "external_id": "external_id", + "name": "name", + "arn": "tagged_arn", }) expectedPayload := client.AwsCredentialsValuePayload{ - RoleArn: "tagged_arn", - ExternalId: "external_id", + RoleArn: "tagged_arn", } var payload client.AwsCredentialsValuePayload diff --git a/examples/resources/env0_aws_credentials/resource.tf b/examples/resources/env0_aws_credentials/resource.tf index da502c99..49ae8062 100644 --- a/examples/resources/env0_aws_credentials/resource.tf +++ b/examples/resources/env0_aws_credentials/resource.tf @@ -1,5 +1,4 @@ resource "env0_aws_credentials" "credentials" { - name = "example" - arn = "Example role ARN" - external_id = "Example external id" -} \ No newline at end of file + name = "example" + arn = "Example role ARN" +} diff --git a/examples/resources/env0_cloud_credentials_project_assignment/resource.tf b/examples/resources/env0_cloud_credentials_project_assignment/resource.tf index f5c9c3fa..964fb4fd 100644 --- a/examples/resources/env0_cloud_credentials_project_assignment/resource.tf +++ b/examples/resources/env0_cloud_credentials_project_assignment/resource.tf @@ -1,7 +1,6 @@ resource "env0_aws_credentials" "credentials" { - name = "example" - arn = "Example role ARN" - external_id = "Example external id" + name = "example" + arn = "Example role ARN" } data "env0_project" "project" { @@ -11,4 +10,4 @@ data "env0_project" "project" { resource "env0_cloud_credentials_project_assignment" "example" { credential_id = env0_aws_credentials.credentials.id project_id = data.env0_project.project.id -} \ No newline at end of file +} diff --git a/tests/integration/006_aws_credentials/main.tf b/tests/integration/006_aws_credentials/main.tf index 74b24092..7edd8198 100644 --- a/tests/integration/006_aws_credentials/main.tf +++ b/tests/integration/006_aws_credentials/main.tf @@ -5,9 +5,8 @@ resource "random_string" "random" { } resource "env0_aws_credentials" "my_role_by_arn" { - name = "Test Role arn ${random_string.random.result}" - arn = "Role ARN" - external_id = "External-id" + name = "Test Role arn ${random_string.random.result}" + arn = "Role ARN" } data "env0_aws_credentials" "my_role_by_arn" { diff --git a/tests/integration/008_cloud_credentials_project_assignment/main.tf b/tests/integration/008_cloud_credentials_project_assignment/main.tf index 577bfa78..34a6a5c8 100644 --- a/tests/integration/008_cloud_credentials_project_assignment/main.tf +++ b/tests/integration/008_cloud_credentials_project_assignment/main.tf @@ -15,9 +15,8 @@ resource "env0_project" "test_project" { } resource "env0_aws_credentials" "credentials" { - name = "example-${random_string.random.result}" - arn = "Example role ARN" - external_id = "Example-external-id" + name = "example-${random_string.random.result}" + arn = "Example role ARN" } data "env0_project_cloud_credentials" "project_cloud_credentials" { diff --git a/tests/integration/023_cost_credentials_project_assignment/main.tf b/tests/integration/023_cost_credentials_project_assignment/main.tf index c245d937..afcc860c 100644 --- a/tests/integration/023_cost_credentials_project_assignment/main.tf +++ b/tests/integration/023_cost_credentials_project_assignment/main.tf @@ -12,9 +12,8 @@ resource "env0_project" "project" { } resource "env0_aws_cost_credentials" "cost" { - name = "cost-${random_string.random.result}" - arn = "arn" - external_id = "external-id" + name = "cost-${random_string.random.result}" + arn = "arn" } resource "env0_cost_credentials_project_assignment" "cost_project_assignment" { diff --git a/tests/integration/024_cloud_credentials/main.tf b/tests/integration/024_cloud_credentials/main.tf index 4ad45c8a..fb921cc1 100644 --- a/tests/integration/024_cloud_credentials/main.tf +++ b/tests/integration/024_cloud_credentials/main.tf @@ -5,9 +5,8 @@ resource "random_string" "random" { } resource "env0_aws_credentials" "cred1" { - name = "Test Role arn1 ${random_string.random.result}" - arn = "Role ARN1" - external_id = "External-id1" + name = "Test Role arn1 ${random_string.random.result}" + arn = "Role ARN1" } resource "env0_gcp_credentials" "cred2" {