From ceb6e2d0829ae9a4c4d7ba1ccc23069c284d3a06 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Wed, 8 Jan 2025 15:02:02 -0800 Subject: [PATCH 1/5] Only create one delegations set --- src/pkg/cli/cert.go | 2 +- src/pkg/cli/client/byoc/aws/byoc.go | 116 ++++---- .../client/byoc/aws/byoc_integration_test.go | 15 + src/pkg/cli/client/byoc/aws/byoc_test.go | 265 ++++++++++++++++++ src/pkg/clouds/aws/route53.go | 42 ++- src/pkg/clouds/aws/route53_test.go | 22 +- src/pkg/dns/check.go | 8 +- 7 files changed, 401 insertions(+), 69 deletions(-) diff --git a/src/pkg/cli/cert.go b/src/pkg/cli/cert.go index a543beb3f..9c9a66bdf 100644 --- a/src/pkg/cli/cert.go +++ b/src/pkg/cli/cert.go @@ -188,7 +188,7 @@ func waitForTLS(ctx context.Context, domain string) error { func waitForCNAME(ctx context.Context, domain string, targets []string, client client.FabricClient) error { for i, target := range targets { - targets[i] = strings.TrimSuffix(strings.ToLower(target), ".") + targets[i] = dns.Normalize(strings.ToLower(target)) } ticker := time.NewTicker(5 * time.Second) diff --git a/src/pkg/cli/client/byoc/aws/byoc.go b/src/pkg/cli/client/byoc/aws/byoc.go index 0bafd1274..e894079c2 100644 --- a/src/pkg/cli/client/byoc/aws/byoc.go +++ b/src/pkg/cli/client/byoc/aws/byoc.go @@ -22,6 +22,7 @@ import ( "github.com/DefangLabs/defang/src/pkg/clouds/aws" "github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs" "github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs/cfn" + "github.com/DefangLabs/defang/src/pkg/dns" "github.com/DefangLabs/defang/src/pkg/http" "github.com/DefangLabs/defang/src/pkg/logs" "github.com/DefangLabs/defang/src/pkg/term" @@ -304,8 +305,7 @@ func (b *ByocAws) findZone(ctx context.Context, domain, roleARN string) (string, r53Client := route53.NewFromConfig(cfg) - domain = strings.TrimSuffix(domain, ".") - domain = strings.ToLower(domain) + domain = dns.Normalize(strings.ToLower(domain)) for { zone, err := aws.GetHostedZoneByName(ctx, domain, r53Client) if errors.Is(err, aws.ErrZoneNotFound) { @@ -322,81 +322,99 @@ func (b *ByocAws) findZone(ctx context.Context, domain, roleARN string) (string, } func (b *ByocAws) PrepareDomainDelegation(ctx context.Context, req client.PrepareDomainDelegationRequest) (*client.PrepareDomainDelegationResponse, error) { - projectDomain := b.GetProjectDomain(req.Project, req.DelegateDomain) - cfg, err := b.driver.LoadConfig(ctx) if err != nil { return nil, AnnotateAwsError(err) } r53Client := route53.NewFromConfig(cfg) + projectDomain := b.GetProjectDomain(req.Project, req.DelegateDomain) + nsServers, delegationSetId, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + return nil, AnnotateAwsError(err) + } + resp := client.PrepareDomainDelegationResponse{ + NameServers: nsServers, + DelegationSetId: delegationSetId, + } + return &resp, nil +} + +func prepareDomainDelegation(ctx context.Context, projectDomain string, r53Client aws.Route53API) (nsServers []string, delegationSetId string, err error) { // There's four cases to consider: - // 1. The subdomain zone does not exist: we get NS records from the delegation set and let CD/Pulumi create the hosted zone + // 1. The subdomain zone does not exist: we create/get a delegation set and get its NS records and let CD/Pulumi create the hosted zone // 2. The subdomain zone exists: - // a. The zone was created by the older CLI: we need to get the NS records from the existing zone - // b. The zone was created by the new CD/Pulumi: we get the NS records from the delegation set and let CD/Pulumi create the hosted zone - // c. The zone was created another way: the deployment will likely fail with a "zone already exists" error + // a. The zone was created by the older CLI: we need to get the NS records from the existing zone and pass to Fabric; no delegation set + // b. The zone was created by the new CD/Pulumi: we get the NS records from the delegation set and let CD/Pulumi create/update the hosted zone + // c. The zone was created another way: get the NS records from the existing zone and pass to Fabric; no delegation set - var nsServers []string + var delegationSet *r53types.DelegationSet zone, err := aws.GetHostedZoneByName(ctx, projectDomain, r53Client) if err != nil { + // The only acceptable error is that the zone was not found if !errors.Is(err, aws.ErrZoneNotFound) { - return nil, AnnotateAwsError(err) // TODO: we should not fail deployment if this fails + return nil, "", err // TODO: we should not fail deployment if this fails } term.Debugf("Zone %q not found, delegation set will be created", projectDomain) - // Case 1: The zone doesn't exist: we'll create a delegation set and let CD/Pulumi create the hosted zone + + // Case 1: The zone doesn't exist: we'll create/get a delegation set and let CD/Pulumi create the hosted zone + + // Avoid creating a new delegation set if one already exists + delegationSet, err = aws.GetDelegationSet(ctx, r53Client) + // Create a new delegation set if it doesn't exist + if errors.Is(err, aws.ErrNoDelegationSetFound) { + // Create a new delegation set. There's a race condition here, where two deployments could create two different delegation sets, + // but this is acceptable because the next time the zone is deployed, we'll get the existing delegation set from the zone. + delegationSet, err = aws.CreateDelegationSet(ctx, nil, r53Client) + } + if err != nil { + return nil, "", err + } } else { // Case 2: Get the NS records for the existing subdomain zone nsServers, err = aws.ListResourceRecords(ctx, *zone.Id, projectDomain, r53types.RRTypeNs, r53Client) if err != nil { - return nil, AnnotateAwsError(err) // TODO: we should not fail deployment if this fails + return nil, "", err // TODO: we should not fail deployment if this fails } term.Debugf("Zone %q found, NS records: %v", projectDomain, nsServers) - } - var resp client.PrepareDomainDelegationResponse - if zone == nil || zone.Config.Comment == nil || *zone.Config.Comment != aws.CreateHostedZoneComment { - // Case 2b or 2c: The zone does not exist, or was not created by an older version of this CLI. - // Get the NS records for the delegation set (using the existing zone) and let Pulumi create the hosted zone for us - var zoneId *string - if zone != nil { - zoneId = zone.Id + // Check if the zone was created by the older CLI (before the delegation set was introduced) + if zone.Config.Comment != nil && *zone.Config.Comment == aws.CreateHostedZoneCommentLegacy { + // Case 2a: The zone was created by the older CLI, we'll use the existing NS records; track how many times this happens + track.Evt("Compose-Up delegateSubdomain old", track.P("domain", projectDomain)) + return nsServers, "", nil } - // TODO: avoid creating the delegation set if we're in preview mode - delegationSet, err := aws.CreateDelegationSet(ctx, zoneId, r53Client) - var delegationSetAlreadyCreated *r53types.DelegationSetAlreadyCreated - var delegationSetAlreadyReusable *r53types.DelegationSetAlreadyReusable - if errors.As(err, &delegationSetAlreadyCreated) || errors.As(err, &delegationSetAlreadyReusable) { + + // Case 2b or 2c: The zone was not created by an older version of this CLI. We'll get the delegation set and let CD/Pulumi create/update the hosted zone + // TODO: we need to detect the case 2c where the zone was created by another tool and we need to use the existing NS records + + // Create a reusable delegation set for the existing subdomain zone + delegationSet, err = aws.CreateDelegationSet(ctx, zone.Id, r53Client) + if delegationSetAlreadyReusable := new(r53types.DelegationSetAlreadyReusable); errors.As(err, &delegationSetAlreadyReusable) { term.Debug("Route53 delegation set already created:", err) - delegationSet, err = aws.GetDelegationSet(ctx, r53Client) + delegationSet, err = aws.GetDelegationSetByZone(ctx, zone.Id, r53Client) } if err != nil { - return nil, AnnotateAwsError(err) - } - if len(delegationSet.NameServers) == 0 { - return nil, errors.New("no NS records found for the delegation set") // should not happen - } - term.Debug("Route53 delegation set ID:", *delegationSet.Id) - resp.DelegationSetId = strings.TrimPrefix(*delegationSet.Id, "/delegationset/") - - // Ensure the NS records match the ones from the delegation set if the zone already exists - if zoneId != nil { - sort.Strings(nsServers) - sort.Strings(delegationSet.NameServers) - if !slices.Equal(delegationSet.NameServers, nsServers) { - track.Evt("Compose-Up delegateSubdomain diff", track.P("fromDS", delegationSet.NameServers), track.P("fromZone", nsServers)) - term.Debugf("NS records for the existing subdomain zone do not match the delegation set: %v <> %v", delegationSet.NameServers, nsServers) - } + return nil, "", err } + } - nsServers = delegationSet.NameServers - } else { - // Case 2a: The zone was created by the older CLI, we'll use the existing NS records; track how many times this happens - track.Evt("Compose-Up delegateSubdomain old", track.P("domain", projectDomain)) + if len(delegationSet.NameServers) == 0 { + return nil, "", errors.New("no NS records found for the delegation set") // should not happen } - resp.NameServers = nsServers + term.Debug("Route53 delegation set ID:", *delegationSet.Id) + delegationSetId = strings.TrimPrefix(*delegationSet.Id, "/delegationset/") - return &resp, nil + // Ensure the NS records match the ones from the delegation set if the zone already exists + sort.Strings(nsServers) + sort.Strings(delegationSet.NameServers) + if !slices.Equal(delegationSet.NameServers, nsServers) { + track.Evt("Compose-Up delegateSubdomain diff", track.P("fromDS", delegationSet.NameServers), track.P("fromZone", nsServers)) + term.Debugf("NS records for the existing subdomain zone do not match the delegation set: %v <> %v", delegationSet.NameServers, nsServers) + // FIXME: this occurred 4 times + } + + return delegationSet.NameServers, delegationSetId, nil } func (b *ByocAws) AccountInfo(ctx context.Context) (client.AccountInfo, error) { @@ -847,7 +865,7 @@ func (b *ByocAws) update(ctx context.Context, projectName, delegateDomain string } // Do a DNS lookup for DomainName and confirm it's indeed a CNAME to the service's public FQDN cname, _ := net.LookupCNAME(service.DomainName) - if strings.TrimSuffix(cname, ".") != si.PublicFqdn { + if dns.Normalize(cname) != si.PublicFqdn { dnsRole, _ := service.Extensions["x-defang-dns-role"].(string) zoneId, err := b.findZone(ctx, service.DomainName, dnsRole) if err != nil { diff --git a/src/pkg/cli/client/byoc/aws/byoc_integration_test.go b/src/pkg/cli/client/byoc/aws/byoc_integration_test.go index 8769ad317..16270441a 100644 --- a/src/pkg/cli/client/byoc/aws/byoc_integration_test.go +++ b/src/pkg/cli/client/byoc/aws/byoc_integration_test.go @@ -8,6 +8,8 @@ import ( "testing" defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/route53" "github.com/bufbuild/connect-go" ) @@ -150,3 +152,16 @@ func TestListSecrets(t *testing.T) { } }) } + +func TestPrepareDomainDelegation(t *testing.T) { + ctx := context.Background() + cfg, err := config.LoadDefaultConfig(ctx) + if err != nil { + t.Fatal(err) + } + + r53Client := route53.NewFromConfig(cfg) + + testPrepareDomainDelegationNew(t, r53Client) + testPrepareDomainDelegationLegacy(t, r53Client) +} diff --git a/src/pkg/cli/client/byoc/aws/byoc_test.go b/src/pkg/cli/client/byoc/aws/byoc_test.go index 429b4597a..17878c8fa 100644 --- a/src/pkg/cli/client/byoc/aws/byoc_test.go +++ b/src/pkg/cli/client/byoc/aws/byoc_test.go @@ -6,18 +6,24 @@ import ( "context" "embed" "encoding/json" + "errors" "io" "path" + "slices" "strings" "sync" "testing" + "github.com/DefangLabs/defang/src/pkg" "github.com/DefangLabs/defang/src/pkg/cli/client/byoc" "github.com/DefangLabs/defang/src/pkg/clouds/aws" "github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs" "github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs/cfn" "github.com/DefangLabs/defang/src/pkg/types" defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" + "github.com/aws/aws-sdk-go-v2/service/route53" + r53types "github.com/aws/aws-sdk-go-v2/service/route53/types" + "github.com/aws/smithy-go/ptr" composeTypes "github.com/compose-spec/compose-go/v2/types" ) @@ -167,3 +173,262 @@ func TestSubscribe(t *testing.T) { }) } } + +type r53HostedZone struct { + r53types.HostedZone + r53types.DelegationSet // no ID => not reusable +} + +type route53API interface { + aws.Route53API + DeleteHostedZone(ctx context.Context, params *route53.DeleteHostedZoneInput, optFns ...func(*route53.Options)) (*route53.DeleteHostedZoneOutput, error) +} + +type r53Mock struct { + hostedZones []r53HostedZone + delegationSets []r53types.DelegationSet +} + +func (r r53Mock) ListHostedZonesByName(ctx context.Context, params *route53.ListHostedZonesByNameInput, optFns ...func(*route53.Options)) (*route53.ListHostedZonesByNameOutput, error) { + var hostedZones []r53types.HostedZone + for _, hz := range r.hostedZones { + if params.DNSName != nil && *hz.Name < *params.DNSName { // assume ASCII order + continue + } + hostedZones = append(hostedZones, hz.HostedZone) + if params.MaxItems != nil && len(hostedZones) >= int(*params.MaxItems) { + break + } + } + return &route53.ListHostedZonesByNameOutput{ + HostedZones: hostedZones, + DNSName: params.DNSName, + MaxItems: params.MaxItems, + HostedZoneId: params.HostedZoneId, + }, nil +} + +func (r r53Mock) ListResourceRecordSets(ctx context.Context, params *route53.ListResourceRecordSetsInput, optFns ...func(*route53.Options)) (*route53.ListResourceRecordSetsOutput, error) { + for _, hz := range r.hostedZones { + if *hz.HostedZone.Id != *params.HostedZoneId { + continue + } + var recordSets []r53types.ResourceRecord + if params.StartRecordType == r53types.RRTypeNs { + // Copy the NS records from the hosted zone + for _, ns := range hz.NameServers { + recordSets = append(recordSets, r53types.ResourceRecord{Value: ptr.String(ns)}) + } + } + return &route53.ListResourceRecordSetsOutput{ + MaxItems: params.MaxItems, + ResourceRecordSets: []r53types.ResourceRecordSet{ + { + Name: ptr.String(*hz.Name), + Type: params.StartRecordType, + ResourceRecords: recordSets, + }, + }, + }, nil + } + return nil, errors.New("hosted zone not found") +} + +func (r *r53Mock) CreateReusableDelegationSet(ctx context.Context, params *route53.CreateReusableDelegationSetInput, optFns ...func(*route53.Options)) (*route53.CreateReusableDelegationSetOutput, error) { + for _, ds := range r.delegationSets { + if *ds.CallerReference == *params.CallerReference { + return nil, &r53types.DelegationSetAlreadyCreated{} + } + } + var delegationSet *r53types.DelegationSet + if params.HostedZoneId != nil { + for _, hz := range r.hostedZones { + if strings.HasSuffix(*hz.HostedZone.Id, *params.HostedZoneId) { + delegationSet = &hz.DelegationSet + break + } + } + if delegationSet == nil { + return nil, &r53types.NoSuchHostedZone{} + } + if delegationSet.Id != nil { + return nil, &r53types.DelegationSetAlreadyReusable{} + } + delegationSet.Id = ptr.String("/delegationset/N" + strings.ToUpper(pkg.RandomID())) + delegationSet.CallerReference = params.CallerReference + } else { + delegationSet = &r53types.DelegationSet{ + CallerReference: params.CallerReference, + Id: ptr.String("/delegationset/N" + strings.ToUpper(pkg.RandomID())), + NameServers: []string{r.randNameServer(), r.randNameServer()}, + } + } + r.delegationSets = append(r.delegationSets, *delegationSet) + return &route53.CreateReusableDelegationSetOutput{ + DelegationSet: delegationSet, + Location: ptr.String("https://route53.amazonaws.com/2013-04-01" + *delegationSet.Id), + }, nil +} + +func (r r53Mock) ListReusableDelegationSets(ctx context.Context, params *route53.ListReusableDelegationSetsInput, optFns ...func(*route53.Options)) (*route53.ListReusableDelegationSetsOutput, error) { + return &route53.ListReusableDelegationSetsOutput{ + DelegationSets: r.delegationSets, + Marker: params.Marker, + MaxItems: params.MaxItems, + }, nil +} + +func (r53Mock) randNameServer() string { + return pkg.RandomID() + ".example.com" +} + +func (r r53Mock) GetHostedZone(ctx context.Context, params *route53.GetHostedZoneInput, optFns ...func(*route53.Options)) (*route53.GetHostedZoneOutput, error) { + for _, hz := range r.hostedZones { + if strings.HasSuffix(*hz.HostedZone.Id, *params.Id) { + return &route53.GetHostedZoneOutput{ + HostedZone: &hz.HostedZone, + DelegationSet: &hz.DelegationSet, + }, nil + } + } + return nil, &r53types.NoSuchHostedZone{} +} + +func (r r53Mock) DeleteHostedZone(ctx context.Context, params *route53.DeleteHostedZoneInput, optFns ...func(*route53.Options)) (*route53.DeleteHostedZoneOutput, error) { + return &route53.DeleteHostedZoneOutput{}, nil +} + +func (r *r53Mock) CreateHostedZone(ctx context.Context, params *route53.CreateHostedZoneInput, optFns ...func(*route53.Options)) (*route53.CreateHostedZoneOutput, error) { + hostedZone := r53types.HostedZone{ + Id: ptr.String("/hostedzone/Z" + strings.ToUpper(pkg.RandomID())), + CallerReference: params.CallerReference, + Config: params.HostedZoneConfig, + Name: params.Name, + } + var delegationSet *r53types.DelegationSet + for _, ds := range r.delegationSets { + if strings.HasSuffix(*ds.Id, *params.DelegationSetId) { + delegationSet = &ds + break + } + } + if delegationSet == nil { + delegationSet = &r53types.DelegationSet{ + NameServers: []string{r.randNameServer(), r.randNameServer()}, + } + } + r.hostedZones = append(r.hostedZones, r53HostedZone{ + HostedZone: hostedZone, + DelegationSet: *delegationSet, + }) + return &route53.CreateHostedZoneOutput{ + DelegationSet: delegationSet, + HostedZone: &hostedZone, + Location: ptr.String("https://route53.amazonaws.com/2013-04-01" + *hostedZone.Id), + }, nil +} + +func TestPrepareDomainDelegationMocked(t *testing.T) { + testPrepareDomainDelegationNew(t, &r53Mock{}) + testPrepareDomainDelegationLegacy(t, &r53Mock{}) +} + +func testPrepareDomainDelegationNew(t *testing.T, r53Client route53API) { + const projectDomain = "byoc.example.internal" + + nsServers, delegationSetId, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if len(nsServers) == 0 { + t.Error("expected name servers") + } + if delegationSetId == "" { + t.Fatal("expected delegation set id") + } + + t.Run("reuse existing delegation set", func(t *testing.T) { + nsServers2, delegationSetId2, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(nsServers, nsServers2) { + t.Error("expected same name servers") + } + if delegationSetId != delegationSetId2 { + t.Error("expected same delegation set id") + } + }) + + t.Run("reuse existing hosted zone", func(t *testing.T) { + // Now create the hosted zone like Pulumi would + hz, err := r53Client.CreateHostedZone(ctx, &route53.CreateHostedZoneInput{ + CallerReference: ptr.String(projectDomain + " from testPrepareDomainDelegationNew " + pkg.RandomID()), + Name: ptr.String(projectDomain), + DelegationSetId: ptr.String(delegationSetId), + HostedZoneConfig: &r53types.HostedZoneConfig{}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, err := r53Client.DeleteHostedZone(ctx, &route53.DeleteHostedZoneInput{ + Id: hz.HostedZone.Id, + }) + if err != nil { + t.Error(err) + } + }) + + nsServers2, delegationSetId2, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(nsServers, nsServers2) { + t.Error("expected same name servers") + } + if delegationSetId != delegationSetId2 { + t.Error("expected same delegation set id") + } + }) +} + +func testPrepareDomainDelegationLegacy(t *testing.T, r53Client route53API) { + const projectDomain = "byoc-legacy.example.internal" + + ctx := context.Background() + + // "Create" the legacy hosted zone + hz, err := r53Client.CreateHostedZone(ctx, &route53.CreateHostedZoneInput{ + CallerReference: ptr.String(projectDomain + " from testPrepareDomainDelegationLegacy " + pkg.RandomID()), + Name: ptr.String(projectDomain), + HostedZoneConfig: &r53types.HostedZoneConfig{ + Comment: ptr.String(aws.CreateHostedZoneCommentLegacy), + }, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, err := r53Client.DeleteHostedZone(ctx, &route53.DeleteHostedZoneInput{ + Id: hz.HostedZone.Id, + }) + if err != nil { + t.Error(err) + } + }) + + nsServers, delegationSetId, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if len(nsServers) == 0 { + t.Error("expected name servers") + } + if !slices.Equal(nsServers, hz.DelegationSet.NameServers) { + t.Error("expected same name servers") + } + if delegationSetId != "" { + t.Fatal("expected no delegation set id") + } +} diff --git a/src/pkg/clouds/aws/route53.go b/src/pkg/clouds/aws/route53.go index 5a3e90b3c..705e2d77a 100644 --- a/src/pkg/clouds/aws/route53.go +++ b/src/pkg/clouds/aws/route53.go @@ -3,9 +3,9 @@ package aws import ( "context" "errors" - "strings" "time" + "github.com/DefangLabs/defang/src/pkg/dns" "github.com/aws/aws-sdk-go-v2/service/route53" "github.com/aws/aws-sdk-go-v2/service/route53/types" "github.com/aws/smithy-go/ptr" @@ -17,9 +17,18 @@ var ( ErrNoDelegationSetFound = errors.New("no Route53 delegation set found") ) -func CreateDelegationSet(ctx context.Context, zoneId *string, r53 *route53.Client) (*types.DelegationSet, error) { +type Route53API interface { + CreateHostedZone(ctx context.Context, params *route53.CreateHostedZoneInput, optFns ...func(*route53.Options)) (*route53.CreateHostedZoneOutput, error) + CreateReusableDelegationSet(ctx context.Context, params *route53.CreateReusableDelegationSetInput, optFns ...func(*route53.Options)) (*route53.CreateReusableDelegationSetOutput, error) + GetHostedZone(ctx context.Context, params *route53.GetHostedZoneInput, optFns ...func(*route53.Options)) (*route53.GetHostedZoneOutput, error) + ListReusableDelegationSets(ctx context.Context, params *route53.ListReusableDelegationSetsInput, optFns ...func(*route53.Options)) (*route53.ListReusableDelegationSetsOutput, error) + ListHostedZonesByName(ctx context.Context, params *route53.ListHostedZonesByNameInput, optFns ...func(*route53.Options)) (*route53.ListHostedZonesByNameOutput, error) + ListResourceRecordSets(ctx context.Context, params *route53.ListResourceRecordSetsInput, optFns ...func(*route53.Options)) (*route53.ListResourceRecordSetsOutput, error) +} + +func CreateDelegationSet(ctx context.Context, zoneId *string, r53 Route53API) (*types.DelegationSet, error) { params := &route53.CreateReusableDelegationSetInput{ - CallerReference: ptr.String("Created by Defang CLI" + time.Now().String()), + CallerReference: ptr.String("Created by Defang CLI " + time.Now().Format(time.RFC3339Nano)), HostedZoneId: zoneId, } resp, err := r53.CreateReusableDelegationSet(ctx, params) @@ -29,7 +38,18 @@ func CreateDelegationSet(ctx context.Context, zoneId *string, r53 *route53.Clien return resp.DelegationSet, err } -func GetDelegationSet(ctx context.Context, r53 *route53.Client) (*types.DelegationSet, error) { +func GetDelegationSetByZone(ctx context.Context, zoneId *string, r53 Route53API) (*types.DelegationSet, error) { + params := &route53.GetHostedZoneInput{ + Id: zoneId, + } + resp, err := r53.GetHostedZone(ctx, params) + if err != nil { + return nil, err + } + return resp.DelegationSet, nil +} + +func GetDelegationSet(ctx context.Context, r53 Route53API) (*types.DelegationSet, error) { params := &route53.ListReusableDelegationSetsInput{ MaxItems: ptr.Int32(1), } @@ -43,7 +63,7 @@ func GetDelegationSet(ctx context.Context, r53 *route53.Client) (*types.Delegati return &resp.DelegationSets[0], nil } -func GetHostedZoneByName(ctx context.Context, domain string, r53 *route53.Client) (*types.HostedZone, error) { +func GetHostedZoneByName(ctx context.Context, domain string, r53 Route53API) (*types.HostedZone, error) { params := &route53.ListHostedZonesByNameInput{ DNSName: ptr.String(domain), MaxItems: ptr.Int32(1), @@ -65,15 +85,15 @@ func GetHostedZoneByName(ctx context.Context, domain string, r53 *route53.Client return &zone, nil } -const CreateHostedZoneComment = "Created by defang cli" +const CreateHostedZoneCommentLegacy = "Created by defang cli" // Deprecated: let Pulumi create the hosted zone -func CreateHostedZone(ctx context.Context, domain string, r53 *route53.Client) (*types.HostedZone, error) { +func CreateHostedZone(ctx context.Context, domain string, r53 Route53API) (*types.HostedZone, error) { params := &route53.CreateHostedZoneInput{ Name: ptr.String(domain), CallerReference: ptr.String(domain + time.Now().String()), HostedZoneConfig: &types.HostedZoneConfig{ - Comment: ptr.String(CreateHostedZoneComment), + Comment: ptr.String(CreateHostedZoneCommentLegacy), }, } resp, err := r53.CreateHostedZone(ctx, params) @@ -83,7 +103,7 @@ func CreateHostedZone(ctx context.Context, domain string, r53 *route53.Client) ( return resp.HostedZone, nil } -func ListResourceRecords(ctx context.Context, zoneId, recordName string, recordType types.RRType, r53 *route53.Client) ([]string, error) { +func ListResourceRecords(ctx context.Context, zoneId, recordName string, recordType types.RRType, r53 Route53API) ([]string, error) { listInput := &route53.ListResourceRecordSetsInput{ HostedZoneId: ptr.String(zoneId), StartRecordName: ptr.String(recordName), @@ -103,11 +123,11 @@ func ListResourceRecords(ctx context.Context, zoneId, recordName string, recordT records := listResp.ResourceRecordSets[0].ResourceRecords values := make([]string, len(records)) for i, record := range records { - values[i] = strings.TrimSuffix(*record.Value, ".") // normalize the value + values[i] = dns.Normalize(*record.Value) } return values, nil } func isSameDomain(domain1 string, domain2 string) bool { - return strings.TrimSuffix(domain1, ".") == strings.TrimSuffix(domain2, ".") + return dns.Normalize(domain1) == dns.Normalize(domain2) } diff --git a/src/pkg/clouds/aws/route53_test.go b/src/pkg/clouds/aws/route53_test.go index 06748d645..de98645aa 100644 --- a/src/pkg/clouds/aws/route53_test.go +++ b/src/pkg/clouds/aws/route53_test.go @@ -13,6 +13,8 @@ import ( ) func TestGetDelegationSet(t *testing.T) { + t.Skip("broken") + ctx := context.Background() cfg, err := config.LoadDefaultConfig(ctx) if err != nil { @@ -23,11 +25,13 @@ func TestGetDelegationSet(t *testing.T) { var ds *types.DelegationSet t.Cleanup(func() { - _, err := r53Client.DeleteReusableDelegationSet(ctx, &route53.DeleteReusableDelegationSetInput{ - Id: ds.Id, - }) - if err != nil { - t.Error(err) + if ds != nil { + _, err := r53Client.DeleteReusableDelegationSet(ctx, &route53.DeleteReusableDelegationSetInput{ + Id: ds.Id, + }) + if err != nil { + t.Error(err) + } } }) @@ -64,6 +68,12 @@ func TestGetDelegationSet(t *testing.T) { if *dss.Id != *ds.Id { t.Errorf("expected delegation set id %s, got: %s", *ds.Id, *dss.Id) } - }) + // Second call should fail + _, err = CreateDelegationSet(ctx, nil, r53Client) + var apiErr *types.DelegationSetAlreadyCreated + if !errors.As(err, &apiErr) { + t.Errorf("expected DelegationSetAlreadyCreated error, got: %v", err) + } + }) } diff --git a/src/pkg/dns/check.go b/src/pkg/dns/check.go index 73c0f03b3..4e07b901f 100644 --- a/src/pkg/dns/check.go +++ b/src/pkg/dns/check.go @@ -24,11 +24,15 @@ var ( errDNSNotInSync = errors.New("DNS not in sync") ) +func Normalize(domain string) string { + return strings.TrimSuffix(domain, ".") +} + // The DNS is considered ready if the CNAME of the domain is pointing to the ALB domain and in sync // OR if the A record of the domain is pointing to the same IP addresses of the ALB domain and in sync func CheckDomainDNSReady(ctx context.Context, domain string, validCNAMEs []string) bool { for i, validCNAME := range validCNAMEs { - validCNAMEs[i] = strings.TrimSuffix(validCNAME, ".") + validCNAMEs[i] = Normalize(validCNAME) } cname, err := getCNAMEInSync(ctx, domain) Logger.Debugf("CNAME for %v is: '%v', err: %v", domain, cname, err) @@ -37,7 +41,7 @@ func CheckDomainDNSReady(ctx context.Context, domain string, validCNAMEs []strin Logger.Debugf("CNAME for %v is not in sync: %v", domain, cname) return false } - cname = strings.TrimSuffix(cname, ".") + cname = Normalize(cname) if slices.Contains(validCNAMEs, cname) { Logger.Debugf("CNAME for %v is in sync: %v", domain, cname) return true From 5e2fca84207807e648691e674e365dd70228e386 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Wed, 8 Jan 2025 15:11:34 -0800 Subject: [PATCH 2/5] update proto --- src/protos/io/defang/v1/fabric.pb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protos/io/defang/v1/fabric.pb.go b/src/protos/io/defang/v1/fabric.pb.go index 0a56fbd05..4463d8d7f 100644 --- a/src/protos/io/defang/v1/fabric.pb.go +++ b/src/protos/io/defang/v1/fabric.pb.go @@ -2,7 +2,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.1 +// protoc-gen-go v1.36.2 // protoc (unknown) // source: io/defang/v1/fabric.proto From d631f92e946e20235d26037dcff751c81334b3e4 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Wed, 8 Jan 2025 21:21:46 -0800 Subject: [PATCH 3/5] Move domain delegation to new files --- src/pkg/cli/client/byoc/aws/byoc.go | 80 ----- .../client/byoc/aws/byoc_integration_test.go | 15 - src/pkg/cli/client/byoc/aws/byoc_test.go | 265 ----------------- src/pkg/cli/client/byoc/aws/domain.go | 116 ++++++++ .../byoc/aws/domain_integration_test.go | 24 ++ src/pkg/cli/client/byoc/aws/domain_test.go | 278 ++++++++++++++++++ 6 files changed, 418 insertions(+), 360 deletions(-) create mode 100644 src/pkg/cli/client/byoc/aws/domain.go create mode 100644 src/pkg/cli/client/byoc/aws/domain_integration_test.go create mode 100644 src/pkg/cli/client/byoc/aws/domain_test.go diff --git a/src/pkg/cli/client/byoc/aws/byoc.go b/src/pkg/cli/client/byoc/aws/byoc.go index e894079c2..da99105bd 100644 --- a/src/pkg/cli/client/byoc/aws/byoc.go +++ b/src/pkg/cli/client/byoc/aws/byoc.go @@ -10,7 +10,6 @@ import ( "net" "os" "slices" - "sort" "strings" "sync" "time" @@ -26,14 +25,12 @@ import ( "github.com/DefangLabs/defang/src/pkg/http" "github.com/DefangLabs/defang/src/pkg/logs" "github.com/DefangLabs/defang/src/pkg/term" - "github.com/DefangLabs/defang/src/pkg/track" "github.com/DefangLabs/defang/src/pkg/types" defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" awssdk "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/credentials/stscreds" cwTypes "github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs/types" "github.com/aws/aws-sdk-go-v2/service/route53" - r53types "github.com/aws/aws-sdk-go-v2/service/route53/types" "github.com/aws/aws-sdk-go-v2/service/s3" s3types "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/aws/aws-sdk-go-v2/service/sts" @@ -340,83 +337,6 @@ func (b *ByocAws) PrepareDomainDelegation(ctx context.Context, req client.Prepar return &resp, nil } -func prepareDomainDelegation(ctx context.Context, projectDomain string, r53Client aws.Route53API) (nsServers []string, delegationSetId string, err error) { - // There's four cases to consider: - // 1. The subdomain zone does not exist: we create/get a delegation set and get its NS records and let CD/Pulumi create the hosted zone - // 2. The subdomain zone exists: - // a. The zone was created by the older CLI: we need to get the NS records from the existing zone and pass to Fabric; no delegation set - // b. The zone was created by the new CD/Pulumi: we get the NS records from the delegation set and let CD/Pulumi create/update the hosted zone - // c. The zone was created another way: get the NS records from the existing zone and pass to Fabric; no delegation set - - var delegationSet *r53types.DelegationSet - zone, err := aws.GetHostedZoneByName(ctx, projectDomain, r53Client) - if err != nil { - // The only acceptable error is that the zone was not found - if !errors.Is(err, aws.ErrZoneNotFound) { - return nil, "", err // TODO: we should not fail deployment if this fails - } - term.Debugf("Zone %q not found, delegation set will be created", projectDomain) - - // Case 1: The zone doesn't exist: we'll create/get a delegation set and let CD/Pulumi create the hosted zone - - // Avoid creating a new delegation set if one already exists - delegationSet, err = aws.GetDelegationSet(ctx, r53Client) - // Create a new delegation set if it doesn't exist - if errors.Is(err, aws.ErrNoDelegationSetFound) { - // Create a new delegation set. There's a race condition here, where two deployments could create two different delegation sets, - // but this is acceptable because the next time the zone is deployed, we'll get the existing delegation set from the zone. - delegationSet, err = aws.CreateDelegationSet(ctx, nil, r53Client) - } - if err != nil { - return nil, "", err - } - } else { - // Case 2: Get the NS records for the existing subdomain zone - nsServers, err = aws.ListResourceRecords(ctx, *zone.Id, projectDomain, r53types.RRTypeNs, r53Client) - if err != nil { - return nil, "", err // TODO: we should not fail deployment if this fails - } - term.Debugf("Zone %q found, NS records: %v", projectDomain, nsServers) - - // Check if the zone was created by the older CLI (before the delegation set was introduced) - if zone.Config.Comment != nil && *zone.Config.Comment == aws.CreateHostedZoneCommentLegacy { - // Case 2a: The zone was created by the older CLI, we'll use the existing NS records; track how many times this happens - track.Evt("Compose-Up delegateSubdomain old", track.P("domain", projectDomain)) - return nsServers, "", nil - } - - // Case 2b or 2c: The zone was not created by an older version of this CLI. We'll get the delegation set and let CD/Pulumi create/update the hosted zone - // TODO: we need to detect the case 2c where the zone was created by another tool and we need to use the existing NS records - - // Create a reusable delegation set for the existing subdomain zone - delegationSet, err = aws.CreateDelegationSet(ctx, zone.Id, r53Client) - if delegationSetAlreadyReusable := new(r53types.DelegationSetAlreadyReusable); errors.As(err, &delegationSetAlreadyReusable) { - term.Debug("Route53 delegation set already created:", err) - delegationSet, err = aws.GetDelegationSetByZone(ctx, zone.Id, r53Client) - } - if err != nil { - return nil, "", err - } - } - - if len(delegationSet.NameServers) == 0 { - return nil, "", errors.New("no NS records found for the delegation set") // should not happen - } - term.Debug("Route53 delegation set ID:", *delegationSet.Id) - delegationSetId = strings.TrimPrefix(*delegationSet.Id, "/delegationset/") - - // Ensure the NS records match the ones from the delegation set if the zone already exists - sort.Strings(nsServers) - sort.Strings(delegationSet.NameServers) - if !slices.Equal(delegationSet.NameServers, nsServers) { - track.Evt("Compose-Up delegateSubdomain diff", track.P("fromDS", delegationSet.NameServers), track.P("fromZone", nsServers)) - term.Debugf("NS records for the existing subdomain zone do not match the delegation set: %v <> %v", delegationSet.NameServers, nsServers) - // FIXME: this occurred 4 times - } - - return delegationSet.NameServers, delegationSetId, nil -} - func (b *ByocAws) AccountInfo(ctx context.Context) (client.AccountInfo, error) { // Use STS to get the account ID cfg, err := b.driver.LoadConfig(ctx) diff --git a/src/pkg/cli/client/byoc/aws/byoc_integration_test.go b/src/pkg/cli/client/byoc/aws/byoc_integration_test.go index 16270441a..8769ad317 100644 --- a/src/pkg/cli/client/byoc/aws/byoc_integration_test.go +++ b/src/pkg/cli/client/byoc/aws/byoc_integration_test.go @@ -8,8 +8,6 @@ import ( "testing" defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" - "github.com/aws/aws-sdk-go-v2/config" - "github.com/aws/aws-sdk-go-v2/service/route53" "github.com/bufbuild/connect-go" ) @@ -152,16 +150,3 @@ func TestListSecrets(t *testing.T) { } }) } - -func TestPrepareDomainDelegation(t *testing.T) { - ctx := context.Background() - cfg, err := config.LoadDefaultConfig(ctx) - if err != nil { - t.Fatal(err) - } - - r53Client := route53.NewFromConfig(cfg) - - testPrepareDomainDelegationNew(t, r53Client) - testPrepareDomainDelegationLegacy(t, r53Client) -} diff --git a/src/pkg/cli/client/byoc/aws/byoc_test.go b/src/pkg/cli/client/byoc/aws/byoc_test.go index 17878c8fa..429b4597a 100644 --- a/src/pkg/cli/client/byoc/aws/byoc_test.go +++ b/src/pkg/cli/client/byoc/aws/byoc_test.go @@ -6,24 +6,18 @@ import ( "context" "embed" "encoding/json" - "errors" "io" "path" - "slices" "strings" "sync" "testing" - "github.com/DefangLabs/defang/src/pkg" "github.com/DefangLabs/defang/src/pkg/cli/client/byoc" "github.com/DefangLabs/defang/src/pkg/clouds/aws" "github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs" "github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs/cfn" "github.com/DefangLabs/defang/src/pkg/types" defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" - "github.com/aws/aws-sdk-go-v2/service/route53" - r53types "github.com/aws/aws-sdk-go-v2/service/route53/types" - "github.com/aws/smithy-go/ptr" composeTypes "github.com/compose-spec/compose-go/v2/types" ) @@ -173,262 +167,3 @@ func TestSubscribe(t *testing.T) { }) } } - -type r53HostedZone struct { - r53types.HostedZone - r53types.DelegationSet // no ID => not reusable -} - -type route53API interface { - aws.Route53API - DeleteHostedZone(ctx context.Context, params *route53.DeleteHostedZoneInput, optFns ...func(*route53.Options)) (*route53.DeleteHostedZoneOutput, error) -} - -type r53Mock struct { - hostedZones []r53HostedZone - delegationSets []r53types.DelegationSet -} - -func (r r53Mock) ListHostedZonesByName(ctx context.Context, params *route53.ListHostedZonesByNameInput, optFns ...func(*route53.Options)) (*route53.ListHostedZonesByNameOutput, error) { - var hostedZones []r53types.HostedZone - for _, hz := range r.hostedZones { - if params.DNSName != nil && *hz.Name < *params.DNSName { // assume ASCII order - continue - } - hostedZones = append(hostedZones, hz.HostedZone) - if params.MaxItems != nil && len(hostedZones) >= int(*params.MaxItems) { - break - } - } - return &route53.ListHostedZonesByNameOutput{ - HostedZones: hostedZones, - DNSName: params.DNSName, - MaxItems: params.MaxItems, - HostedZoneId: params.HostedZoneId, - }, nil -} - -func (r r53Mock) ListResourceRecordSets(ctx context.Context, params *route53.ListResourceRecordSetsInput, optFns ...func(*route53.Options)) (*route53.ListResourceRecordSetsOutput, error) { - for _, hz := range r.hostedZones { - if *hz.HostedZone.Id != *params.HostedZoneId { - continue - } - var recordSets []r53types.ResourceRecord - if params.StartRecordType == r53types.RRTypeNs { - // Copy the NS records from the hosted zone - for _, ns := range hz.NameServers { - recordSets = append(recordSets, r53types.ResourceRecord{Value: ptr.String(ns)}) - } - } - return &route53.ListResourceRecordSetsOutput{ - MaxItems: params.MaxItems, - ResourceRecordSets: []r53types.ResourceRecordSet{ - { - Name: ptr.String(*hz.Name), - Type: params.StartRecordType, - ResourceRecords: recordSets, - }, - }, - }, nil - } - return nil, errors.New("hosted zone not found") -} - -func (r *r53Mock) CreateReusableDelegationSet(ctx context.Context, params *route53.CreateReusableDelegationSetInput, optFns ...func(*route53.Options)) (*route53.CreateReusableDelegationSetOutput, error) { - for _, ds := range r.delegationSets { - if *ds.CallerReference == *params.CallerReference { - return nil, &r53types.DelegationSetAlreadyCreated{} - } - } - var delegationSet *r53types.DelegationSet - if params.HostedZoneId != nil { - for _, hz := range r.hostedZones { - if strings.HasSuffix(*hz.HostedZone.Id, *params.HostedZoneId) { - delegationSet = &hz.DelegationSet - break - } - } - if delegationSet == nil { - return nil, &r53types.NoSuchHostedZone{} - } - if delegationSet.Id != nil { - return nil, &r53types.DelegationSetAlreadyReusable{} - } - delegationSet.Id = ptr.String("/delegationset/N" + strings.ToUpper(pkg.RandomID())) - delegationSet.CallerReference = params.CallerReference - } else { - delegationSet = &r53types.DelegationSet{ - CallerReference: params.CallerReference, - Id: ptr.String("/delegationset/N" + strings.ToUpper(pkg.RandomID())), - NameServers: []string{r.randNameServer(), r.randNameServer()}, - } - } - r.delegationSets = append(r.delegationSets, *delegationSet) - return &route53.CreateReusableDelegationSetOutput{ - DelegationSet: delegationSet, - Location: ptr.String("https://route53.amazonaws.com/2013-04-01" + *delegationSet.Id), - }, nil -} - -func (r r53Mock) ListReusableDelegationSets(ctx context.Context, params *route53.ListReusableDelegationSetsInput, optFns ...func(*route53.Options)) (*route53.ListReusableDelegationSetsOutput, error) { - return &route53.ListReusableDelegationSetsOutput{ - DelegationSets: r.delegationSets, - Marker: params.Marker, - MaxItems: params.MaxItems, - }, nil -} - -func (r53Mock) randNameServer() string { - return pkg.RandomID() + ".example.com" -} - -func (r r53Mock) GetHostedZone(ctx context.Context, params *route53.GetHostedZoneInput, optFns ...func(*route53.Options)) (*route53.GetHostedZoneOutput, error) { - for _, hz := range r.hostedZones { - if strings.HasSuffix(*hz.HostedZone.Id, *params.Id) { - return &route53.GetHostedZoneOutput{ - HostedZone: &hz.HostedZone, - DelegationSet: &hz.DelegationSet, - }, nil - } - } - return nil, &r53types.NoSuchHostedZone{} -} - -func (r r53Mock) DeleteHostedZone(ctx context.Context, params *route53.DeleteHostedZoneInput, optFns ...func(*route53.Options)) (*route53.DeleteHostedZoneOutput, error) { - return &route53.DeleteHostedZoneOutput{}, nil -} - -func (r *r53Mock) CreateHostedZone(ctx context.Context, params *route53.CreateHostedZoneInput, optFns ...func(*route53.Options)) (*route53.CreateHostedZoneOutput, error) { - hostedZone := r53types.HostedZone{ - Id: ptr.String("/hostedzone/Z" + strings.ToUpper(pkg.RandomID())), - CallerReference: params.CallerReference, - Config: params.HostedZoneConfig, - Name: params.Name, - } - var delegationSet *r53types.DelegationSet - for _, ds := range r.delegationSets { - if strings.HasSuffix(*ds.Id, *params.DelegationSetId) { - delegationSet = &ds - break - } - } - if delegationSet == nil { - delegationSet = &r53types.DelegationSet{ - NameServers: []string{r.randNameServer(), r.randNameServer()}, - } - } - r.hostedZones = append(r.hostedZones, r53HostedZone{ - HostedZone: hostedZone, - DelegationSet: *delegationSet, - }) - return &route53.CreateHostedZoneOutput{ - DelegationSet: delegationSet, - HostedZone: &hostedZone, - Location: ptr.String("https://route53.amazonaws.com/2013-04-01" + *hostedZone.Id), - }, nil -} - -func TestPrepareDomainDelegationMocked(t *testing.T) { - testPrepareDomainDelegationNew(t, &r53Mock{}) - testPrepareDomainDelegationLegacy(t, &r53Mock{}) -} - -func testPrepareDomainDelegationNew(t *testing.T, r53Client route53API) { - const projectDomain = "byoc.example.internal" - - nsServers, delegationSetId, err := prepareDomainDelegation(ctx, projectDomain, r53Client) - if err != nil { - t.Fatal(err) - } - if len(nsServers) == 0 { - t.Error("expected name servers") - } - if delegationSetId == "" { - t.Fatal("expected delegation set id") - } - - t.Run("reuse existing delegation set", func(t *testing.T) { - nsServers2, delegationSetId2, err := prepareDomainDelegation(ctx, projectDomain, r53Client) - if err != nil { - t.Fatal(err) - } - if !slices.Equal(nsServers, nsServers2) { - t.Error("expected same name servers") - } - if delegationSetId != delegationSetId2 { - t.Error("expected same delegation set id") - } - }) - - t.Run("reuse existing hosted zone", func(t *testing.T) { - // Now create the hosted zone like Pulumi would - hz, err := r53Client.CreateHostedZone(ctx, &route53.CreateHostedZoneInput{ - CallerReference: ptr.String(projectDomain + " from testPrepareDomainDelegationNew " + pkg.RandomID()), - Name: ptr.String(projectDomain), - DelegationSetId: ptr.String(delegationSetId), - HostedZoneConfig: &r53types.HostedZoneConfig{}, - }) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - _, err := r53Client.DeleteHostedZone(ctx, &route53.DeleteHostedZoneInput{ - Id: hz.HostedZone.Id, - }) - if err != nil { - t.Error(err) - } - }) - - nsServers2, delegationSetId2, err := prepareDomainDelegation(ctx, projectDomain, r53Client) - if err != nil { - t.Fatal(err) - } - if !slices.Equal(nsServers, nsServers2) { - t.Error("expected same name servers") - } - if delegationSetId != delegationSetId2 { - t.Error("expected same delegation set id") - } - }) -} - -func testPrepareDomainDelegationLegacy(t *testing.T, r53Client route53API) { - const projectDomain = "byoc-legacy.example.internal" - - ctx := context.Background() - - // "Create" the legacy hosted zone - hz, err := r53Client.CreateHostedZone(ctx, &route53.CreateHostedZoneInput{ - CallerReference: ptr.String(projectDomain + " from testPrepareDomainDelegationLegacy " + pkg.RandomID()), - Name: ptr.String(projectDomain), - HostedZoneConfig: &r53types.HostedZoneConfig{ - Comment: ptr.String(aws.CreateHostedZoneCommentLegacy), - }, - }) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - _, err := r53Client.DeleteHostedZone(ctx, &route53.DeleteHostedZoneInput{ - Id: hz.HostedZone.Id, - }) - if err != nil { - t.Error(err) - } - }) - - nsServers, delegationSetId, err := prepareDomainDelegation(ctx, projectDomain, r53Client) - if err != nil { - t.Fatal(err) - } - if len(nsServers) == 0 { - t.Error("expected name servers") - } - if !slices.Equal(nsServers, hz.DelegationSet.NameServers) { - t.Error("expected same name servers") - } - if delegationSetId != "" { - t.Fatal("expected no delegation set id") - } -} diff --git a/src/pkg/cli/client/byoc/aws/domain.go b/src/pkg/cli/client/byoc/aws/domain.go new file mode 100644 index 000000000..af3f731f0 --- /dev/null +++ b/src/pkg/cli/client/byoc/aws/domain.go @@ -0,0 +1,116 @@ +package aws + +import ( + "context" + "errors" + "slices" + "strings" + + "github.com/DefangLabs/defang/src/pkg/clouds/aws" + "github.com/DefangLabs/defang/src/pkg/dns" + "github.com/DefangLabs/defang/src/pkg/term" + "github.com/DefangLabs/defang/src/pkg/track" + "github.com/aws/aws-sdk-go-v2/service/route53/types" +) + +func prepareDomainDelegation(ctx context.Context, projectDomain string, r53Client aws.Route53API) (nsServers []string, delegationSetId string, err error) { + // There's four cases to consider: + // 1. The subdomain zone does not exist: we create/get a delegation set and get its NS records and let CD/Pulumi create the hosted zone + // 2. The subdomain zone exists: + // a. The zone was created by the older CLI: we need to get the NS records from the existing zone and pass to Fabric; no delegation set + // b. The zone was created by the new CD/Pulumi: we get the NS records from the delegation set and let CD/Pulumi create/update the hosted zone + // c. The zone was created another way: get the NS records from the existing zone and pass to Fabric; no delegation set + + var delegationSet *types.DelegationSet + zone, err := aws.GetHostedZoneByName(ctx, projectDomain, r53Client) + if err != nil { + // The only acceptable error is that the zone was not found + if !errors.Is(err, aws.ErrZoneNotFound) { + return nil, "", err // TODO: we should not fail deployment if GetHostedZoneByName fails + } + term.Debugf("Zone %q not found, delegation set will be created", projectDomain) + + // Case 1: The zone doesn't exist: we'll create/get a delegation set and let CD/Pulumi create the hosted zone + delegationSet, err = prepareDomainDelegationFromDelegationSet(ctx, r53Client) + if err != nil { + return nil, "", err + } + } else { + // Case 2: Get the NS records for the existing subdomain zone + delegationSet, err = prepareDomainDelegationFromZone(ctx, zone, r53Client) + if err != nil { + return nil, "", err + } + } + + if len(delegationSet.NameServers) == 0 { + return nil, "", errors.New("no NS records found for the delegation set") // should not happen + } + if delegationSet.Id != nil { + term.Debug("Route53 delegation set ID:", *delegationSet.Id) + delegationSetId = strings.TrimPrefix(*delegationSet.Id, "/delegationset/") + } + + return delegationSet.NameServers, delegationSetId, nil +} + +func prepareDomainDelegationFromDelegationSet(ctx context.Context, r53Client aws.Route53API) (*types.DelegationSet, error) { + // Avoid creating a new delegation set if one already exists + delegationSet, err := aws.GetDelegationSet(ctx, r53Client) + // Create a new delegation set if it doesn't exist + if errors.Is(err, aws.ErrNoDelegationSetFound) { + // Create a new delegation set. There's a race condition here, where two deployments could create two different delegation sets, + // but this is acceptable because the next time the zone is deployed, we'll get the existing delegation set from the zone. + delegationSet, err = aws.CreateDelegationSet(ctx, nil, r53Client) + } + if err != nil { + return nil, err + } + return delegationSet, err +} + +func prepareDomainDelegationFromZone(ctx context.Context, zone *types.HostedZone, r53Client aws.Route53API) (*types.DelegationSet, error) { + projectDomain := dns.Normalize(*zone.Name) + nsServers, err := aws.ListResourceRecords(ctx, *zone.Id, projectDomain, types.RRTypeNs, r53Client) + if err != nil { + return nil, err // TODO: we should not fail deployment if ListResourceRecords fails + } + term.Debugf("Zone %q found, NS records: %v", projectDomain, nsServers) + + // Check if the zone was created by the older CLI (delegation set was introduced in v0.6.4) + var delegationSet *types.DelegationSet + if zone.Config.Comment != nil && *zone.Config.Comment == aws.CreateHostedZoneCommentLegacy { + // Case 2a: The zone was created by the older CLI, we'll use the existing NS records; track how many times this happens + track.Evt("Compose-Up delegateSubdomain old", track.P("domain", projectDomain)) + + // Create a dummy delegation set with the existing NS records (but no ID) + delegationSet = &types.DelegationSet{ + NameServers: nsServers, + } + } else { + // Case 2b or 2c: The zone was not created by an older version of this CLI. We'll get the delegation set and let CD/Pulumi create/update the hosted zone + // TODO: we need to detect the case 2c where the zone was created by another tool and we need to use the existing NS records + + // Create a reusable delegation set for the existing subdomain zone + delegationSet, err = aws.CreateDelegationSet(ctx, zone.Id, r53Client) + if delegationSetAlreadyReusable := new(types.DelegationSetAlreadyReusable); errors.As(err, &delegationSetAlreadyReusable) { + term.Debug("Route53 delegation set already created:", err) + delegationSet, err = aws.GetDelegationSetByZone(ctx, zone.Id, r53Client) + } + } + + // Ensure the zone's NS records match the ones from the delegation set if the zone already exists + if !slicesEqualUnordered(delegationSet.NameServers, nsServers) { + track.Evt("Compose-Up delegateSubdomain diff", track.P("fromDS", delegationSet.NameServers), track.P("fromZone", nsServers)) + term.Debugf("NS records for the existing subdomain zone do not match the delegation set: %v <> %v", delegationSet.NameServers, nsServers) + // FIXME: this occurred 34 times between 2024-10-01 and 2025-01-01 + } + + return delegationSet, err +} + +func slicesEqualUnordered(a, b []string) bool { + slices.Sort(a) + slices.Sort(b) + return slices.Equal(a, b) +} diff --git a/src/pkg/cli/client/byoc/aws/domain_integration_test.go b/src/pkg/cli/client/byoc/aws/domain_integration_test.go new file mode 100644 index 000000000..029c20a74 --- /dev/null +++ b/src/pkg/cli/client/byoc/aws/domain_integration_test.go @@ -0,0 +1,24 @@ +//go:build integration + +package aws + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/route53" +) + +func TestPrepareDomainDelegation(t *testing.T) { + ctx := context.Background() + cfg, err := config.LoadDefaultConfig(ctx) + if err != nil { + t.Fatal(err) + } + + r53Client := route53.NewFromConfig(cfg) + + testPrepareDomainDelegationNew(t, r53Client) + testPrepareDomainDelegationLegacy(t, r53Client) +} diff --git a/src/pkg/cli/client/byoc/aws/domain_test.go b/src/pkg/cli/client/byoc/aws/domain_test.go new file mode 100644 index 000000000..c4d632c5b --- /dev/null +++ b/src/pkg/cli/client/byoc/aws/domain_test.go @@ -0,0 +1,278 @@ +package aws + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + + "github.com/DefangLabs/defang/src/pkg" + "github.com/DefangLabs/defang/src/pkg/clouds/aws" + "github.com/aws/aws-sdk-go-v2/service/route53" + "github.com/aws/aws-sdk-go-v2/service/route53/types" + "github.com/aws/smithy-go/ptr" +) + +func TestPrepareDomainDelegationMocked(t *testing.T) { + testPrepareDomainDelegationNew(t, &r53Mock{}) + testPrepareDomainDelegationLegacy(t, &r53Mock{}) +} + +type r53HostedZone struct { + types.HostedZone + types.DelegationSet // no ID => not reusable +} + +type route53API interface { + aws.Route53API + DeleteHostedZone(ctx context.Context, params *route53.DeleteHostedZoneInput, optFns ...func(*route53.Options)) (*route53.DeleteHostedZoneOutput, error) +} + +type r53Mock struct { + hostedZones []r53HostedZone + delegationSets []types.DelegationSet +} + +func (r r53Mock) ListHostedZonesByName(ctx context.Context, params *route53.ListHostedZonesByNameInput, optFns ...func(*route53.Options)) (*route53.ListHostedZonesByNameOutput, error) { + var hostedZones []types.HostedZone + for _, hz := range r.hostedZones { + if params.DNSName != nil && *hz.Name < *params.DNSName { // assume ASCII order + continue + } + hostedZones = append(hostedZones, hz.HostedZone) + if params.MaxItems != nil && len(hostedZones) >= int(*params.MaxItems) { + break + } + } + return &route53.ListHostedZonesByNameOutput{ + HostedZones: hostedZones, + DNSName: params.DNSName, + MaxItems: params.MaxItems, + HostedZoneId: params.HostedZoneId, + }, nil +} + +func (r r53Mock) ListResourceRecordSets(ctx context.Context, params *route53.ListResourceRecordSetsInput, optFns ...func(*route53.Options)) (*route53.ListResourceRecordSetsOutput, error) { + for _, hz := range r.hostedZones { + if *hz.HostedZone.Id != *params.HostedZoneId { + continue + } + var recordSets []types.ResourceRecord + if params.StartRecordType == types.RRTypeNs { + // Copy the NS records from the hosted zone + for _, ns := range hz.NameServers { + recordSets = append(recordSets, types.ResourceRecord{Value: ptr.String(ns)}) + } + } + return &route53.ListResourceRecordSetsOutput{ + MaxItems: params.MaxItems, + ResourceRecordSets: []types.ResourceRecordSet{ + { + Name: ptr.String(*hz.Name), + Type: params.StartRecordType, + ResourceRecords: recordSets, + }, + }, + }, nil + } + return nil, errors.New("hosted zone not found") +} + +func (r *r53Mock) CreateReusableDelegationSet(ctx context.Context, params *route53.CreateReusableDelegationSetInput, optFns ...func(*route53.Options)) (*route53.CreateReusableDelegationSetOutput, error) { + for _, ds := range r.delegationSets { + if *ds.CallerReference == *params.CallerReference { + return nil, &types.DelegationSetAlreadyCreated{} + } + } + var delegationSet *types.DelegationSet + if params.HostedZoneId != nil { + for _, hz := range r.hostedZones { + if strings.HasSuffix(*hz.HostedZone.Id, *params.HostedZoneId) { + delegationSet = &hz.DelegationSet + break + } + } + if delegationSet == nil { + return nil, &types.NoSuchHostedZone{} + } + if delegationSet.Id != nil { + return nil, &types.DelegationSetAlreadyReusable{} + } + delegationSet.Id = ptr.String("/delegationset/N" + strings.ToUpper(pkg.RandomID())) + delegationSet.CallerReference = params.CallerReference + } else { + delegationSet = &types.DelegationSet{ + CallerReference: params.CallerReference, + Id: ptr.String("/delegationset/N" + strings.ToUpper(pkg.RandomID())), + NameServers: []string{r.randNameServer(), r.randNameServer()}, + } + } + r.delegationSets = append(r.delegationSets, *delegationSet) + return &route53.CreateReusableDelegationSetOutput{ + DelegationSet: delegationSet, + Location: ptr.String("https://route53.amazonaws.com/2013-04-01" + *delegationSet.Id), + }, nil +} + +func (r r53Mock) ListReusableDelegationSets(ctx context.Context, params *route53.ListReusableDelegationSetsInput, optFns ...func(*route53.Options)) (*route53.ListReusableDelegationSetsOutput, error) { + return &route53.ListReusableDelegationSetsOutput{ + DelegationSets: r.delegationSets, + Marker: params.Marker, + MaxItems: params.MaxItems, + }, nil +} + +func (r53Mock) randNameServer() string { + return pkg.RandomID() + ".example.com" +} + +func (r r53Mock) GetHostedZone(ctx context.Context, params *route53.GetHostedZoneInput, optFns ...func(*route53.Options)) (*route53.GetHostedZoneOutput, error) { + for _, hz := range r.hostedZones { + if strings.HasSuffix(*hz.HostedZone.Id, *params.Id) { + return &route53.GetHostedZoneOutput{ + HostedZone: &hz.HostedZone, + DelegationSet: &hz.DelegationSet, + }, nil + } + } + return nil, &types.NoSuchHostedZone{} +} + +func (r r53Mock) DeleteHostedZone(ctx context.Context, params *route53.DeleteHostedZoneInput, optFns ...func(*route53.Options)) (*route53.DeleteHostedZoneOutput, error) { + return &route53.DeleteHostedZoneOutput{}, nil +} + +func (r *r53Mock) CreateHostedZone(ctx context.Context, params *route53.CreateHostedZoneInput, optFns ...func(*route53.Options)) (*route53.CreateHostedZoneOutput, error) { + hostedZone := types.HostedZone{ + Id: ptr.String("/hostedzone/Z" + strings.ToUpper(pkg.RandomID())), + CallerReference: params.CallerReference, + Config: params.HostedZoneConfig, + Name: params.Name, + } + var delegationSet *types.DelegationSet + for _, ds := range r.delegationSets { + if strings.HasSuffix(*ds.Id, *params.DelegationSetId) { + delegationSet = &ds + break + } + } + if delegationSet == nil { + delegationSet = &types.DelegationSet{ + NameServers: []string{r.randNameServer(), r.randNameServer()}, + } + } + r.hostedZones = append(r.hostedZones, r53HostedZone{ + HostedZone: hostedZone, + DelegationSet: *delegationSet, + }) + slices.SortFunc(r.hostedZones, func(a, b r53HostedZone) int { + return strings.Compare(*a.Name, *b.Name) + }) + return &route53.CreateHostedZoneOutput{ + DelegationSet: delegationSet, + HostedZone: &hostedZone, + Location: ptr.String("https://route53.amazonaws.com/2013-04-01" + *hostedZone.Id), + }, nil +} + +func testPrepareDomainDelegationNew(t *testing.T, r53Client route53API) { + const projectDomain = "byoc.example.internal" + + nsServers, delegationSetId, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if len(nsServers) == 0 { + t.Error("expected name servers") + } + if delegationSetId == "" { + t.Fatal("expected delegation set id") + } + + t.Run("reuse existing delegation set", func(t *testing.T) { + nsServers2, delegationSetId2, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if !slicesEqualUnordered(nsServers, nsServers2) { + t.Error("expected same name servers") + } + if delegationSetId != delegationSetId2 { + t.Error("expected same delegation set id") + } + }) + + t.Run("reuse existing hosted zone", func(t *testing.T) { + // Now create the hosted zone like Pulumi would + hz, err := r53Client.CreateHostedZone(ctx, &route53.CreateHostedZoneInput{ + CallerReference: ptr.String(projectDomain + " from testPrepareDomainDelegationNew " + pkg.RandomID()), + Name: ptr.String(projectDomain), + DelegationSetId: ptr.String(delegationSetId), + HostedZoneConfig: &types.HostedZoneConfig{}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, err := r53Client.DeleteHostedZone(ctx, &route53.DeleteHostedZoneInput{ + Id: hz.HostedZone.Id, + }) + if err != nil { + t.Error(err) + } + }) + + nsServers2, delegationSetId2, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if !slicesEqualUnordered(nsServers, nsServers2) { + t.Error("expected same name servers") + } + if delegationSetId != delegationSetId2 { + t.Error("expected same delegation set id") + } + }) +} + +func testPrepareDomainDelegationLegacy(t *testing.T, r53Client route53API) { + const projectDomain = "byoc-legacy.example.internal" + + ctx := context.Background() + + // "Create" the legacy hosted zone + hz, err := r53Client.CreateHostedZone(ctx, &route53.CreateHostedZoneInput{ + CallerReference: ptr.String(projectDomain + " from testPrepareDomainDelegationLegacy " + pkg.RandomID()), + Name: ptr.String(projectDomain), + HostedZoneConfig: &types.HostedZoneConfig{ + Comment: ptr.String(aws.CreateHostedZoneCommentLegacy), + }, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, err := r53Client.DeleteHostedZone(ctx, &route53.DeleteHostedZoneInput{ + Id: hz.HostedZone.Id, + }) + if err != nil { + t.Error(err) + } + }) + + nsServers, delegationSetId, err := prepareDomainDelegation(ctx, projectDomain, r53Client) + if err != nil { + t.Fatal(err) + } + if len(nsServers) == 0 { + t.Error("expected name servers") + } + + if !slicesEqualUnordered(nsServers, hz.DelegationSet.NameServers) { + t.Error("expected same name servers") + } + if delegationSetId != "" { + t.Fatal("expected no delegation set id") + } +} From b3b154a0daafb2b4bab29265cd1c10e758576d81 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Thu, 9 Jan 2025 05:38:02 -0800 Subject: [PATCH 4/5] Removed obsolete comment --- src/pkg/cli/client/byoc/aws/domain.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pkg/cli/client/byoc/aws/domain.go b/src/pkg/cli/client/byoc/aws/domain.go index af3f731f0..34da078e6 100644 --- a/src/pkg/cli/client/byoc/aws/domain.go +++ b/src/pkg/cli/client/byoc/aws/domain.go @@ -103,7 +103,6 @@ func prepareDomainDelegationFromZone(ctx context.Context, zone *types.HostedZone if !slicesEqualUnordered(delegationSet.NameServers, nsServers) { track.Evt("Compose-Up delegateSubdomain diff", track.P("fromDS", delegationSet.NameServers), track.P("fromZone", nsServers)) term.Debugf("NS records for the existing subdomain zone do not match the delegation set: %v <> %v", delegationSet.NameServers, nsServers) - // FIXME: this occurred 34 times between 2024-10-01 and 2025-01-01 } return delegationSet, err From 56d3730cb033c3164949175f6fedda0c3d810600 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Thu, 9 Jan 2025 13:27:49 -0800 Subject: [PATCH 5/5] PR comments --- src/pkg/cli/client/byoc/aws/domain.go | 8 ++++---- src/pkg/clouds/aws/route53.go | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/pkg/cli/client/byoc/aws/domain.go b/src/pkg/cli/client/byoc/aws/domain.go index 34da078e6..39b9b1352 100644 --- a/src/pkg/cli/client/byoc/aws/domain.go +++ b/src/pkg/cli/client/byoc/aws/domain.go @@ -31,13 +31,13 @@ func prepareDomainDelegation(ctx context.Context, projectDomain string, r53Clien term.Debugf("Zone %q not found, delegation set will be created", projectDomain) // Case 1: The zone doesn't exist: we'll create/get a delegation set and let CD/Pulumi create the hosted zone - delegationSet, err = prepareDomainDelegationFromDelegationSet(ctx, r53Client) + delegationSet, err = getOrCreateDelegationSet(ctx, r53Client) if err != nil { return nil, "", err } } else { // Case 2: Get the NS records for the existing subdomain zone - delegationSet, err = prepareDomainDelegationFromZone(ctx, zone, r53Client) + delegationSet, err = getOrCreateDelegationSetByZone(ctx, zone, r53Client) if err != nil { return nil, "", err } @@ -54,7 +54,7 @@ func prepareDomainDelegation(ctx context.Context, projectDomain string, r53Clien return delegationSet.NameServers, delegationSetId, nil } -func prepareDomainDelegationFromDelegationSet(ctx context.Context, r53Client aws.Route53API) (*types.DelegationSet, error) { +func getOrCreateDelegationSet(ctx context.Context, r53Client aws.Route53API) (*types.DelegationSet, error) { // Avoid creating a new delegation set if one already exists delegationSet, err := aws.GetDelegationSet(ctx, r53Client) // Create a new delegation set if it doesn't exist @@ -69,7 +69,7 @@ func prepareDomainDelegationFromDelegationSet(ctx context.Context, r53Client aws return delegationSet, err } -func prepareDomainDelegationFromZone(ctx context.Context, zone *types.HostedZone, r53Client aws.Route53API) (*types.DelegationSet, error) { +func getOrCreateDelegationSetByZone(ctx context.Context, zone *types.HostedZone, r53Client aws.Route53API) (*types.DelegationSet, error) { projectDomain := dns.Normalize(*zone.Name) nsServers, err := aws.ListResourceRecords(ctx, *zone.Id, projectDomain, types.RRTypeNs, r53Client) if err != nil { diff --git a/src/pkg/clouds/aws/route53.go b/src/pkg/clouds/aws/route53.go index 705e2d77a..13f446476 100644 --- a/src/pkg/clouds/aws/route53.go +++ b/src/pkg/clouds/aws/route53.go @@ -3,6 +3,7 @@ package aws import ( "context" "errors" + "math/rand" "time" "github.com/DefangLabs/defang/src/pkg/dns" @@ -50,9 +51,7 @@ func GetDelegationSetByZone(ctx context.Context, zoneId *string, r53 Route53API) } func GetDelegationSet(ctx context.Context, r53 Route53API) (*types.DelegationSet, error) { - params := &route53.ListReusableDelegationSetsInput{ - MaxItems: ptr.Int32(1), - } + params := &route53.ListReusableDelegationSetsInput{} resp, err := r53.ListReusableDelegationSets(ctx, params) if err != nil { return nil, err @@ -60,7 +59,9 @@ func GetDelegationSet(ctx context.Context, r53 Route53API) (*types.DelegationSet if len(resp.DelegationSets) == 0 { return nil, ErrNoDelegationSetFound } - return &resp.DelegationSets[0], nil + // Return a random delegation set, to work around the 100 zones-per-delegation-set limit, + // because we can't easily tell how many zones are using each delegation set. + return &resp.DelegationSets[rand.Intn(len(resp.DelegationSets))], nil } func GetHostedZoneByName(ctx context.Context, domain string, r53 Route53API) (*types.HostedZone, error) {