Fix finding NS record of the DNS zone (#157)

giantswarm · Feb 7, 2024 · eba8338 · eba8338
1 parent bb28fe5
commit eba8338
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 52 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix finding NS record of the DNS zone. This only happens in rare cases.
+
 ## [0.14.4] - 2024-01-31
 
 ### Changed

diff --git a/pkg/aws/route53.go b/pkg/aws/route53.go
@@ -193,8 +193,8 @@ func (r *Route53) DeleteHostedZone(ctx context.Context, logger logr.Logger, zone
 	return nil
 }
 
-func (r *Route53) GetHostedZoneNSRecords(ctx context.Context, logger logr.Logger, zoneId string) (*resolver.DNSRecord, error) {
-	logger = logger.WithValues("zoneId", zoneId)
+func (r *Route53) GetHostedZoneNSRecord(ctx context.Context, logger logr.Logger, zoneId string, zoneName string) (*resolver.DNSRecord, error) {
+	logger = logger.WithValues("zoneId", zoneId, "zoneName", zoneName)
 
 	var resourceRecordSet *route53.ResourceRecordSet
 
@@ -206,19 +206,25 @@ func (r *Route53) GetHostedZoneNSRecords(ctx context.Context, logger logr.Logger
 		logger.Info("Requesting NS record")
 
 		listResourceRecordSetsOutput, err := r.client.ListResourceRecordSetsWithContext(ctx, &route53.ListResourceRecordSetsInput{
-			HostedZoneId: awssdk.String(zoneId),
-			MaxItems:     awssdk.String("1"), // First entry is always NS record
+			HostedZoneId:    awssdk.String(zoneId),
+			MaxItems:        awssdk.String("1"),
+			StartRecordType: awssdk.String(route53.RRTypeNs),
+
+			// `StartRecordType` must be specified together with `StartRecordName`, so we specify the zone domain
+			StartRecordName: awssdk.String(zoneName),
 		})
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
 
-		// Ensure the above assumptions for request behavior hold
 		if len(listResourceRecordSetsOutput.ResourceRecordSets) != 1 {
-			return nil, errors.New("logic error - did not receive exactly one resource record")
+			return nil, errors.New("logic error - did not receive exactly one resource record set")
 		}
 		if *listResourceRecordSetsOutput.ResourceRecordSets[0].Type != route53.RRTypeNs {
-			return nil, errors.New("logic error - did not receive a resource record set of type NS")
+			return nil, errors.Errorf(
+				"logic error - did not receive a resource record set of type NS (got name %q and type %q)",
+				*listResourceRecordSetsOutput.ResourceRecordSets[0].Name,
+				*listResourceRecordSetsOutput.ResourceRecordSets[0].Type)
 		}
 		if len(listResourceRecordSetsOutput.ResourceRecordSets[0].ResourceRecords) == 0 {
 			return nil, errors.New("did not receive any NS resource record")

diff --git a/pkg/resolver/package.go b/pkg/resolver/package.go
@@ -44,7 +44,7 @@ type Route53Client interface {
 	CreateHostedZone(ctx context.Context, logger logr.Logger, dnsZone DnsZone) (string, error)
 	DeleteHostedZone(ctx context.Context, logger logr.Logger, zoneId string) error
 	GetHostedZoneIdByName(ctx context.Context, logger logr.Logger, zoneName string) (string, error)
-	GetHostedZoneNSRecords(ctx context.Context, logger logr.Logger, zoneId string) (*DNSRecord, error)
+	GetHostedZoneNSRecord(ctx context.Context, logger logr.Logger, zoneId string, zoneName string) (*DNSRecord, error)
 	AddDelegationToParentZone(ctx context.Context, logger logr.Logger, parentZoneId string, resourceRecord *DNSRecord) error
 	DeleteDelegationFromParentZone(ctx context.Context, logger logr.Logger, parentZoneId string, resourceRecord *DNSRecord) error
 	AddDnsRecordsToHostedZone(ctx context.Context, logger logr.Logger, hostedZoneId string, dnsRecords []DNSRecord) error

diff --git a/pkg/resolver/resolverfakes/fake_route53client.go b/pkg/resolver/resolverfakes/fake_route53client.go
diff --git a/pkg/resolver/zoner.go b/pkg/resolver/zoner.go
@@ -90,7 +90,7 @@ func (d *Zoner) CreateHostedZone(ctx context.Context, logger logr.Logger, cluste
 			return errors.WithStack(err)
 		}
 
-		nsRecord, err := route53Client.GetHostedZoneNSRecords(ctx, logger, hostedZoneId)
+		nsRecord, err := route53Client.GetHostedZoneNSRecord(ctx, logger, hostedZoneId, dnsZoneToCreate.DnsName)
 		if err != nil {
 			return errors.WithStack(err)
 		}
@@ -132,7 +132,7 @@ func (d *Zoner) DeleteHostedZone(ctx context.Context, logger logr.Logger, cluste
 			return errors.WithStack(err)
 		}
 
-		nsRecord, err := route53Client.GetHostedZoneNSRecords(ctx, logger, hostedZoneId)
+		nsRecord, err := route53Client.GetHostedZoneNSRecord(ctx, logger, hostedZoneId, hostedZoneName)
 		if err != nil {
 			return errors.WithStack(err)
 		}