Merge pull request #29991 from deepan83/main

b-cachekeyparameters-29910
hashicorp · Mar 15, 2023 · eb02d81 · eb02d81
2 parents ff1ecde + eda2bdf
commit eb02d81
Show file tree

Hide file tree

Showing 7 changed files with 287 additions and 307 deletions.
diff --git a/.changelog/29991.txt b/.changelog/29991.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_api_gateway_integration: Fix bug that cleared unchanged `cache_key_parameters` values on Update
+```
diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go
@@ -2,13 +2,11 @@ package apigateway
 
 import (
 	"context"
-	"log"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/apigateway"
 	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
 	"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
@@ -21,7 +19,8 @@ func ResourceAccount() *schema.Resource {
 		CreateWithoutTimeout: resourceAccountUpdate,
 		ReadWithoutTimeout:   resourceAccountRead,
 		UpdateWithoutTimeout: resourceAccountUpdate,
-		DeleteWithoutTimeout: resourceAccountDelete,
+		DeleteWithoutTimeout: schema.NoopContext,
+
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
@@ -51,89 +50,73 @@ func ResourceAccount() *schema.Resource {
 	}
 }
 
-func resourceAccountRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).APIGatewayConn()
 
-	account, err := conn.GetAccountWithContext(ctx, &apigateway.GetAccountInput{})
-	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "reading API Gateway Account: %s", err)
+	input := &apigateway.UpdateAccountInput{}
+
+	// Unfortunately AWS API doesn't allow empty ARNs,
+	// even though that's default settings for new AWS accounts
+	// BadRequestException: The role ARN is not well formed
+	if v, ok := d.GetOk("cloudwatch_role_arn"); ok {
+		input.PatchOperations = []*apigateway.PatchOperation{{
+			Op:    aws.String(apigateway.OpReplace),
+			Path:  aws.String("/cloudwatchRoleArn"),
+			Value: aws.String(v.(string)),
+		}}
+	} else {
+		input.PatchOperations = []*apigateway.PatchOperation{}
 	}
 
-	log.Printf("[DEBUG] Received API Gateway Account: %s", account)
+	_, err := tfresource.RetryWhen(ctx, propagationTimeout,
+		func() (interface{}, error) {
+			return conn.UpdateAccountWithContext(ctx, input)
+		},
+		func(err error) (bool, error) {
+			if tfawserr.ErrMessageContains(err, apigateway.ErrCodeBadRequestException, "The role ARN does not have required permissions") {
+				return true, err
+			}
 
-	if _, ok := d.GetOk("cloudwatch_role_arn"); ok {
-		// CloudwatchRoleArn cannot be empty nor made empty via API
-		// This resource can however be useful w/out defining cloudwatch_role_arn
-		// (e.g. for referencing throttle_settings)
-		d.Set("cloudwatch_role_arn", account.CloudwatchRoleArn)
+			if tfawserr.ErrMessageContains(err, apigateway.ErrCodeBadRequestException, "API Gateway could not successfully write to CloudWatch Logs using the ARN specified") {
+				return true, err
+			}
+
+			return false, err
+		},
+	)
+
+	if err != nil {
+		return sdkdiag.AppendErrorf(diags, "updating API Gateway Account: %s", err)
 	}
-	if err := d.Set("throttle_settings", FlattenThrottleSettings(account.ThrottleSettings)); err != nil {
-		return sdkdiag.AppendErrorf(diags, "reading API Gateway Account: %s", err)
+
+	if d.IsNewResource() {
+		d.SetId("api-gateway-account")
 	}
 
-	return diags
+	return append(diags, resourceAccountRead(ctx, d, meta)...)
 }
 
-func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+func resourceAccountRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).APIGatewayConn()
 
-	input := apigateway.UpdateAccountInput{}
-	operations := make([]*apigateway.PatchOperation, 0)
-
-	if d.HasChange("cloudwatch_role_arn") {
-		arn := d.Get("cloudwatch_role_arn").(string)
-		if len(arn) > 0 {
-			// Unfortunately AWS API doesn't allow empty ARNs,
-			// even though that's default settings for new AWS accounts
-			// BadRequestException: The role ARN is not well formed
-			operations = append(operations, &apigateway.PatchOperation{
-				Op:    aws.String("replace"),
-				Path:  aws.String("/cloudwatchRoleArn"),
-				Value: aws.String(arn),
-			})
-		}
-	}
-	input.PatchOperations = operations
-
-	log.Printf("[INFO] Updating API Gateway Account: %s", input)
-
-	// Retry due to eventual consistency of IAM
-	expectedErrMsg := "The role ARN does not have required permissions"
-	otherErrMsg := "API Gateway could not successfully write to CloudWatch Logs using the ARN specified"
-	var out *apigateway.Account
-	var err error
-	err = resource.RetryContext(ctx, propagationTimeout, func() *resource.RetryError {
-		out, err = conn.UpdateAccountWithContext(ctx, &input)
-
-		if err != nil {
-			if tfawserr.ErrMessageContains(err, "BadRequestException", expectedErrMsg) ||
-				tfawserr.ErrMessageContains(err, "BadRequestException", otherErrMsg) {
-				log.Printf("[DEBUG] Retrying API Gateway Account update: %s", err)
-				return resource.RetryableError(err)
-			}
-			return resource.NonRetryableError(err)
-		}
+	account, err := conn.GetAccountWithContext(ctx, &apigateway.GetAccountInput{})
 
-		return nil
-	})
-	if tfresource.TimedOut(err) {
-		out, err = conn.UpdateAccountWithContext(ctx, &input)
-	}
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "Updating API Gateway Account failed: %s", err)
+		return sdkdiag.AppendErrorf(diags, "reading API Gateway Account: %s", err)
 	}
-	log.Printf("[DEBUG] API Gateway Account updated: %s", out)
 
-	d.SetId("api-gateway-account")
-	return append(diags, resourceAccountRead(ctx, d, meta)...)
-}
-
-func resourceAccountDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	var
-	// There is no API for "deleting" account or resetting it to "default" settings
-	diags diag.Diagnostics
+	if _, ok := d.GetOk("cloudwatch_role_arn"); ok {
+		// Backwards compatibility:
+		// CloudwatchRoleArn cannot be empty nor made empty via API
+		// This resource can however be useful w/out defining cloudwatch_role_arn
+		// (e.g. for referencing throttle_settings)
+		d.Set("cloudwatch_role_arn", account.CloudwatchRoleArn)
+	}
+	if err := d.Set("throttle_settings", flattenThrottleSettings(account.ThrottleSettings)); err != nil {
+		return sdkdiag.AppendErrorf(diags, "setting throttle_settings: %s", err)
+	}
 
 	return diags
 }