Merge pull request #27991 from hashicorp/td-awserr-casts

Adds Semgrep rules for casting of `awserr`

.changelog/27991.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_opsworks_permission: `stack_id` and `user_arn` are both Required and ForceNew
+```
+
+```release-note:enhancement
+resource/aws_efs_mount_target: Add configurable timeouts for Create and Delete
+```

.ci/.semgrep.yml

@@ -833,21 +833,6 @@ rules:
     pattern: int(*$VALUE)
     severity: WARNING
 
-  - id: aws-go-sdk-error-code-helper
-    languages: [go]
-    message: "Use `tfawserr` helpers for checking AWS Go SDK errors (e.g. `tfawserr.ErrMessageContains(err, CODE, MESSAGE)`)"
-    paths:
-      include:
-        - internal/
-    patterns:
-      - pattern-either:
-          - pattern: if $AWSERR, $OK := $ORIGINALERR.(awserr.Error); $OK && $AWSERR.Code() == $CODE { $BODY }
-          - pattern: |
-              if $AWSERR, $OK := $ORIGINALERR.(awserr.Error); $OK {
-                if $AWSERR.Code() == $CODE { $BODY }
-              }
-    severity: WARNING
-
   - id: fmt-Errorf-awserr-Error-Code
     languages: [go]
     message: Prefer `err` with `%w` format verb instead of `err.Code()` or `err.Message()`

.ci/semgrep/aws/awserr.yml

@@ -0,0 +1,9 @@
+rules:
+  - id: aws-go-sdk-error-code-helper
+    languages: [go]
+    message: 'Use `tfawserr` helpers for checking AWS Go SDK v1 errors (e.g. `tfawserr.ErrMessageContains(err, CODE, MESSAGE)`)'
+    paths:
+      include:
+        - internal/
+    pattern: $AWSERR, $OK := $ORIGINALERR.(awserr.Error)
+    severity: WARNING

internal/acctest/crypto.go

@@ -25,7 +25,28 @@ const (
 	PEMBlockTypePublicKey          = `PUBLIC KEY`
 )
 
-var tlsX509CertificateSerialNumberLimit = new(big.Int).Lsh(big.NewInt(1), 128) //nolint:gomnd
+var (
+	tlsX509CertificateSerialNumberLimit = new(big.Int).Lsh(big.NewInt(1), 128) //nolint:gomnd
+)
+
+// TLSPEMRemovePublicKeyEncapsulationBoundaries removes public key
+// pre and post encapsulation boundaries from a PEM string.
+func TLSPEMRemovePublicKeyEncapsulationBoundaries(pem string) string {
+	return removePEMEncapsulationBoundaries(pem, PEMBlockTypePublicKey)
+}
+
+func removePEMEncapsulationBoundaries(pem, label string) string {
+	return strings.ReplaceAll(strings.ReplaceAll(pem, pemPreEncapsulationBoundary(label), ""), pemPostEncapsulationBoundary(label), "")
+}
+
+// See https://www.rfc-editor.org/rfc/rfc7468#section-2.
+func pemPreEncapsulationBoundary(label string) string {
+	return `-----BEGIN ` + label + `-----`
+}
+
+func pemPostEncapsulationBoundary(label string) string {
+	return `-----END ` + label + `-----`
+}
 
 // TLSECDSAPublicKeyPEM generates an ECDSA private key PEM string using the specified elliptic curve.
 // Wrap with TLSPEMEscapeNewlines() to allow simple fmt.Sprintf()
@@ -392,6 +413,10 @@ func TLSPEMEscapeNewlines(pem string) string {
 	return strings.ReplaceAll(pem, "\n", "\\n")
 }
 
+func TLSPEMRemoveNewlines(pem string) string {
+	return strings.ReplaceAll(pem, "\n", "")
+}
+
 func ellipticCurveForName(name string) elliptic.Curve {
 	switch name {
 	case "P-224":

internal/acctest/crypto_test.go

@@ -87,3 +87,47 @@ func TestTLSECDSAPublicKeyPEM(t *testing.T) {
 		t.Errorf("key does not contain PUBLIC KEY: %s", publicKey)
 	}
 }
+
+func TestTLSPEMEscapeNewlines(t *testing.T) {
+	t.Parallel()
+
+	input := `
+ABCD
+12345
+`
+	want := "\\nABCD\\n12345\\n"
+
+	if got := acctest.TLSPEMEscapeNewlines(input); got != want {
+		t.Errorf("got: %s\nwant: %s", got, want)
+	}
+}
+
+func TestTLSPEMRemovePublicKeyEncapsulationBoundaries(t *testing.T) {
+	t.Parallel()
+
+	input := `-----BEGIN PUBLIC KEY-----
+ABCD
+12345
+-----END PUBLIC KEY-----
+`
+	want := "\nABCD\n12345\n\n"
+
+	if got := acctest.TLSPEMRemovePublicKeyEncapsulationBoundaries(input); got != want {
+		t.Errorf("got: %s\nwant: %s", got, want)
+	}
+}
+
+func TestTLSPEMRemoveNewlines(t *testing.T) {
+	t.Parallel()
+
+	input := `
+ABCD
+12345
+
+`
+	want := "ABCD12345"
+
+	if got := acctest.TLSPEMRemoveNewlines(input); got != want {
+		t.Errorf("got: %s\nwant: %s", got, want)
+	}
+}

internal/service/apigateway/api_key.go

@@ -11,11 +11,13 @@ import (
 	"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-plugin-sdk/v2/helper/validation"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
 	"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
 	tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
+	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
 )
 
@@ -83,17 +85,20 @@ func resourceAPIKeyCreate(ctx context.Context, d *schema.ResourceData, meta inte
 	conn := meta.(*conns.AWSClient).APIGatewayConn()
 	defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig
 	tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{})))
-	log.Printf("[DEBUG] Creating API Gateway API Key")
 
-	apiKey, err := conn.CreateApiKeyWithContext(ctx, &apigateway.CreateApiKeyInput{
-		Name:        aws.String(d.Get("name").(string)),
+	name := d.Get("name").(string)
+	input := &apigateway.CreateApiKeyInput{
 		Description: aws.String(d.Get("description").(string)),
 		Enabled:     aws.Bool(d.Get("enabled").(bool)),
-		Value:       aws.String(d.Get("value").(string)),
+		Name:        aws.String(name),
 		Tags:        Tags(tags.IgnoreAWS()),
-	})
+		Value:       aws.String(d.Get("value").(string)),
+	}
+
+	apiKey, err := conn.CreateApiKeyWithContext(ctx, input)
+
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "creating API Gateway API Key: %s", err)
+		return sdkdiag.AppendErrorf(diags, "creating API Gateway API Key (%s): %s", name, err)
 	}
 
 	d.SetId(aws.StringValue(apiKey.Id))
@@ -107,19 +112,15 @@ func resourceAPIKeyRead(ctx context.Context, d *schema.ResourceData, meta interf
 	defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig
 	ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig
 
-	log.Printf("[DEBUG] Reading API Gateway API Key: %s", d.Id())
+	apiKey, err := FindAPIKeyByID(ctx, conn, d.Id())
 
-	apiKey, err := conn.GetApiKeyWithContext(ctx, &apigateway.GetApiKeyInput{
-		ApiKey:       aws.String(d.Id()),
-		IncludeValue: aws.Bool(true),
-	})
-	if err != nil {
-		if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, apigateway.ErrCodeNotFoundException) {
-			log.Printf("[WARN] API Gateway API Key (%s) not found, removing from state", d.Id())
-			d.SetId("")
-			return diags
-		}
+	if !d.IsNewResource() && tfresource.NotFound(err) {
+		log.Printf("[WARN] API Gateway API Key (%s) not found, removing from state", d.Id())
+		d.SetId("")
+		return diags
+	}
 
+	if err != nil {
 		return sdkdiag.AppendErrorf(diags, "reading API Gateway API Key (%s): %s", d.Id(), err)
 	}
 
@@ -187,31 +188,33 @@ func resourceAPIKeyUpdate(ctx context.Context, d *schema.ResourceData, meta inte
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).APIGatewayConn()
 
-	log.Printf("[DEBUG] Updating API Gateway API Key: %s", d.Id())
+	if d.HasChangesExcept("tags", "tags_all") {
+		_, err := conn.UpdateApiKeyWithContext(ctx, &apigateway.UpdateApiKeyInput{
+			ApiKey:          aws.String(d.Id()),
+			PatchOperations: resourceAPIKeyUpdateOperations(d),
+		})
+
+		if err != nil {
+			return sdkdiag.AppendErrorf(diags, "updating API Gateway API Key (%s): %s", d.Id(), err)
+		}
+	}
 
 	if d.HasChange("tags_all") {
 		o, n := d.GetChange("tags_all")
+
 		if err := UpdateTags(ctx, conn, d.Get("arn").(string), o, n); err != nil {
 			return sdkdiag.AppendErrorf(diags, "updating tags: %s", err)
 		}
 	}
 
-	_, err := conn.UpdateApiKeyWithContext(ctx, &apigateway.UpdateApiKeyInput{
-		ApiKey:          aws.String(d.Id()),
-		PatchOperations: resourceAPIKeyUpdateOperations(d),
-	})
-	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "updating API Gateway API Key (%s): %s", d.Id(), err)
-	}
-
 	return append(diags, resourceAPIKeyRead(ctx, d, meta)...)
 }
 
 func resourceAPIKeyDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).APIGatewayConn()
-	log.Printf("[DEBUG] Deleting API Gateway API Key: %s", d.Id())
 
+	log.Printf("[DEBUG] Deleting API Gateway API Key: %s", d.Id())
 	_, err := conn.DeleteApiKeyWithContext(ctx, &apigateway.DeleteApiKeyInput{
 		ApiKey: aws.String(d.Id()),
 	})
@@ -226,3 +229,29 @@ func resourceAPIKeyDelete(ctx context.Context, d *schema.ResourceData, meta inte
 
 	return diags
 }
+
+func FindAPIKeyByID(ctx context.Context, conn *apigateway.APIGateway, id string) (*apigateway.ApiKey, error) {
+	input := &apigateway.GetApiKeyInput{
+		ApiKey:       aws.String(id),
+		IncludeValue: aws.Bool(true),
+	}
+
+	output, err := conn.GetApiKeyWithContext(ctx, input)
+
+	if tfawserr.ErrCodeEquals(err, apigateway.ErrCodeNotFoundException) {
+		return nil, &resource.NotFoundError{
+			LastError:   err,
+			LastRequest: input,
+		}
+	}
+
+	if err != nil {
+		return nil, err
+	}
+
+	if output == nil {
+		return nil, tfresource.NewEmptyResultError(input)
+	}
+
+	return output, nil
+}

internal/service/apigateway/api_key_data_source.go

@@ -5,7 +5,6 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/apigateway"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
@@ -56,13 +55,11 @@ func dataSourceAPIKeyRead(ctx context.Context, d *schema.ResourceData, meta inte
 	conn := meta.(*conns.AWSClient).APIGatewayConn()
 	ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig
 
-	apiKey, err := conn.GetApiKeyWithContext(ctx, &apigateway.GetApiKeyInput{
-		ApiKey:       aws.String(d.Get("id").(string)),
-		IncludeValue: aws.Bool(true),
-	})
+	id := d.Get("id").(string)
+	apiKey, err := FindAPIKeyByID(ctx, conn, id)
 
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "reading API Gateway API Key (%s): %s", d.Get("id").(string), err)
+		return sdkdiag.AppendErrorf(diags, "reading API Gateway API Key (%s): %s", id, err)
 	}
 
 	d.SetId(aws.StringValue(apiKey.Id))
@@ -76,5 +73,6 @@ func dataSourceAPIKeyRead(ctx context.Context, d *schema.ResourceData, meta inte
 	if err := d.Set("tags", KeyValueTags(apiKey.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
 		return sdkdiag.AppendErrorf(diags, "setting tags: %s", err)
 	}
+
 	return diags
 }

internal/service/apigateway/api_key_data_source_test.go

@@ -11,9 +11,9 @@ import (
 )
 
 func TestAccAPIGatewayAPIKeyDataSource_basic(t *testing.T) {
-	rName := sdkacctest.RandString(8)
-	resourceName1 := "aws_api_gateway_api_key.example_key"
-	dataSourceName1 := "data.aws_api_gateway_api_key.test_key"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_api_gateway_api_key.test"
+	dataSourceName := "data.aws_api_gateway_api_key.test"
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:                 func() { acctest.PreCheck(t) },
@@ -23,14 +23,14 @@ func TestAccAPIGatewayAPIKeyDataSource_basic(t *testing.T) {
 			{
 				Config: testAccAPIKeyDataSourceConfig_basic(rName),
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttrPair(resourceName1, "id", dataSourceName1, "id"),
-					resource.TestCheckResourceAttrPair(resourceName1, "name", dataSourceName1, "name"),
-					resource.TestCheckResourceAttrPair(resourceName1, "value", dataSourceName1, "value"),
-					resource.TestCheckResourceAttrPair(resourceName1, "enabled", dataSourceName1, "enabled"),
-					resource.TestCheckResourceAttrPair(resourceName1, "description", dataSourceName1, "description"),
-					resource.TestCheckResourceAttrSet(dataSourceName1, "last_updated_date"),
-					resource.TestCheckResourceAttrSet(dataSourceName1, "created_date"),
-					resource.TestCheckResourceAttr(dataSourceName1, "tags.%", "0"),
+					resource.TestCheckResourceAttrPair(resourceName, "id", dataSourceName, "id"),
+					resource.TestCheckResourceAttrPair(resourceName, "name", dataSourceName, "name"),
+					resource.TestCheckResourceAttrPair(resourceName, "value", dataSourceName, "value"),
+					resource.TestCheckResourceAttrPair(resourceName, "enabled", dataSourceName, "enabled"),
+					resource.TestCheckResourceAttrPair(resourceName, "description", dataSourceName, "description"),
+					resource.TestCheckResourceAttrSet(dataSourceName, "last_updated_date"),
+					resource.TestCheckResourceAttrSet(dataSourceName, "created_date"),
+					resource.TestCheckResourceAttr(dataSourceName, "tags.%", "0"),
 				),
 			},
 		},
@@ -39,12 +39,12 @@ func TestAccAPIGatewayAPIKeyDataSource_basic(t *testing.T) {
 
 func testAccAPIKeyDataSourceConfig_basic(r string) string {
 	return fmt.Sprintf(`
-resource "aws_api_gateway_api_key" "example_key" {
-  name = "%s"
+resource "aws_api_gateway_api_key" "test" {
+  name = %[1]q
 }
 
-data "aws_api_gateway_api_key" "test_key" {
-  id = aws_api_gateway_api_key.example_key.id
+data "aws_api_gateway_api_key" "test" {
+  id = aws_api_gateway_api_key.test.id
 }
 `, r)
 }