From 15d497b4ca6ec9b3b6390d12fdb3c57e09c57dab Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 19 Jul 2018 09:46:36 -0700 Subject: [PATCH 1/3] issuer/route53: fix delete for 'NotExist' errors Fixes #736. Prior to this change, it was quite possible to end up with a queue of cleanup tasks that would never succeed. --- pkg/issuer/acme/dns/route53/route53.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/issuer/acme/dns/route53/route53.go b/pkg/issuer/acme/dns/route53/route53.go index c17ad779f..e22d2c038 100644 --- a/pkg/issuer/acme/dns/route53/route53.go +++ b/pkg/issuer/acme/dns/route53/route53.go @@ -9,6 +9,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/client" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/request" @@ -113,14 +114,14 @@ func (*DNSProvider) Timeout() (timeout, interval time.Duration) { func (r *DNSProvider) Present(domain, token, keyAuth string) error { fqdn, value, _ := util.DNS01Record(domain, keyAuth) value = `"` + value + `"` - return r.changeRecord("UPSERT", fqdn, value, route53TTL) + return r.changeRecord(route53.ChangeActionUpsert, fqdn, value, route53TTL) } // CleanUp removes the TXT record matching the specified parameters func (r *DNSProvider) CleanUp(domain, token, keyAuth string) error { fqdn, value, _ := util.DNS01Record(domain, keyAuth) value = `"` + value + `"` - return r.changeRecord("DELETE", fqdn, value, route53TTL) + return r.changeRecord(route53.ChangeActionDelete, fqdn, value, route53TTL) } func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error { @@ -136,7 +137,7 @@ func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error { Comment: aws.String("Managed by cert-manager"), Changes: []*route53.Change{ { - Action: aws.String(action), + Action: &action, ResourceRecordSet: recordSet, }, }, @@ -145,7 +146,15 @@ func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error { resp, err := r.client.ChangeResourceRecordSets(reqParams) if err != nil { + if awserr, ok := err.(awserr.Error); ok { + if action == route53.ChangeActionDelete && awserr.Code() == route53.ErrCodeInvalidChangeBatch { + // If we try to delete something and get a 'InvalidChangeBatch' that + // means it's already deleted, no need to consider it an error. + return nil + } + } return fmt.Errorf("Failed to change Route 53 record set: %v", err) + } statusID := resp.ChangeInfo.Id @@ -207,7 +216,7 @@ func (r *DNSProvider) getHostedZoneID(fqdn string) (string, error) { func newTXTRecordSet(fqdn, value string, ttl int) *route53.ResourceRecordSet { return &route53.ResourceRecordSet{ Name: aws.String(fqdn), - Type: aws.String("TXT"), + Type: aws.String(route53.RRTypeTxt), TTL: aws.Int64(int64(ttl)), ResourceRecords: []*route53.ResourceRecord{ {Value: aws.String(value)}, From efb339bac559960e5962c0c5bd894ee4cc626f47 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 19 Jul 2018 11:24:12 -0700 Subject: [PATCH 2/3] Gopkg: fix inputs digest --- Gopkg.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gopkg.lock b/Gopkg.lock index 47e7452ee..62e636dca 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -931,6 +931,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "d792bbf24d87653ea8f00046b922dca780a50fdd60e8ea9540c9f34c9a28e675" + inputs-digest = "06281eceaf33428082bf511ba2b6f50c05b0211704689672b1a74c8f1d615a04" solver-name = "gps-cdcl" solver-version = 1 From ea84532a5cc546006690cdbaeb04f8a0fa3a25fd Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Fri, 20 Jul 2018 10:09:21 -0700 Subject: [PATCH 3/3] issuer/route53: log ignored InvalidChangeBatch err --- pkg/issuer/acme/dns/route53/route53.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/issuer/acme/dns/route53/route53.go b/pkg/issuer/acme/dns/route53/route53.go index e22d2c038..707ce82f1 100644 --- a/pkg/issuer/acme/dns/route53/route53.go +++ b/pkg/issuer/acme/dns/route53/route53.go @@ -148,6 +148,7 @@ func (r *DNSProvider) changeRecord(action, fqdn, value string, ttl int) error { if err != nil { if awserr, ok := err.(awserr.Error); ok { if action == route53.ChangeActionDelete && awserr.Code() == route53.ErrCodeInvalidChangeBatch { + glog.V(5).Infof("ignoring InvalidChangeBatch error: %v", err) // If we try to delete something and get a 'InvalidChangeBatch' that // means it's already deleted, no need to consider it an error. return nil