aws: fail on ListCertificatesPages error

`getACMCertificateSummaries` did not check `ListCertificatesPages` error and returned empty `CertificateSummary` slice and no error via `filterCertificatesByTag` if `filterTag` is configured. Empty `CertificateSummary` results in empty `existingStackCertificateARNs` for each `loadBalancer` model which triggered stack update: ``` level=info msg="Updating \"internet-facing\" stack for 0 certificates / 0 ingresses" ``` and then stack deletion on the next cycle: ``` level=info msg="Deleted orphaned stack \"kube-ingress-aws-controller-aws-XXX\"" ``` due to empty certificate tags after previous update. Follow up on #658 Signed-off-by: Alexander Yastrebov <[email protected]>
zalando-incubator · Oct 15, 2024 · 3ea0e2c · 3ea0e2c
1 parent 9d1db4c
commit 3ea0e2c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 3 deletions.
diff --git a/aws/acm.go b/aws/acm.go
@@ -44,16 +44,18 @@ func getACMCertificateSummaries(api acmiface.ACMAPI, filterTag string) ([]*acm.C
 	}
 	acmSummaries := make([]*acm.CertificateSummary, 0)
 
-	err := api.ListCertificatesPages(params, func(page *acm.ListCertificatesOutput, lastPage bool) bool {
+	if err := api.ListCertificatesPages(params, func(page *acm.ListCertificatesOutput, lastPage bool) bool {
 		acmSummaries = append(acmSummaries, page.CertificateSummaryList...)
 		return true
-	})
+	}); err != nil {
+		return nil, err
+	}
 
 	if tag := strings.Split(filterTag, "="); filterTag != "=" && len(tag) == 2 {
 		return filterCertificatesByTag(api, acmSummaries, tag[0], tag[1])
 	}
 
-	return acmSummaries, err
+	return acmSummaries, nil
 }
 
 func filterCertificatesByTag(api acmiface.ACMAPI, allSummaries []*acm.CertificateSummary, key, value string) ([]*acm.CertificateSummary, error) {

diff --git a/aws/acm_test.go b/aws/acm_test.go
@@ -1,6 +1,7 @@
 package aws
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -142,6 +143,18 @@ func TestACM(t *testing.T) {
 				DomainNames: []string{"foobar.de"},
 			},
 		},
+		{
+			msg: "Fail on ListCertificatesPages error",
+			api: fake.NewACMClient(
+				acm.ListCertificatesOutput{}, nil, nil,
+			).WithListCertificatesPages(func(input *acm.ListCertificatesInput, fn func(p *acm.ListCertificatesOutput, lastPage bool) (shouldContinue bool)) error {
+				return fmt.Errorf("ListCertificatesPages error")
+			}),
+			filterTag: "production=true",
+			expect: acmExpect{
+				Error: "ListCertificatesPages error",
+			},
+		},
 	} {
 		t.Run(ti.msg, func(t *testing.T) {
 			provider := newACMCertProvider(ti.api, ti.filterTag)