Skip to content

Commit

Permalink
Categorize certain aws-route53 errors as configuration problems (#398)
Browse files Browse the repository at this point in the history
* Categorize certian aws-route53 errors as configuration problems

* Address code review comments
  • Loading branch information
stoyanr authored Aug 31, 2021
1 parent 7514078 commit ee1f49c
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 2 deletions.
19 changes: 19 additions & 0 deletions pkg/aws/client/client_route53.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package client
import (
"context"
"fmt"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -315,3 +316,21 @@ func isValuesDoNotMatchError(err error) bool {
}
return false
}

// IsNoSuchHostedZoneError returns true if the error indicates a non-existing route53 hosted zone.
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 [^\ ]+`)

// IsNotPermittedInZoneError returns true if the error indicates that the DNS name is not permitted in the route53 hosted 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
}
7 changes: 6 additions & 1 deletion pkg/controller/dnsrecord/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
}
}
Expand Down
49 changes: 48 additions & 1 deletion pkg/controller/dnsrecord/actuator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -124,14 +130,17 @@ 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
return nil
},
)
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)
Expand All @@ -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() {
Expand Down

0 comments on commit ee1f49c

Please sign in to comment.