diff --git a/.semgrep.yml b/.semgrep.yml index 0a8c8c4a7e1b..1e8f05ecde47 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -227,6 +227,44 @@ rules: - pattern: if $VALUE, $OK := d.GetOk($KEY); $OK && len($VALUE.(string)) > 0 { $BODY } severity: WARNING + - id: helper-schema-ResourceData-SetId-empty-without-IsNewResource-check + languages: [go] + message: Calling `d.SetId("")` should ensure `!d.IsNewResource()` is checked first + paths: + exclude: + - aws/resource_aws_api_gateway_*.go + - aws/resource_aws_apigatewayv2_*.go + - aws/resource_aws_app_cookie_stickiness_policy.go + - aws/resource_aws_appautoscaling_*.go + - aws/resource_aws_appmesh_*.go + - aws/resource_aws_appsync_*.go + - aws/resource_aws_athena_*.go + - aws/resource_aws_autoscaling_*.go + - aws/resource_aws_autoscalingplans_scaling_plan.go + - aws/resource_aws_[b-g]*.go + - aws/resource_aws_i*.go + - aws/resource_aws_[k-t]*.go + - aws/resource_aws_[v-x]*.go + include: + - aws/resource*.go + patterns: + - pattern-either: + - pattern: | + d.SetId("") + ... + return nil + - pattern-not-inside: | + if ... { + if <... d.IsNewResource() ...> { ... } + ... + d.SetId("") + ... + return nil + } + - pattern-not-inside: | + if <... d.IsNewResource() ...> { ... } + severity: WARNING + - id: helper-schema-resource-Retry-without-TimeoutError-check languages: [go] message: Check resource.Retry() errors with tfresource.TimedOut() diff --git a/aws/resource_aws_accessanalyzer_analyzer.go b/aws/resource_aws_accessanalyzer_analyzer.go index c07154b97fc5..52fda89c7c69 100644 --- a/aws/resource_aws_accessanalyzer_analyzer.go +++ b/aws/resource_aws_accessanalyzer_analyzer.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/accessanalyzer" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "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" @@ -110,7 +111,7 @@ func resourceAwsAccessAnalyzerAnalyzerRead(d *schema.ResourceData, meta interfac output, err := conn.GetAnalyzer(input) - if isAWSErr(err, accessanalyzer.ErrCodeResourceNotFoundException, "") { + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, accessanalyzer.ErrCodeResourceNotFoundException) { log.Printf("[WARN] Access Analyzer Analyzer (%s) not found, removing from state", d.Id()) d.SetId("") return nil diff --git a/aws/resource_aws_acm_certificate_validation.go b/aws/resource_aws_acm_certificate_validation.go index 89ffc3459e6a..e35a802e3afc 100644 --- a/aws/resource_aws_acm_certificate_validation.go +++ b/aws/resource_aws_acm_certificate_validation.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/acm" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -177,19 +178,32 @@ func resourceAwsAcmCertificateValidationRead(d *schema.ResourceData, meta interf resp, err := acmconn.DescribeCertificate(params) - if err != nil && isAWSErr(err, acm.ErrCodeResourceNotFoundException, "") { + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, acm.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] ACM Certificate (%s) not found, removing from state", d.Id()) d.SetId("") return nil - } else if err != nil { - return fmt.Errorf("Error describing certificate: %w", err) } - if aws.StringValue(resp.Certificate.Status) != acm.CertificateStatusIssued { - log.Printf("[INFO] Certificate status not issued, was %s, tainting validation", aws.StringValue(resp.Certificate.Status)) + if err != nil { + return fmt.Errorf("error describing ACM Certificate (%s): %w", d.Id(), err) + } + + if resp == nil || resp.Certificate == nil { + return fmt.Errorf("error describing ACM Certificate (%s): empty response", d.Id()) + } + + if status := aws.StringValue(resp.Certificate.Status); status != acm.CertificateStatusIssued { + if d.IsNewResource() { + return fmt.Errorf("ACM Certificate (%s) status not issued: %s", d.Id(), status) + } + + log.Printf("[WARN] ACM Certificate (%s) status not issued (%s), removing from state", d.Id(), status) d.SetId("") - } else { - d.SetId(aws.TimeValue(resp.Certificate.IssuedAt).String()) + return nil } + + d.SetId(aws.TimeValue(resp.Certificate.IssuedAt).String()) + return nil } diff --git a/aws/resource_aws_acmpca_certificate.go b/aws/resource_aws_acmpca_certificate.go index 5a6317d0ec47..afbcb51f8a3c 100644 --- a/aws/resource_aws_acmpca_certificate.go +++ b/aws/resource_aws_acmpca_certificate.go @@ -169,13 +169,19 @@ func resourceAwsAcmpcaCertificateRead(d *schema.ResourceData, meta interface{}) log.Printf("[DEBUG] Reading ACM PCA Certificate: %s", getCertificateInput) certificateOutput, err := conn.GetCertificate(getCertificateInput) + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, acmpca.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] ACM PCA Certificate (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { - if isAWSErr(err, acmpca.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] ACM PCA Certificate (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - return fmt.Errorf("error reading ACM PCA Certificate: %s", err) + return fmt.Errorf("error reading ACM PCA Certificate (%s): %w", d.Id(), err) + } + + if certificateOutput == nil { + return fmt.Errorf("error reading ACM PCA Certificate (%s): empty response", d.Id()) } d.Set("arn", d.Id()) diff --git a/aws/resource_aws_acmpca_certificate_authority.go b/aws/resource_aws_acmpca_certificate_authority.go index 2455f26e22a1..a27cde4b8825 100644 --- a/aws/resource_aws_acmpca_certificate_authority.go +++ b/aws/resource_aws_acmpca_certificate_authority.go @@ -324,17 +324,21 @@ func resourceAwsAcmpcaCertificateAuthorityRead(d *schema.ResourceData, meta inte certificateAuthority, err := finder.CertificateAuthorityByARN(conn, d.Id()) - if isAWSErr(err, acmpca.ErrCodeResourceNotFoundException, "") { + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, acmpca.ErrCodeResourceNotFoundException) { log.Printf("[WARN] ACM PCA Certificate Authority (%s) not found, removing from state", d.Id()) d.SetId("") return nil } if err != nil { - return fmt.Errorf("error reading ACM PCA Certificate Authority: %s", err) + return fmt.Errorf("error reading ACM PCA Certificate Authority (%s): %w", d.Id(), err) } if certificateAuthority == nil || aws.StringValue(certificateAuthority.Status) == acmpca.CertificateAuthorityStatusDeleted { + if d.IsNewResource() { + return fmt.Errorf("error reading ACM PCA Certificate Authority (%s): not found or deleted", d.Id()) + } + log.Printf("[WARN] ACM PCA Certificate Authority (%s) not found, removing from state", d.Id()) d.SetId("") return nil @@ -365,17 +369,17 @@ func resourceAwsAcmpcaCertificateAuthorityRead(d *schema.ResourceData, meta inte log.Printf("[DEBUG] Reading ACM PCA Certificate Authority Certificate: %s", getCertificateAuthorityCertificateInput) getCertificateAuthorityCertificateOutput, err := conn.GetCertificateAuthorityCertificate(getCertificateAuthorityCertificateInput) - if err != nil { - if isAWSErr(err, acmpca.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] ACM PCA Certificate Authority (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - // Returned when in PENDING_CERTIFICATE status - // InvalidStateException: The certificate authority XXXXX is not in the correct state to have a certificate signing request. - if !isAWSErr(err, acmpca.ErrCodeInvalidStateException, "") { - return fmt.Errorf("error reading ACM PCA Certificate Authority Certificate: %s", err) - } + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, acmpca.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] ACM PCA Certificate Authority (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + // Returned when in PENDING_CERTIFICATE status + // InvalidStateException: The certificate authority XXXXX is not in the correct state to have a certificate signing request. + if err != nil && !tfawserr.ErrCodeEquals(err, acmpca.ErrCodeInvalidStateException) { + return fmt.Errorf("error reading ACM PCA Certificate Authority (%s) Certificate: %w", d.Id(), err) } d.Set("certificate", "") @@ -392,15 +396,17 @@ func resourceAwsAcmpcaCertificateAuthorityRead(d *schema.ResourceData, meta inte log.Printf("[DEBUG] Reading ACM PCA Certificate Authority Certificate Signing Request: %s", getCertificateAuthorityCsrInput) getCertificateAuthorityCsrOutput, err := conn.GetCertificateAuthorityCsr(getCertificateAuthorityCsrInput) - if err != nil { - if isAWSErr(err, acmpca.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] ACM PCA Certificate Authority (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - if !isAWSErr(err, acmpca.ErrCodeInvalidStateException, "") { - return fmt.Errorf("error reading ACM PCA Certificate Authority Certificate Signing Request: %s", err) - } + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, acmpca.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] ACM PCA Certificate Authority (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + // Returned when in PENDING_CERTIFICATE status + // InvalidStateException: The certificate authority XXXXX is not in the correct state to have a certificate signing request. + if err != nil && !tfawserr.ErrCodeEquals(err, acmpca.ErrCodeInvalidStateException) { + return fmt.Errorf("error reading ACM PCA Certificate Authority (%s) Certificate Signing Request: %w", d.Id(), err) } d.Set("certificate_signing_request", "") diff --git a/aws/resource_aws_ami.go b/aws/resource_aws_ami.go index 574a9ad7a1fc..5a11ffe13e0e 100644 --- a/aws/resource_aws_ami.go +++ b/aws/resource_aws_ami.go @@ -356,7 +356,11 @@ func resourceAwsAmiRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Unable to find AMI after retries: %s", err) } - if len(res.Images) != 1 { + if res == nil || len(res.Images) != 1 { + if d.IsNewResource() { + return fmt.Errorf("error reading EC2 AMI (%s): empty response", d.Id()) + } + log.Printf("[WARN] AMI (%s) not found, removing from state", d.Id()) d.SetId("") return nil @@ -379,6 +383,10 @@ func resourceAwsAmiRead(d *schema.ResourceData, meta interface{}) error { } if state == ec2.ImageStateDeregistered { + if d.IsNewResource() { + return fmt.Errorf("error reading EC2 AMI (%s): deregistered", d.Id()) + } + log.Printf("[WARN] AMI (%s) not found, removing from state", d.Id()) d.SetId("") return nil diff --git a/aws/resource_aws_ami_launch_permission.go b/aws/resource_aws_ami_launch_permission.go index 43bcd8f21f82..8d4d190d2812 100644 --- a/aws/resource_aws_ami_launch_permission.go +++ b/aws/resource_aws_ami_launch_permission.go @@ -77,6 +77,10 @@ func resourceAwsAmiLaunchPermissionRead(d *schema.ResourceData, meta interface{} return fmt.Errorf("error reading AMI launch permission (%s): %w", d.Id(), err) } if !exists { + if d.IsNewResource() { + return fmt.Errorf("error reading EC2 AMI Launch Permission (%s): not found", d.Id()) + } + log.Printf("[WARN] AMI launch permission (%s) not found, removing from state", d.Id()) d.SetId("") return nil