Feat: stop sending external_id for env0_aws_credentials resource

env0 · Mar 12, 2023 · 5b22348 · 5b22348
1 parent 5b306cb
commit 5b22348
Show file tree

Hide file tree

Showing 13 changed files with 29 additions and 87 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -17,31 +17,27 @@ 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,
 	}
 
 	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
@@ -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
@@ -1,5 +1,4 @@
 resource "env0_aws_credentials" "credentials" {
-  name        = "example"
-  arn         = "Example role ARN"
-  external_id = "Example external id"
-}
+  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
@@ -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
-}
+}
diff --git 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
@@ -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
@@ -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
@@ -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" {