From 65db84ed382abca622bef1d0371385accbfa6a5b Mon Sep 17 00:00:00 2001 From: Stoyan Rachev Date: Fri, 27 Aug 2021 17:02:49 +0300 Subject: [PATCH] Categorize certian aws-route53 errors as configuration problems --- pkg/aws/client/client_route53.go | 17 ++++++++ pkg/controller/dnsrecord/actuator.go | 7 +++- pkg/controller/dnsrecord/actuator_test.go | 49 ++++++++++++++++++++++- 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/pkg/aws/client/client_route53.go b/pkg/aws/client/client_route53.go index acb62ac97..15eb5846e 100644 --- a/pkg/aws/client/client_route53.go +++ b/pkg/aws/client/client_route53.go @@ -17,6 +17,7 @@ package client import ( "context" "fmt" + "regexp" "strconv" "strings" "time" @@ -315,3 +316,19 @@ func isValuesDoNotMatchError(err error) bool { } return false } + +func IsNoSuchHostedZoneError(err error) bool { + if aerr, ok := err.(awserr.Error); ok && aerr.Code() == route53.ErrCodeNoSuchHostedZone { + return true + } + return false +} + +var notPermittedInZoneRegex = regexp.MustCompile(`RRSet with DNS name [^\ ]+ is not permitted in zone [^\ ]+`) + +func IsNotPermittedInZoneError(err error) bool { + if aerr, ok := err.(awserr.Error); ok && aerr.Code() == route53.ErrCodeInvalidChangeBatch && notPermittedInZoneRegex.MatchString(aerr.Message()) { + return true + } + return false +} diff --git a/pkg/controller/dnsrecord/actuator.go b/pkg/controller/dnsrecord/actuator.go index 9ae358539..2694f0af4 100644 --- a/pkg/controller/dnsrecord/actuator.go +++ b/pkg/controller/dnsrecord/actuator.go @@ -23,6 +23,7 @@ import ( "github.com/gardener/gardener/extensions/pkg/controller/dnsrecord" controllererror "github.com/gardener/gardener/extensions/pkg/controller/error" gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" extensionsv1alpha1helper "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1/helper" kutil "github.com/gardener/gardener/pkg/utils/kubernetes" @@ -81,8 +82,12 @@ func (a *actuator) Reconcile(ctx context.Context, dns *extensionsv1alpha1.DNSRec ttl := extensionsv1alpha1helper.GetDNSRecordTTL(dns.Spec.TTL) a.logger.Info("Creating or updating DNS recordset", "zone", zone, "name", dns.Spec.Name, "type", dns.Spec.RecordType, "values", dns.Spec.Values, "dnsrecord", kutil.ObjectName(dns)) if err := awsClient.CreateOrUpdateDNSRecordSet(ctx, zone, dns.Spec.Name, string(dns.Spec.RecordType), dns.Spec.Values, ttl); err != nil { + cause := fmt.Errorf("could not create or update DNS recordset in zone %s with name %s, type %s, and values %v: %+v", zone, dns.Spec.Name, dns.Spec.RecordType, dns.Spec.Values, err) + if awsclient.IsNoSuchHostedZoneError(err) || awsclient.IsNotPermittedInZoneError(err) { + cause = gardencorev1beta1helper.NewErrorWithCodes(cause.Error(), gardencorev1beta1.ErrorConfigurationProblem) + } return &controllererror.RequeueAfterError{ - Cause: fmt.Errorf("could not create or update DNS recordset in zone %s with name %s, type %s, and values %v: %+v", zone, dns.Spec.Name, dns.Spec.RecordType, dns.Spec.Values, err), + Cause: cause, RequeueAfter: requeueAfterOnProviderError, } } diff --git a/pkg/controller/dnsrecord/actuator_test.go b/pkg/controller/dnsrecord/actuator_test.go index 7da1121fe..b16744622 100644 --- a/pkg/controller/dnsrecord/actuator_test.go +++ b/pkg/controller/dnsrecord/actuator_test.go @@ -16,12 +16,18 @@ package dnsrecord_test import ( "context" + "errors" "github.com/gardener/gardener-extension-provider-aws/pkg/aws" mockawsclient "github.com/gardener/gardener-extension-provider-aws/pkg/aws/client/mock" . "github.com/gardener/gardener-extension-provider-aws/pkg/controller/dnsrecord" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/route53" "github.com/gardener/gardener/extensions/pkg/controller/dnsrecord" + controllererror "github.com/gardener/gardener/extensions/pkg/controller/error" + gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client" kutil "github.com/gardener/gardener/pkg/utils/kubernetes" @@ -124,7 +130,7 @@ var _ = Describe("Actuator", func() { }) Describe("#Reconcile", func() { - It("should reconcile the DNSRecord", func() { + BeforeEach(func() { c.EXPECT().Get(ctx, kutil.Key(namespace, name), gomock.AssignableToTypeOf(&corev1.Secret{})).DoAndReturn( func(_ context.Context, _ client.ObjectKey, obj *corev1.Secret) error { *obj = *secret @@ -132,6 +138,9 @@ var _ = Describe("Actuator", func() { }, ) awsClientFactory.EXPECT().NewClient(accessKeyID, secretAccessKey, aws.DefaultDNSRegion).Return(awsClient, nil) + }) + + It("should reconcile the DNSRecord", func() { awsClient.EXPECT().GetDNSHostedZones(ctx).Return(zones, nil) awsClient.EXPECT().CreateOrUpdateDNSRecordSet(ctx, zone, domainName, string(extensionsv1alpha1.DNSRecordTypeA), []string{address}, int64(120)).Return(nil) awsClient.EXPECT().DeleteDNSRecordSet(ctx, zone, "comment-"+domainName, "TXT", nil, int64(0)).Return(nil) @@ -153,6 +162,44 @@ var _ = Describe("Actuator", func() { err := a.Reconcile(ctx, dns, nil) Expect(err).NotTo(HaveOccurred()) }) + + It("should fail if creating the DNS record set failed", func() { + dns.Spec.Zone = pointer.String(zone) + + awsClient.EXPECT().CreateOrUpdateDNSRecordSet(ctx, zone, domainName, string(extensionsv1alpha1.DNSRecordTypeA), []string{address}, int64(120)). + Return(errors.New("test")) + + err := a.Reconcile(ctx, dns, nil) + Expect(err).To(HaveOccurred()) + _, ok := err.(*controllererror.RequeueAfterError).Cause.(gardencorev1beta1helper.Coder) + Expect(ok).To(BeFalse()) + }) + + It("should fail with ERR_CONFIGURATION_PROBLEM if there is no such hosted zone", func() { + dns.Spec.Zone = pointer.String(zone) + + awsClient.EXPECT().CreateOrUpdateDNSRecordSet(ctx, zone, domainName, string(extensionsv1alpha1.DNSRecordTypeA), []string{address}, int64(120)). + Return(awserr.New(route53.ErrCodeNoSuchHostedZone, "", nil)) + + err := a.Reconcile(ctx, dns, nil) + Expect(err).To(HaveOccurred()) + coder, ok := err.(*controllererror.RequeueAfterError).Cause.(gardencorev1beta1helper.Coder) + Expect(ok).To(BeTrue()) + Expect(coder.Codes()).To(Equal([]gardencorev1beta1.ErrorCode{gardencorev1beta1.ErrorConfigurationProblem})) + }) + + It("should fail with ERR_CONFIGURATION_PROBLEM if the domain name is not permitted in the zone", func() { + dns.Spec.Zone = pointer.String(zone) + + awsClient.EXPECT().CreateOrUpdateDNSRecordSet(ctx, zone, domainName, string(extensionsv1alpha1.DNSRecordTypeA), []string{address}, int64(120)). + Return(awserr.New(route53.ErrCodeInvalidChangeBatch, "RRSet with DNS name api.aws.foobar.shoot.example.com. is not permitted in zone foo.com.", nil)) + + err := a.Reconcile(ctx, dns, nil) + Expect(err).To(HaveOccurred()) + coder, ok := err.(*controllererror.RequeueAfterError).Cause.(gardencorev1beta1helper.Coder) + Expect(ok).To(BeTrue()) + Expect(coder.Codes()).To(Equal([]gardencorev1beta1.ErrorCode{gardencorev1beta1.ErrorConfigurationProblem})) + }) }) Describe("#Delete", func() {