From 0b02dd7b480de8686e35029b556ff11920aade8a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 18 Jan 2023 15:46:48 -0500 Subject: [PATCH 1/6] elbv2/listener_certificate: Display errors on read --- internal/service/elbv2/listener_certificate.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/internal/service/elbv2/listener_certificate.go b/internal/service/elbv2/listener_certificate.go index 75df588e68c..071abd1f931 100644 --- a/internal/service/elbv2/listener_certificate.go +++ b/internal/service/elbv2/listener_certificate.go @@ -11,6 +11,7 @@ import ( "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" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) @@ -114,15 +115,22 @@ func resourceListenerCertificateRead(d *schema.ResourceData, meta interface{}) e return nil }) + if tfresource.TimedOut(err) { certificate, err = findListenerCertificate(certificateArn, listenerArn, true, nil, conn) } + + if !d.IsNewResource() && errs.Contains(err, "certificate not found") { + log.Printf("[WARN] ELBv2 Listener Certificate (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err == nil && certificate == nil { + return fmt.Errorf("reading ELB v2 Listener Certificate (%s): certificate not found", d.Id()) + } + if err != nil { - if certificate == nil { - log.Printf("[WARN] ELB v2 Listener Certificate (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } return fmt.Errorf("reading ELB v2 Listener Certificate (%s): %w", d.Id(), err) } From 560027c3e6318a3e439cc540270ca02881e7f644 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 18 Jan 2023 16:01:48 -0500 Subject: [PATCH 2/6] Add changelog --- .changelog/28968.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/28968.txt diff --git a/.changelog/28968.txt b/.changelog/28968.txt new file mode 100644 index 00000000000..9aca1037424 --- /dev/null +++ b/.changelog/28968.txt @@ -0,0 +1,7 @@ +```release-note:bug +resource/aws_lb_listener_certificate: Show errors in certain cases where they were previously only logged and resource was removed from state +``` + +```release-note:bug +resource/aws_iam_server_certificate: Avoid errors on delete when no error occurred +``` From b3c070cf15d11a487613ffff90cc8c9b9bcc8bba Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 18 Jan 2023 16:02:29 -0500 Subject: [PATCH 3/6] iam/server_certificate: Avoid errors on delete when there is none --- internal/service/iam/server_certificate.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/service/iam/server_certificate.go b/internal/service/iam/server_certificate.go index d465c119ab2..536d7face15 100644 --- a/internal/service/iam/server_certificate.go +++ b/internal/service/iam/server_certificate.go @@ -276,7 +276,11 @@ func resourceServerCertificateDelete(d *schema.ResourceData, meta interface{}) e }) } - return fmt.Errorf("deleting IAM Server Certificate (%s): %w", d.Id(), err) + if err != nil { + return fmt.Errorf("deleting IAM Server Certificate (%s): %w", d.Id(), err) + } + + return nil } func resourceServerCertificateImport( From 06a2b114211f87d95c3bcf54c0f43409f041e86a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 18 Jan 2023 17:09:45 -0500 Subject: [PATCH 4/6] Streamline logic --- .../service/elbv2/listener_certificate.go | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/internal/service/elbv2/listener_certificate.go b/internal/service/elbv2/listener_certificate.go index 071abd1f931..09cace79346 100644 --- a/internal/service/elbv2/listener_certificate.go +++ b/internal/service/elbv2/listener_certificate.go @@ -11,9 +11,10 @@ import ( "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" + "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" + "github.com/hashicorp/terraform-provider-aws/names" ) func ResourceListenerCertificate() *schema.Resource { @@ -42,6 +43,11 @@ func ResourceListenerCertificate() *schema.Resource { } } +const ( + ResNameListenerCertificate = "Listener Certificate" + ListenerCertificateNotFound = "ListenerCertificateNotFound" +) + func resourceListenerCertificateCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).ELBV2Conn() @@ -97,19 +103,14 @@ func resourceListenerCertificateRead(d *schema.ResourceData, meta interface{}) e log.Printf("[DEBUG] Reading certificate: %s of listener: %s", certificateArn, listenerArn) - var certificate *elbv2.Certificate err = resource.Retry(1*time.Minute, func() *resource.RetryError { var err error - certificate, err = findListenerCertificate(certificateArn, listenerArn, true, nil, conn) - if err != nil { - return resource.NonRetryableError(err) + err = findListenerCertificate(certificateArn, listenerArn, true, nil, conn) + if tfresource.NotFound(err) && d.IsNewResource() { + return resource.RetryableError(err) } - if certificate == nil { - err = fmt.Errorf("certificate not found: %s", certificateArn) - if d.IsNewResource() { - return resource.RetryableError(err) - } + if err != nil { return resource.NonRetryableError(err) } @@ -117,21 +118,17 @@ func resourceListenerCertificateRead(d *schema.ResourceData, meta interface{}) e }) if tfresource.TimedOut(err) { - certificate, err = findListenerCertificate(certificateArn, listenerArn, true, nil, conn) + err = findListenerCertificate(certificateArn, listenerArn, true, nil, conn) } - if !d.IsNewResource() && errs.Contains(err, "certificate not found") { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] ELBv2 Listener Certificate (%s) not found, removing from state", d.Id()) d.SetId("") return nil } - if err == nil && certificate == nil { - return fmt.Errorf("reading ELB v2 Listener Certificate (%s): certificate not found", d.Id()) - } - if err != nil { - return fmt.Errorf("reading ELB v2 Listener Certificate (%s): %w", d.Id(), err) + return create.Error(names.ELBV2, create.ErrActionReading, ResNameListenerCertificate, d.Id(), err) } d.Set("certificate_arn", certificateArn) @@ -171,7 +168,7 @@ func resourceListenerCertificateDelete(d *schema.ResourceData, meta interface{}) return nil } -func findListenerCertificate(certificateArn, listenerArn string, skipDefault bool, nextMarker *string, conn *elbv2.ELBV2) (*elbv2.Certificate, error) { +func findListenerCertificate(certificateArn, listenerArn string, skipDefault bool, nextMarker *string, conn *elbv2.ELBV2) error { params := &elbv2.DescribeListenerCertificatesInput{ ListenerArn: aws.String(listenerArn), PageSize: aws.Int64(400), @@ -182,7 +179,7 @@ func findListenerCertificate(certificateArn, listenerArn string, skipDefault boo resp, err := conn.DescribeListenerCertificates(params) if err != nil { - return nil, err + return err } for _, cert := range resp.Certificates { @@ -191,12 +188,16 @@ func findListenerCertificate(certificateArn, listenerArn string, skipDefault boo } if aws.StringValue(cert.CertificateArn) == certificateArn { - return cert, nil + return nil } } if resp.NextMarker != nil { return findListenerCertificate(certificateArn, listenerArn, skipDefault, resp.NextMarker, conn) } - return nil, nil + + return &resource.NotFoundError{ + LastRequest: params, + Message: fmt.Sprintf("%s: certificate %s for listener %s not found", ListenerCertificateNotFound, certificateArn, listenerArn), + } } From 334a61e7e3410bfb820c6b899dd307c55ab058d6 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 18 Jan 2023 17:20:36 -0500 Subject: [PATCH 5/6] Modernize --- .../service/elbv2/listener_certificate.go | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/internal/service/elbv2/listener_certificate.go b/internal/service/elbv2/listener_certificate.go index 09cace79346..819daabfd36 100644 --- a/internal/service/elbv2/listener_certificate.go +++ b/internal/service/elbv2/listener_certificate.go @@ -56,11 +56,9 @@ func resourceListenerCertificateCreate(d *schema.ResourceData, meta interface{}) params := &elbv2.AddListenerCertificatesInput{ ListenerArn: aws.String(listenerArn), - Certificates: []*elbv2.Certificate{ - { - CertificateArn: aws.String(certificateArn), - }, - }, + Certificates: []*elbv2.Certificate{{ + CertificateArn: aws.String(certificateArn), + }}, } log.Printf("[DEBUG] Adding certificate: %s of listener: %s", certificateArn, listenerArn) @@ -85,7 +83,7 @@ func resourceListenerCertificateCreate(d *schema.ResourceData, meta interface{}) } if err != nil { - return fmt.Errorf("adding LB Listener Certificate: %w", err) + return create.Error(names.ELBV2, create.ErrActionCreating, ResNameListenerCertificate, d.Id(), err) } d.SetId(listenerCertificateCreateID(listenerArn, certificateArn)) @@ -98,7 +96,7 @@ func resourceListenerCertificateRead(d *schema.ResourceData, meta interface{}) e listenerArn, certificateArn, err := listenerCertificateParseID(d.Id()) if err != nil { - return fmt.Errorf("reading ELB v2 Listener Certificate (%s): %w", d.Id(), err) + return create.Error(names.ELBV2, create.ErrActionReading, ResNameListenerCertificate, d.Id(), err) } log.Printf("[DEBUG] Reading certificate: %s of listener: %s", certificateArn, listenerArn) @@ -122,7 +120,7 @@ func resourceListenerCertificateRead(d *schema.ResourceData, meta interface{}) e } if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] ELBv2 Listener Certificate (%s) not found, removing from state", d.Id()) + create.LogNotFoundRemoveState(names.ELBV2, create.ErrActionReading, ResNameListenerCertificate, d.Id()) d.SetId("") return nil } @@ -147,11 +145,9 @@ func resourceListenerCertificateDelete(d *schema.ResourceData, meta interface{}) params := &elbv2.RemoveListenerCertificatesInput{ ListenerArn: aws.String(listenerArn), - Certificates: []*elbv2.Certificate{ - { - CertificateArn: aws.String(certificateArn), - }, - }, + Certificates: []*elbv2.Certificate{{ + CertificateArn: aws.String(certificateArn), + }}, } _, err := conn.RemoveListenerCertificates(params) @@ -162,7 +158,8 @@ func resourceListenerCertificateDelete(d *schema.ResourceData, meta interface{}) if tfawserr.ErrCodeEquals(err, elbv2.ErrCodeListenerNotFoundException) { return nil } - return fmt.Errorf("Error removing LB Listener Certificate: %w", err) + + return create.Error(names.ELBV2, create.ErrActionDeleting, ResNameListenerCertificate, d.Id(), err) } return nil From aeedd17741bf01e4fafbd4b4134a9f339aeea149 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 18 Jan 2023 21:12:39 -0500 Subject: [PATCH 6/6] Merge variable declaration --- internal/service/elbv2/listener_certificate.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/service/elbv2/listener_certificate.go b/internal/service/elbv2/listener_certificate.go index 819daabfd36..47b6c6cef19 100644 --- a/internal/service/elbv2/listener_certificate.go +++ b/internal/service/elbv2/listener_certificate.go @@ -102,8 +102,7 @@ func resourceListenerCertificateRead(d *schema.ResourceData, meta interface{}) e log.Printf("[DEBUG] Reading certificate: %s of listener: %s", certificateArn, listenerArn) err = resource.Retry(1*time.Minute, func() *resource.RetryError { - var err error - err = findListenerCertificate(certificateArn, listenerArn, true, nil, conn) + err := findListenerCertificate(certificateArn, listenerArn, true, nil, conn) if tfresource.NotFound(err) && d.IsNewResource() { return resource.RetryableError(err) }