From 7fb25f44ceba19241a7085d78566294c401f50c0 Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Thu, 5 Oct 2023 13:29:14 +0200 Subject: [PATCH 01/11] Allow usage of (multiple) AWS profiles using .credentials file --- main.go | 45 +++++++++-- pkg/apis/externaldns/types.go | 2 + pkg/apis/externaldns/types_test.go | 5 ++ provider/aws/aws.go | 120 +++++++++++++++++++---------- provider/aws/aws_test.go | 96 +++++++++++++---------- provider/aws/session.go | 2 + 6 files changed, 184 insertions(+), 86 deletions(-) diff --git a/main.go b/main.go index 22fefec95c..74e6ad632f 100644 --- a/main.go +++ b/main.go @@ -195,9 +195,39 @@ func main() { zoneTypeFilter := provider.NewZoneTypeFilter(cfg.AWSZoneType) zoneTagFilter := provider.NewZoneTagFilter(cfg.AWSZoneTagFilter) - var awsSession *session.Session + awsSessions := make(map[string]*session.Session, len(cfg.AWSProfiles)) + var awsDefaultSession *session.Session if cfg.Provider == "aws" || cfg.Provider == "aws-sd" || cfg.Registry == "dynamodb" { - awsSession, err = aws.NewSession( + if len(cfg.AWSProfiles) == 0 || (len(cfg.AWSProfiles) == 1 && cfg.AWSProfiles[0] == "") { + session, err := aws.NewSession( + aws.AWSSessionConfig{ + AssumeRole: cfg.AWSAssumeRole, + AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, + APIRetries: cfg.AWSAPIRetries, + }, + ) + if err != nil { + log.Fatal(err) + } + awsSessions[aws.DefaultAWSProfile] = session + } else { + for _, profile := range cfg.AWSProfiles { + session, err := aws.NewSession( + aws.AWSSessionConfig{ + AssumeRole: cfg.AWSAssumeRole, + AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, + APIRetries: cfg.AWSAPIRetries, + Profile: profile, + }, + ) + if err != nil { + log.Fatal(err) + } + awsSessions[profile] = session + } + } + + awsDefaultSession, err = aws.NewSession( aws.AWSSessionConfig{ AssumeRole: cfg.AWSAssumeRole, AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, @@ -227,6 +257,11 @@ func main() { case "alibabacloud": p, err = alibabacloud.NewAlibabaCloudProvider(cfg.AlibabaCloudConfigFile, domainFilter, zoneIDFilter, cfg.AlibabaCloudZoneType, cfg.DryRun) case "aws": + clients := make(map[string]aws.Route53API, len(awsSessions)) + for profile, session := range awsSessions { + clients[profile] = route53.New(session) + } + p, err = aws.NewAWSProvider( aws.AWSConfig{ DomainFilter: domainFilter, @@ -243,7 +278,7 @@ func main() { DryRun: cfg.DryRun, ZoneCacheDuration: cfg.AWSZoneCacheDuration, }, - route53.New(awsSession), + clients, ) case "aws-sd": // Check that only compatible Registry is used with AWS-SD @@ -251,7 +286,7 @@ func main() { log.Infof("Registry \"%s\" cannot be used with AWS Cloud Map. Switching to \"aws-sd\".", cfg.Registry) cfg.Registry = "aws-sd" } - p, err = awssd.NewAWSSDProvider(domainFilter, cfg.AWSZoneType, cfg.DryRun, cfg.AWSSDServiceCleanup, cfg.TXTOwnerID, sd.New(awsSession)) + p, err = awssd.NewAWSSDProvider(domainFilter, cfg.AWSZoneType, cfg.DryRun, cfg.AWSSDServiceCleanup, cfg.TXTOwnerID, sd.New(awsDefaultSession)) case "azure-dns", "azure": p, err = azure.NewAzureProvider(cfg.AzureConfigFile, domainFilter, zoneNameFilter, zoneIDFilter, cfg.AzureSubscriptionID, cfg.AzureResourceGroup, cfg.AzureUserAssignedIdentityClientID, cfg.DryRun) case "azure-private-dns": @@ -438,7 +473,7 @@ func main() { if cfg.AWSDynamoDBRegion != "" { config = config.WithRegion(cfg.AWSDynamoDBRegion) } - r, err = registry.NewDynamoDBRegistry(p, cfg.TXTOwnerID, dynamodb.New(awsSession, config), cfg.AWSDynamoDBTable, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, []byte(cfg.TXTEncryptAESKey), cfg.TXTCacheInterval) + r, err = registry.NewDynamoDBRegistry(p, cfg.TXTOwnerID, dynamodb.New(awsDefaultSession, config), cfg.AWSDynamoDBTable, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, []byte(cfg.TXTEncryptAESKey), cfg.TXTCacheInterval) case "noop": r, err = registry.NewNoopRegistry(p) case "txt": diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 0b25dcafe6..bef5f9c806 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -84,6 +84,7 @@ type Config struct { AWSZoneType string AWSZoneTagFilter []string AWSAssumeRole string + AWSProfiles []string AWSAssumeRoleExternalID string AWSBatchChangeSize int AWSBatchChangeSizeBytes int @@ -490,6 +491,7 @@ func (cfg *Config) ParseFlags(args []string) error { app.Flag("alibaba-cloud-zone-type", "When using the Alibaba Cloud provider, filter for zones of this type (optional, options: public, private)").Default(defaultConfig.AlibabaCloudZoneType).EnumVar(&cfg.AlibabaCloudZoneType, "", "public", "private") app.Flag("aws-zone-type", "When using the AWS provider, filter for zones of this type (optional, options: public, private)").Default(defaultConfig.AWSZoneType).EnumVar(&cfg.AWSZoneType, "", "public", "private") app.Flag("aws-zone-tags", "When using the AWS provider, filter for zones with these tags").Default("").StringsVar(&cfg.AWSZoneTagFilter) + app.Flag("aws-profile", "When using the AWS provider, name of the profile to use").Default("").StringsVar(&cfg.AWSProfiles) app.Flag("aws-assume-role", "When using the AWS API, assume this IAM role. Useful for hosted zones in another AWS account. Specify the full ARN, e.g. `arn:aws:iam::123455567:role/external-dns` (optional)").Default(defaultConfig.AWSAssumeRole).StringVar(&cfg.AWSAssumeRole) app.Flag("aws-assume-role-external-id", "When using the AWS API and assuming a role then specify this external ID` (optional)").Default(defaultConfig.AWSAssumeRoleExternalID).StringVar(&cfg.AWSAssumeRoleExternalID) app.Flag("aws-batch-change-size", "When using the AWS provider, set the maximum number of changes that will be applied in each batch.").Default(strconv.Itoa(defaultConfig.AWSBatchChangeSize)).IntVar(&cfg.AWSBatchChangeSize) diff --git a/pkg/apis/externaldns/types_test.go b/pkg/apis/externaldns/types_test.go index 987fd5ef37..0120f40e2d 100644 --- a/pkg/apis/externaldns/types_test.go +++ b/pkg/apis/externaldns/types_test.go @@ -65,6 +65,7 @@ var ( AWSEvaluateTargetHealth: true, AWSAPIRetries: 3, AWSPreferCNAME: false, + AWSProfiles: []string{""}, AWSZoneCacheDuration: 0 * time.Second, AWSSDServiceCleanup: false, AWSDynamoDBTable: "external-dns", @@ -179,6 +180,7 @@ var ( AWSEvaluateTargetHealth: false, AWSAPIRetries: 13, AWSPreferCNAME: true, + AWSProfiles: []string{"profile1", "profile2"}, AWSZoneCacheDuration: 10 * time.Second, AWSSDServiceCleanup: true, AWSDynamoDBTable: "custom-table", @@ -366,6 +368,8 @@ func TestParseFlags(t *testing.T) { "--aws-batch-change-interval=2s", "--aws-api-retries=13", "--aws-prefer-cname", + "--aws-profile=profile1", + "--aws-profile=profile2", "--aws-zones-cache-duration=10s", "--aws-sd-service-cleanup", "--no-aws-evaluate-target-health", @@ -492,6 +496,7 @@ func TestParseFlags(t *testing.T) { "EXTERNAL_DNS_AWS_EVALUATE_TARGET_HEALTH": "0", "EXTERNAL_DNS_AWS_API_RETRIES": "13", "EXTERNAL_DNS_AWS_PREFER_CNAME": "true", + "EXTERNAL_DNS_AWS_PROFILE": "profile1\nprofile2", "EXTERNAL_DNS_AWS_ZONES_CACHE_DURATION": "10s", "EXTERNAL_DNS_AWS_SD_SERVICE_CLEANUP": "true", "EXTERNAL_DNS_DYNAMODB_TABLE": "custom-table", diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 7b9dc77237..81ba1dd494 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -18,6 +18,7 @@ package aws import ( "context" + "errors" "fmt" "sort" "strconv" @@ -25,6 +26,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/request" "github.com/aws/aws-sdk-go/service/route53" log "github.com/sirupsen/logrus" @@ -35,7 +37,8 @@ import ( ) const ( - recordTTL = 300 + DefaultAWSProfile = "default" + recordTTL = 300 // From the experiments, it seems that the default MaxItems applied is 100, // and that, on the server side, there is a hard limit of 300 elements per page. // After a discussion with AWS representants, clients should accept @@ -211,6 +214,11 @@ type Route53Change struct { type Route53Changes []*Route53Change +type profiledZone struct { + profile string + zone *route53.HostedZone +} + func (cs Route53Changes) Route53Changes() []*route53.Change { ret := []*route53.Change{} for _, c := range cs { @@ -222,13 +230,13 @@ func (cs Route53Changes) Route53Changes() []*route53.Change { type zonesListCache struct { age time.Time duration time.Duration - zones map[string]*route53.HostedZone + zones map[string]*profiledZone } // AWSProvider is an implementation of Provider for AWS Route53. type AWSProvider struct { provider.BaseProvider - client Route53API + clients map[string]Route53API dryRun bool batchChangeSize int batchChangeSizeBytes int @@ -269,9 +277,9 @@ type AWSConfig struct { } // NewAWSProvider initializes a new AWS Route53 based Provider. -func NewAWSProvider(awsConfig AWSConfig, client Route53API) (*AWSProvider, error) { +func NewAWSProvider(awsConfig AWSConfig, clients map[string]Route53API) (*AWSProvider, error) { provider := &AWSProvider{ - client: client, + clients: clients, domainFilter: awsConfig.DomainFilter, zoneIDFilter: awsConfig.ZoneIDFilter, zoneTypeFilter: awsConfig.ZoneTypeFilter, @@ -293,14 +301,27 @@ func NewAWSProvider(awsConfig AWSConfig, client Route53API) (*AWSProvider, error // Zones returns the list of hosted zones. func (p *AWSProvider) Zones(ctx context.Context) (map[string]*route53.HostedZone, error) { + zones, err := p.zones(ctx) + if err != nil { + return nil, err + } + + result := make(map[string]*route53.HostedZone, len(zones)) + for id, zone := range zones { + result[id] = zone.zone + } + return result, nil +} + +func (p *AWSProvider) zones(ctx context.Context) (map[string]*profiledZone, error) { if p.zonesCache.zones != nil && time.Since(p.zonesCache.age) < p.zonesCache.duration { log.Debug("Using cached zones list") return p.zonesCache.zones, nil } log.Debug("Refreshing zones list cache") - zones := make(map[string]*route53.HostedZone) - + zones := make(map[string]*profiledZone) + var profile string var tagErr error f := func(resp *route53.ListHostedZonesOutput, lastPage bool) (shouldContinue bool) { for _, zone := range resp.HostedZones { @@ -323,7 +344,7 @@ func (p *AWSProvider) Zones(ctx context.Context) (map[string]*route53.HostedZone // Only fetch tags if a tag filter was specified if !p.zoneTagFilter.IsEmpty() { - tags, err := p.tagsForZone(ctx, *zone.Id) + tags, err := p.tagsForZone(ctx, *zone.Id, profile) if err != nil { tagErr = err return false @@ -333,22 +354,40 @@ func (p *AWSProvider) Zones(ctx context.Context) (map[string]*route53.HostedZone } } - zones[aws.StringValue(zone.Id)] = zone + zones[aws.StringValue(zone.Id)] = &profiledZone{ + profile: profile, + zone: zone, + } } return true } - err := p.client.ListHostedZonesPagesWithContext(ctx, &route53.ListHostedZonesInput{}, f) - if err != nil { - return nil, provider.NewSoftError(fmt.Errorf("failed to list hosted zones: %w", err)) - } - if tagErr != nil { - return nil, provider.NewSoftError(fmt.Errorf("failed to list zones tags: %w", tagErr)) + for p, client := range p.clients { + profile = p + err := client.ListHostedZonesPagesWithContext(ctx, &route53.ListHostedZonesInput{}, f) + if err != nil { + var awsErr awserr.Error + if errors.As(err, &awsErr) { + if awsErr.Code() == "AccessDenied" { + log.Warnf("Skipping profile %q due to missing permission: %v", profile, awsErr.Message()) + continue + } + if awsErr.Code() == "InvalidClientTokenId" || awsErr.Code() == "ExpiredToken" || awsErr.Code() == "SignatureDoesNotMatch" { + log.Warnf("Skipping profile %q due to credential issues: %v", profile, awsErr.Message()) + continue + } + } + return nil, provider.NewSoftError(fmt.Errorf("failed to list hosted zones: %w", err)) + } + if tagErr != nil { + return nil, provider.NewSoftError(fmt.Errorf("failed to list zones tags: %w", tagErr)) + } + } for _, zone := range zones { - log.Debugf("Considering zone: %s (domain: %s)", aws.StringValue(zone.Id), aws.StringValue(zone.Name)) + log.Debugf("Considering zone: %s (domain: %s)", aws.StringValue(zone.zone.Id), aws.StringValue(zone.zone.Name)) } if p.zonesCache.duration > time.Duration(0) { @@ -367,7 +406,7 @@ func wildcardUnescape(s string) string { // Records returns the list of records in a given hosted zone. func (p *AWSProvider) Records(ctx context.Context) (endpoints []*endpoint.Endpoint, _ error) { - zones, err := p.Zones(ctx) + zones, err := p.zones(ctx) if err != nil { return nil, provider.NewSoftError(fmt.Errorf("records retrieval failed: %w", err)) } @@ -375,7 +414,7 @@ func (p *AWSProvider) Records(ctx context.Context) (endpoints []*endpoint.Endpoi return p.records(ctx, zones) } -func (p *AWSProvider) records(ctx context.Context, zones map[string]*route53.HostedZone) ([]*endpoint.Endpoint, error) { +func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZone) ([]*endpoint.Endpoint, error) { endpoints := make([]*endpoint.Endpoint, 0) f := func(resp *route53.ListResourceRecordSetsOutput, lastPage bool) (shouldContinue bool) { for _, r := range resp.ResourceRecordSets { @@ -456,12 +495,12 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*route53.Hos for _, z := range zones { params := &route53.ListResourceRecordSetsInput{ - HostedZoneId: z.Id, + HostedZoneId: z.zone.Id, MaxItems: aws.String(route53PageSize), } if err := p.client.ListResourceRecordSetsPagesWithContext(ctx, params, f); err != nil { - return nil, provider.NewSoftError(fmt.Errorf("failed to list resource records sets for zone %s: %w", *z.Id, err)) + return nil, errors.Wrapf(err, "failed to list resource records sets for zone %s", *z.Id) } } @@ -544,7 +583,7 @@ func (p *AWSProvider) GetDomainFilter() endpoint.DomainFilter { // ApplyChanges applies a given set of changes in a given zone. func (p *AWSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { - zones, err := p.Zones(ctx) + zones, err := p.zones(ctx) if err != nil { return provider.NewSoftError(fmt.Errorf("failed to list zones, not applying changes: %w", err)) } @@ -560,7 +599,7 @@ func (p *AWSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e } // submitChanges takes a zone and a collection of Changes and sends them as a single transaction. -func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, zones map[string]*route53.HostedZone) error { +func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, zones map[string]*profiledZone) error { // return early if there is nothing to change if len(changes) == 0 { log.Info("All records are already up to date") @@ -602,8 +641,9 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, successfulChanges := 0 - if _, err := p.client.ChangeResourceRecordSetsWithContext(ctx, params); err != nil { - log.Errorf("Failure in zone %s [Id: %s] when submitting change batch: %v", aws.StringValue(zones[z].Name), z, err) + client := p.clients[zones[z].profile] + if _, err := client.ChangeResourceRecordSetsWithContext(ctx, params); err != nil { + log.Errorf("Failure in zone %s [Id: %s] when submitting change batch: %v", aws.StringValue(zones[z].zone.Name), z, err) changesByOwnership := groupChangesByNameAndOwnershipRelation(b) @@ -617,7 +657,7 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, params.ChangeBatch = &route53.ChangeBatch{ Changes: changes.Route53Changes(), } - if _, err := p.client.ChangeResourceRecordSetsWithContext(ctx, params); err != nil { + if _, err := client.ChangeResourceRecordSetsWithContext(ctx, params); err != nil { failedUpdate = true log.Errorf("Failed submitting change (error: %v), it will be retried in a separate change batch in the next iteration", err) p.failedChangesQueue[z] = append(p.failedChangesQueue[z], changes...) @@ -634,7 +674,7 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, if successfulChanges > 0 { // z is the R53 Hosted Zone ID already as aws.StringValue - log.Infof("%d record(s) in zone %s [Id: %s] were successfully updated", successfulChanges, aws.StringValue(zones[z].Name), z) + log.Infof("%d record(s) in zone %s [Id: %s] were successfully updated", successfulChanges, aws.StringValue(zones[z].zone.Name), z) } if i != len(batchCs)-1 { @@ -869,8 +909,10 @@ func groupChangesByNameAndOwnershipRelation(cs Route53Changes) map[string]Route5 return changesByOwnership } -func (p *AWSProvider) tagsForZone(ctx context.Context, zoneID string) (map[string]string, error) { - response, err := p.client.ListTagsForResourceWithContext(ctx, &route53.ListTagsForResourceInput{ +func (p *AWSProvider) tagsForZone(ctx context.Context, zoneID string, profile string) (map[string]string, error) { + client := p.clients[profile] + + response, err := client.ListTagsForResourceWithContext(ctx, &route53.ListTagsForResourceInput{ ResourceType: aws.String("hostedzone"), ResourceId: aws.String(zoneID), }) @@ -977,11 +1019,11 @@ func sortChangesByActionNameType(cs Route53Changes) Route53Changes { } // changesByZone separates a multi-zone change into a single change per zone. -func changesByZone(zones map[string]*route53.HostedZone, changeSet Route53Changes) map[string]Route53Changes { +func changesByZone(zones map[string]*profiledZone, changeSet Route53Changes) map[string]Route53Changes { changes := make(map[string]Route53Changes) for _, z := range zones { - changes[aws.StringValue(z.Id)] = Route53Changes{} + changes[aws.StringValue(z.zone.Id)] = Route53Changes{} } for _, c := range changeSet { @@ -998,7 +1040,7 @@ func changesByZone(zones map[string]*route53.HostedZone, changeSet Route53Change // if it's not, this will fail rrset := *c.ResourceRecordSet aliasTarget := *rrset.AliasTarget - aliasTarget.HostedZoneId = aws.String(cleanZoneID(aws.StringValue(z.Id))) + aliasTarget.HostedZoneId = aws.String(cleanZoneID(aws.StringValue(z.zone.Id))) rrset.AliasTarget = &aliasTarget c = &Route53Change{ Change: route53.Change{ @@ -1007,8 +1049,8 @@ func changesByZone(zones map[string]*route53.HostedZone, changeSet Route53Change }, } } - changes[aws.StringValue(z.Id)] = append(changes[aws.StringValue(z.Id)], c) - log.Debugf("Adding %s to zone %s [Id: %s]", hostname, aws.StringValue(z.Name), aws.StringValue(z.Id)) + changes[aws.StringValue(z.zone.Id)] = append(changes[aws.StringValue(z.zone.Id)], c) + log.Debugf("Adding %s to zone %s [Id: %s]", hostname, aws.StringValue(z.zone.Name), aws.StringValue(z.zone.Id)) } } @@ -1025,15 +1067,15 @@ func changesByZone(zones map[string]*route53.HostedZone, changeSet Route53Change // suitableZones returns all suitable private zones and the most suitable public zone // // for a given hostname and a set of zones. -func suitableZones(hostname string, zones map[string]*route53.HostedZone) []*route53.HostedZone { - var matchingZones []*route53.HostedZone - var publicZone *route53.HostedZone +func suitableZones(hostname string, zones map[string]*profiledZone) []*profiledZone { + var matchingZones []*profiledZone + var publicZone *profiledZone for _, z := range zones { - if aws.StringValue(z.Name) == hostname || strings.HasSuffix(hostname, "."+aws.StringValue(z.Name)) { - if z.Config == nil || !aws.BoolValue(z.Config.PrivateZone) { + if aws.StringValue(z.zone.Name) == hostname || strings.HasSuffix(hostname, "."+aws.StringValue(z.zone.Name)) { + if z.zone.Config == nil || !aws.BoolValue(z.zone.Config.PrivateZone) { // Only select the best matching public zone - if publicZone == nil || len(aws.StringValue(z.Name)) > len(aws.StringValue(publicZone.Name)) { + if publicZone == nil || len(aws.StringValue(z.zone.Name)) > len(aws.StringValue(publicZone.zone.Name)) { publicZone = z } } else { diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index ab753540af..09d0ebdb35 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -750,14 +750,14 @@ func TestAWSApplyChanges(t *testing.T) { ctx := tt.setup(provider) provider.zonesCache = &zonesListCache{duration: 0 * time.Minute} - counter := NewRoute53APICounter(provider.client) - provider.client = counter + counter := NewRoute53APICounter(provider.clients[DefaultAWSProfile]) + provider.clients[DefaultAWSProfile] = counter require.NoError(t, provider.ApplyChanges(ctx, changes)) assert.Equal(t, 1, counter.calls["ListHostedZonesPages"], tt.name) assert.Equal(t, tt.listRRSets, counter.calls["ListResourceRecordSetsPages"], tt.name) - validateRecords(t, listAWSRecords(t, provider.client, "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ + validateRecords(t, listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ { Name: aws.String("create-test.zone-1.ext-dns-test-2.teapot.zalan.do."), Type: aws.String(route53.RRTypeA), @@ -854,7 +854,7 @@ func TestAWSApplyChanges(t *testing.T) { ResourceRecords: []*route53.ResourceRecord{{Value: aws.String("10 mailhost1.foo.elb.amazonaws.com")}}, }, }) - validateRecords(t, listAWSRecords(t, provider.client, "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ + validateRecords(t, listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ { Name: aws.String("create-test.zone-2.ext-dns-test-2.teapot.zalan.do."), Type: aws.String(route53.RRTypeA), @@ -1023,8 +1023,8 @@ func TestAWSApplyChangesDryRun(t *testing.T) { validateRecords(t, append( - listAWSRecords(t, provider.client, "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), - listAWSRecords(t, provider.client, "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do.")...), + listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), + listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do.")...), originalRecords) } @@ -1064,23 +1064,35 @@ func TestAWSChangesByZones(t *testing.T) { }, } - zones := map[string]*route53.HostedZone{ - "foo-example-org": { - Id: aws.String("foo-example-org"), - Name: aws.String("foo.example.org."), + zones := map[string]*profiledZone{ + "foo-example-org": &profiledZone{ + profile: DefaultAWSProfile, + zone: &route53.HostedZone{ + Id: aws.String("foo-example-org"), + Name: aws.String("foo.example.org."), + }, }, - "bar-example-org": { - Id: aws.String("bar-example-org"), - Name: aws.String("bar.example.org."), + "bar-example-org": &profiledZone{ + profile: DefaultAWSProfile, + zone: &route53.HostedZone{ + Id: aws.String("bar-example-org"), + Name: aws.String("bar.example.org."), + }, }, - "bar-example-org-private": { - Id: aws.String("bar-example-org-private"), - Name: aws.String("bar.example.org."), - Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}, + "bar-example-org-private": &profiledZone{ + profile: DefaultAWSProfile, + zone: &route53.HostedZone{ + Id: aws.String("bar-example-org-private"), + Name: aws.String("bar.example.org."), + Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}, + }, }, - "baz-example-org": { - Id: aws.String("baz-example-org"), - Name: aws.String("baz.example.org."), + "baz-example-org": &profiledZone{ + profile: DefaultAWSProfile, + zone: &route53.HostedZone{ + Id: aws.String("baz-example-org"), + Name: aws.String("baz.example.org."), + }, }, } @@ -1161,7 +1173,7 @@ func TestAWSsubmitChanges(t *testing.T) { } ctx := context.Background() - zones, _ := provider.Zones(ctx) + zones, _ := provider.zones(ctx) records, _ := provider.Records(ctx) cs := make(Route53Changes, 0, len(endpoints)) cs = append(cs, provider.newChanges(route53.ChangeActionCreate, endpoints)...) @@ -1179,7 +1191,7 @@ func TestAWSsubmitChangesError(t *testing.T) { clientStub.MockMethod("ChangeResourceRecordSets", mock.Anything).Return(nil, fmt.Errorf("Mock route53 failure")) ctx := context.Background() - zones, err := provider.Zones(ctx) + zones, err := provider.zones(ctx) require.NoError(t, err) ep := endpoint.NewEndpointWithTTL("fail.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.0.0.1") @@ -1192,7 +1204,7 @@ func TestAWSsubmitChangesRetryOnError(t *testing.T) { provider, clientStub := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) ctx := context.Background() - zones, err := provider.Zones(ctx) + zones, err := provider.zones(ctx) require.NoError(t, err) ep1 := endpoint.NewEndpointWithTTL("success.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.0.0.1") @@ -1636,7 +1648,7 @@ func TestAWSCreateRecordsWithCNAME(t *testing.T) { Create: adjusted, })) - recordSets := listAWSRecords(t, provider.client, "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") + recordSets := listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") validateRecords(t, recordSets, []*route53.ResourceRecordSet{ { @@ -1700,7 +1712,7 @@ func TestAWSCreateRecordsWithALIAS(t *testing.T) { Create: adjusted, })) - recordSets := listAWSRecords(t, provider.client, "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") + recordSets := listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") validateRecords(t, recordSets, []*route53.ResourceRecordSet{ { @@ -1789,40 +1801,40 @@ func TestAWSCanonicalHostedZone(t *testing.T) { } func TestAWSSuitableZones(t *testing.T) { - zones := map[string]*route53.HostedZone{ + zones := map[string]*profiledZone{ // Public domain - "example-org": {Id: aws.String("example-org"), Name: aws.String("example.org.")}, + "example-org": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("example-org"), Name: aws.String("example.org.")}}, // Public subdomain - "bar-example-org": {Id: aws.String("bar-example-org"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(false)}}, + "bar-example-org": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("bar-example-org"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(false)}}}, // Public subdomain - "longfoo-bar-example-org": {Id: aws.String("longfoo-bar-example-org"), Name: aws.String("longfoo.bar.example.org.")}, + "longfoo-bar-example-org": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("longfoo-bar-example-org"), Name: aws.String("longfoo.bar.example.org.")}}, // Private domain - "example-org-private": {Id: aws.String("example-org-private"), Name: aws.String("example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}, + "example-org-private": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("example-org-private"), Name: aws.String("example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}}, // Private subdomain - "bar-example-org-private": {Id: aws.String("bar-example-org-private"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}, + "bar-example-org-private": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("bar-example-org-private"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}}, } for _, tc := range []struct { hostname string - expected []*route53.HostedZone + expected []*profiledZone }{ // bar.example.org is NOT suitable - {"foobar.example.org.", []*route53.HostedZone{zones["example-org-private"], zones["example-org"]}}, + {"foobar.example.org.", []*profiledZone{zones["example-org-private"], zones["example-org"]}}, // all matching private zones are suitable // https://github.com/kubernetes-sigs/external-dns/pull/356 - {"bar.example.org.", []*route53.HostedZone{zones["example-org-private"], zones["bar-example-org-private"], zones["bar-example-org"]}}, + {"bar.example.org.", []*profiledZone{zones["example-org-private"], zones["bar-example-org-private"], zones["bar-example-org"]}}, - {"foo.bar.example.org.", []*route53.HostedZone{zones["example-org-private"], zones["bar-example-org-private"], zones["bar-example-org"]}}, - {"foo.example.org.", []*route53.HostedZone{zones["example-org-private"], zones["example-org"]}}, + {"foo.bar.example.org.", []*profiledZone{zones["example-org-private"], zones["bar-example-org-private"], zones["bar-example-org"]}}, + {"foo.example.org.", []*profiledZone{zones["example-org-private"], zones["example-org"]}}, {"foo.kubernetes.io.", nil}, } { suitableZones := suitableZones(tc.hostname, zones) sort.Slice(suitableZones, func(i, j int) bool { - return *suitableZones[i].Id < *suitableZones[j].Id + return *suitableZones[i].zone.Id < *suitableZones[j].zone.Id }) sort.Slice(tc.expected, func(i, j int) bool { - return *tc.expected[i].Id < *tc.expected[j].Id + return *tc.expected[i].zone.Id < *tc.expected[j].zone.Id }) assert.Equal(t, tc.expected, suitableZones) } @@ -1835,7 +1847,7 @@ func createAWSZone(t *testing.T, provider *AWSProvider, zone *route53.HostedZone HostedZoneConfig: zone.Config, } - if _, err := provider.client.CreateHostedZoneWithContext(context.Background(), params); err != nil { + if _, err := provider.clients[DefaultAWSProfile].CreateHostedZoneWithContext(context.Background(), params); err != nil { require.EqualError(t, err, route53.ErrCodeHostedZoneAlreadyExists) } } @@ -1863,7 +1875,7 @@ func setAWSRecords(t *testing.T, provider *AWSProvider, records []*route53.Resou }) } - zones, err := provider.Zones(ctx) + zones, err := provider.zones(ctx) require.NoError(t, err) err = provider.submitChanges(ctx, changes, zones) require.NoError(t, err) @@ -1893,7 +1905,7 @@ func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilte client := NewRoute53APIStub(t) provider := &AWSProvider{ - client: client, + clients: map[string]Route53API{DefaultAWSProfile: client}, batchChangeSize: defaultBatchChangeSize, batchChangeSizeBytes: defaultBatchChangeSizeBytes, batchChangeSizeValues: defaultBatchChangeSizeValues, @@ -1933,7 +1945,7 @@ func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilte Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(false)}, }) - setupZoneTags(provider.client.(*Route53APIStub)) + setupZoneTags(provider.clients[DefaultAWSProfile].(*Route53APIStub)) setAWSRecords(t, provider, records) diff --git a/provider/aws/session.go b/provider/aws/session.go index 822ab4aff2..f91e051ba2 100644 --- a/provider/aws/session.go +++ b/provider/aws/session.go @@ -35,6 +35,7 @@ type AWSSessionConfig struct { AssumeRole string AssumeRoleExternalID string APIRetries int + Profile string } func NewSession(awsConfig AWSSessionConfig) (*session.Session, error) { @@ -52,6 +53,7 @@ func NewSession(awsConfig AWSSessionConfig) (*session.Session, error) { session, err := session.NewSessionWithOptions(session.Options{ Config: *config, SharedConfigState: session.SharedConfigEnable, + Profile: awsConfig.Profile, }) if err != nil { return nil, fmt.Errorf("instantiating AWS session: %w", err) From a3426d6c44cfa947c62c5d60c50da205dc9aeec8 Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Thu, 5 Oct 2023 21:24:04 +0200 Subject: [PATCH 02/11] Fix linter issues --- provider/aws/aws.go | 1 - 1 file changed, 1 deletion(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 81ba1dd494..c105aa7f71 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -383,7 +383,6 @@ func (p *AWSProvider) zones(ctx context.Context) (map[string]*profiledZone, erro if tagErr != nil { return nil, provider.NewSoftError(fmt.Errorf("failed to list zones tags: %w", tagErr)) } - } for _, zone := range zones { From 38621aafc3b6fb277f8926bdad1b57e00b6bf2f6 Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Fri, 29 Dec 2023 10:43:11 +0100 Subject: [PATCH 03/11] Rebase adaptions --- provider/aws/aws_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 09d0ebdb35..d39e71e6e7 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -1065,21 +1065,21 @@ func TestAWSChangesByZones(t *testing.T) { } zones := map[string]*profiledZone{ - "foo-example-org": &profiledZone{ + "foo-example-org": { profile: DefaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("foo-example-org"), Name: aws.String("foo.example.org."), }, }, - "bar-example-org": &profiledZone{ + "bar-example-org": { profile: DefaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("bar-example-org"), Name: aws.String("bar.example.org."), }, }, - "bar-example-org-private": &profiledZone{ + "bar-example-org-private": { profile: DefaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("bar-example-org-private"), @@ -1087,7 +1087,7 @@ func TestAWSChangesByZones(t *testing.T) { Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}, }, }, - "baz-example-org": &profiledZone{ + "baz-example-org": { profile: DefaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("baz-example-org"), From d2985175393b54a3a1adcced89cbcad40dd61fe1 Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Fri, 29 Dec 2023 10:45:00 +0100 Subject: [PATCH 04/11] add AWS profile aspect to documentation --- docs/tutorials/aws.md | 10 +++++++++- provider/aws/aws.go | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/tutorials/aws.md b/docs/tutorials/aws.md index a1ea9113a9..665d116442 100644 --- a/docs/tutorials/aws.md +++ b/docs/tutorials/aws.md @@ -1,6 +1,6 @@ # Setting up ExternalDNS for Services on AWS -This tutorial describes how to setup ExternalDNS for usage within a Kubernetes cluster on AWS. Make sure to use **>=0.11.0** version of ExternalDNS for this tutorial +This tutorial describes how to setup ExternalDNS for usage within a Kubernetes cluster on AWS. Make sure to use **>=0.15.0** version of ExternalDNS for this tutorial ## IAM Policy @@ -230,6 +230,14 @@ kubectl create secret generic external-dns \ Follow the steps under [Deploy ExternalDNS](#deploy-externaldns) using either RBAC or non-RBAC. Make sure to uncomment the section that mounts volumes, so that the credentials can be mounted. +> [!TIP] +> By default ExternalDNS takes the profile named `default` from the credentials file. If you want to use a different +> profile, you can set the environment variable `EXTERNAL_DNS_AWS_PROFILE` to the desired profile name or use the +> `--aws-profile` command line argument. It is even possible to use more than one profile at ones, separated by space in +> the environment variable `EXTERNAL_DNS_AWS_PROFILE` or by using `--aws-profile` multiple times. In this case +> ExternalDNS looks for the hosted zones in all profiles and keeps maintaining a mapping table between zone and profile +> in order to be able to modify the zones in the correct profile. + ### IAM Roles for Service Accounts [IRSA](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html) ([IAM roles for Service Accounts](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html)) allows cluster operators to map AWS IAM Roles to Kubernetes Service Accounts. This essentially allows only ExternalDNS pods to access Route53 without exposing any static credentials. diff --git a/provider/aws/aws.go b/provider/aws/aws.go index c105aa7f71..6f0ea0ed63 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -370,11 +370,11 @@ func (p *AWSProvider) zones(ctx context.Context) (map[string]*profiledZone, erro var awsErr awserr.Error if errors.As(err, &awsErr) { if awsErr.Code() == "AccessDenied" { - log.Warnf("Skipping profile %q due to missing permission: %v", profile, awsErr.Message()) + log.Warnf("Skipping AWS profile %q due to missing permission: %v", profile, awsErr.Message()) continue } if awsErr.Code() == "InvalidClientTokenId" || awsErr.Code() == "ExpiredToken" || awsErr.Code() == "SignatureDoesNotMatch" { - log.Warnf("Skipping profile %q due to credential issues: %v", profile, awsErr.Message()) + log.Warnf("Skipping AWS profile %q due to credential issues: %v", profile, awsErr.Message()) continue } } From 7fb0996bf0ef268a18db7fb71158eca998493edd Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Wed, 3 Jan 2024 10:20:46 +0100 Subject: [PATCH 05/11] fixing mloiseleur's findings --- provider/aws/aws.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 6f0ea0ed63..f7d51cd2ae 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -313,6 +313,7 @@ func (p *AWSProvider) Zones(ctx context.Context) (map[string]*route53.HostedZone return result, nil } +// zones returns the list of zones per AWS profile func (p *AWSProvider) zones(ctx context.Context) (map[string]*profiledZone, error) { if p.zonesCache.zones != nil && time.Since(p.zonesCache.age) < p.zonesCache.duration { log.Debug("Using cached zones list") @@ -613,6 +614,12 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, var failedZones []string for z, cs := range changesByZone { + log := log.WithFields(log.Fields{ + "zoneName": aws.StringValue(zones[z].zone.Name), + "zoneID": z, + "profile": zones[z].profile, + }) + var failedUpdate bool // group changes into new changes and into changes that failed in a previous iteration and are retried @@ -627,7 +634,7 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, } for _, c := range b { - log.Infof("Desired change: %s %s %s [Id: %s]", *c.Action, *c.ResourceRecordSet.Name, *c.ResourceRecordSet.Type, z) + log.Infof("Desired change: %s %s %s", *c.Action, *c.ResourceRecordSet.Name, *c.ResourceRecordSet.Type) } if !p.dryRun { @@ -642,7 +649,7 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, client := p.clients[zones[z].profile] if _, err := client.ChangeResourceRecordSetsWithContext(ctx, params); err != nil { - log.Errorf("Failure in zone %s [Id: %s] when submitting change batch: %v", aws.StringValue(zones[z].zone.Name), z, err) + log.Errorf("Failure in zone %s when submitting change batch: %v", aws.StringValue(zones[z].zone.Name), err) changesByOwnership := groupChangesByNameAndOwnershipRelation(b) @@ -651,7 +658,7 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, for _, changes := range changesByOwnership { for _, c := range changes { - log.Debugf("Desired change: %s %s %s [Id: %s]", *c.Action, *c.ResourceRecordSet.Name, *c.ResourceRecordSet.Type, z) + log.Debugf("Desired change: %s %s %s", *c.Action, *c.ResourceRecordSet.Name, *c.ResourceRecordSet.Type) } params.ChangeBatch = &route53.ChangeBatch{ Changes: changes.Route53Changes(), @@ -673,7 +680,7 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, if successfulChanges > 0 { // z is the R53 Hosted Zone ID already as aws.StringValue - log.Infof("%d record(s) in zone %s [Id: %s] were successfully updated", successfulChanges, aws.StringValue(zones[z].zone.Name), z) + log.Infof("%d record(s) were successfully updated", successfulChanges) } if i != len(batchCs)-1 { From 3ac5a8eded13288642171a0fd68b2439cb88a407 Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Wed, 3 Jan 2024 11:03:31 +0100 Subject: [PATCH 06/11] fixing mloiseleur's findings --- provider/aws/aws.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index f7d51cd2ae..088dfe9c3a 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -499,8 +499,9 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon MaxItems: aws.String(route53PageSize), } - if err := p.client.ListResourceRecordSetsPagesWithContext(ctx, params, f); err != nil { - return nil, errors.Wrapf(err, "failed to list resource records sets for zone %s", *z.Id) + client := p.clients[z.profile] + if err := client.ListResourceRecordSetsPagesWithContext(ctx, params, f); err != nil { + return nil, errors.Wrapf(err, "failed to list resource records sets for zone %s using aws profile %q", *z.zone.Id, z.profile) } } From 2059367ef4921f78429528954eba3b160bf0ed13 Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Sun, 28 Jan 2024 15:34:25 +0100 Subject: [PATCH 07/11] fixing szuecs's findings --- main.go | 54 ++++------------------------------------- provider/aws/aws.go | 10 +++++--- provider/aws/session.go | 50 +++++++++++++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 54 deletions(-) diff --git a/main.go b/main.go index 74e6ad632f..8948ff19af 100644 --- a/main.go +++ b/main.go @@ -26,7 +26,6 @@ import ( "time" awsSDK "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/dynamodb" "github.com/aws/aws-sdk-go/service/route53" sd "github.com/aws/aws-sdk-go/service/servicediscovery" @@ -195,50 +194,6 @@ func main() { zoneTypeFilter := provider.NewZoneTypeFilter(cfg.AWSZoneType) zoneTagFilter := provider.NewZoneTagFilter(cfg.AWSZoneTagFilter) - awsSessions := make(map[string]*session.Session, len(cfg.AWSProfiles)) - var awsDefaultSession *session.Session - if cfg.Provider == "aws" || cfg.Provider == "aws-sd" || cfg.Registry == "dynamodb" { - if len(cfg.AWSProfiles) == 0 || (len(cfg.AWSProfiles) == 1 && cfg.AWSProfiles[0] == "") { - session, err := aws.NewSession( - aws.AWSSessionConfig{ - AssumeRole: cfg.AWSAssumeRole, - AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, - APIRetries: cfg.AWSAPIRetries, - }, - ) - if err != nil { - log.Fatal(err) - } - awsSessions[aws.DefaultAWSProfile] = session - } else { - for _, profile := range cfg.AWSProfiles { - session, err := aws.NewSession( - aws.AWSSessionConfig{ - AssumeRole: cfg.AWSAssumeRole, - AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, - APIRetries: cfg.AWSAPIRetries, - Profile: profile, - }, - ) - if err != nil { - log.Fatal(err) - } - awsSessions[profile] = session - } - } - - awsDefaultSession, err = aws.NewSession( - aws.AWSSessionConfig{ - AssumeRole: cfg.AWSAssumeRole, - AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, - APIRetries: cfg.AWSAPIRetries, - }, - ) - if err != nil { - log.Fatal(err) - } - } - var p provider.Provider switch cfg.Provider { case "akamai": @@ -257,8 +212,9 @@ func main() { case "alibabacloud": p, err = alibabacloud.NewAlibabaCloudProvider(cfg.AlibabaCloudConfigFile, domainFilter, zoneIDFilter, cfg.AlibabaCloudZoneType, cfg.DryRun) case "aws": - clients := make(map[string]aws.Route53API, len(awsSessions)) - for profile, session := range awsSessions { + sessions := aws.CreateSessions(cfg) + clients := make(map[string]aws.Route53API, len(sessions)) + for profile, session := range sessions { clients[profile] = route53.New(session) } @@ -286,7 +242,7 @@ func main() { log.Infof("Registry \"%s\" cannot be used with AWS Cloud Map. Switching to \"aws-sd\".", cfg.Registry) cfg.Registry = "aws-sd" } - p, err = awssd.NewAWSSDProvider(domainFilter, cfg.AWSZoneType, cfg.DryRun, cfg.AWSSDServiceCleanup, cfg.TXTOwnerID, sd.New(awsDefaultSession)) + p, err = awssd.NewAWSSDProvider(domainFilter, cfg.AWSZoneType, cfg.DryRun, cfg.AWSSDServiceCleanup, cfg.TXTOwnerID, sd.New(aws.CreateDefaultSession(cfg))) case "azure-dns", "azure": p, err = azure.NewAzureProvider(cfg.AzureConfigFile, domainFilter, zoneNameFilter, zoneIDFilter, cfg.AzureSubscriptionID, cfg.AzureResourceGroup, cfg.AzureUserAssignedIdentityClientID, cfg.DryRun) case "azure-private-dns": @@ -473,7 +429,7 @@ func main() { if cfg.AWSDynamoDBRegion != "" { config = config.WithRegion(cfg.AWSDynamoDBRegion) } - r, err = registry.NewDynamoDBRegistry(p, cfg.TXTOwnerID, dynamodb.New(awsDefaultSession, config), cfg.AWSDynamoDBTable, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, []byte(cfg.TXTEncryptAESKey), cfg.TXTCacheInterval) + r, err = registry.NewDynamoDBRegistry(p, cfg.TXTOwnerID, dynamodb.New(aws.CreateDefaultSession(cfg), config), cfg.AWSDynamoDBTable, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, []byte(cfg.TXTEncryptAESKey), cfg.TXTCacheInterval) case "noop": r, err = registry.NewNoopRegistry(p) case "txt": diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 088dfe9c3a..e383280824 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -370,13 +370,15 @@ func (p *AWSProvider) zones(ctx context.Context) (map[string]*profiledZone, erro if err != nil { var awsErr awserr.Error if errors.As(err, &awsErr) { - if awsErr.Code() == "AccessDenied" { + switch awsErr.Code() { + case "AccessDenied": log.Warnf("Skipping AWS profile %q due to missing permission: %v", profile, awsErr.Message()) continue - } - if awsErr.Code() == "InvalidClientTokenId" || awsErr.Code() == "ExpiredToken" || awsErr.Code() == "SignatureDoesNotMatch" { + case "InvalidClientTokenId", "ExpiredToken", "SignatureDoesNotMatch": log.Warnf("Skipping AWS profile %q due to credential issues: %v", profile, awsErr.Message()) continue + default: + // noting to do here. Falling through to general error handling } } return nil, provider.NewSoftError(fmt.Errorf("failed to list hosted zones: %w", err)) @@ -501,7 +503,7 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon client := p.clients[z.profile] if err := client.ListResourceRecordSetsPagesWithContext(ctx, params, f); err != nil { - return nil, errors.Wrapf(err, "failed to list resource records sets for zone %s using aws profile %q", *z.zone.Id, z.profile) + return nil, fmt.Errorf("failed to list resource records sets for zone %s using aws profile %q: %w", *z.zone.Id, z.profile, err) } } diff --git a/provider/aws/session.go b/provider/aws/session.go index f91e051ba2..e788ceda1f 100644 --- a/provider/aws/session.go +++ b/provider/aws/session.go @@ -38,7 +38,55 @@ type AWSSessionConfig struct { Profile string } -func NewSession(awsConfig AWSSessionConfig) (*session.Session, error) { +func CreateDefaultSession(cfg *externaldns.Config) *session.Session { + result, err := newSession( + AWSSessionConfig{ + AssumeRole: cfg.AWSAssumeRole, + AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, + APIRetries: cfg.AWSAPIRetries, + }, + ) + if err != nil { + logrus.Fatal(err) + } + return result +} + +func CreateSessions(cfg *externaldns.Config) map[string]*session.Session { + result := make(map[string]*session.Session) + + if len(cfg.AWSProfiles) == 0 || (len(cfg.AWSProfiles) == 1 && cfg.AWSProfiles[0] == "") { + session, err := newSession( + AWSSessionConfig{ + AssumeRole: cfg.AWSAssumeRole, + AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, + APIRetries: cfg.AWSAPIRetries, + }, + ) + if err != nil { + logrus.Fatal(err) + } + result[DefaultAWSProfile] = session + } else { + for _, profile := range cfg.AWSProfiles { + session, err := newSession( + AWSSessionConfig{ + AssumeRole: cfg.AWSAssumeRole, + AssumeRoleExternalID: cfg.AWSAssumeRoleExternalID, + APIRetries: cfg.AWSAPIRetries, + Profile: profile, + }, + ) + if err != nil { + logrus.Fatal(err) + } + result[profile] = session + } + } + return result +} + +func newSession(awsConfig AWSSessionConfig) (*session.Session, error) { config := aws.NewConfig().WithMaxRetries(awsConfig.APIRetries) config.WithHTTPClient( From fd74c25661c08c11ee41d14a00653b78f3cd5c7e Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Tue, 5 Mar 2024 23:27:07 +0100 Subject: [PATCH 08/11] rebase --- provider/aws/aws.go | 2 +- provider/aws/aws_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index e383280824..93669df18d 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -279,7 +279,7 @@ type AWSConfig struct { // NewAWSProvider initializes a new AWS Route53 based Provider. func NewAWSProvider(awsConfig AWSConfig, clients map[string]Route53API) (*AWSProvider, error) { provider := &AWSProvider{ - clients: clients, + clients: clients, domainFilter: awsConfig.DomainFilter, zoneIDFilter: awsConfig.ZoneIDFilter, zoneTypeFilter: awsConfig.ZoneTypeFilter, diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index d39e71e6e7..0b898266c0 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -1905,7 +1905,7 @@ func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilte client := NewRoute53APIStub(t) provider := &AWSProvider{ - clients: map[string]Route53API{DefaultAWSProfile: client}, + clients: map[string]Route53API{DefaultAWSProfile: client}, batchChangeSize: defaultBatchChangeSize, batchChangeSizeBytes: defaultBatchChangeSizeBytes, batchChangeSizeValues: defaultBatchChangeSizeValues, From d3fb0eb03ab25067e90a84f30c49d344481df2bf Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Wed, 6 Mar 2024 10:00:41 +0100 Subject: [PATCH 09/11] add test for newSession() --- provider/aws/session_test.go | 74 ++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 provider/aws/session_test.go diff --git a/provider/aws/session_test.go b/provider/aws/session_test.go new file mode 100644 index 0000000000..206fcf940c --- /dev/null +++ b/provider/aws/session_test.go @@ -0,0 +1,74 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package aws + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_newSession(t *testing.T) { + t.Run("should use profile from credentials file", func(t *testing.T) { + // setup + credsFile, err := prepareCredentialsFile(t) + defer os.Remove(credsFile.Name()) + require.NoError(t, err) + os.Setenv("AWS_SHARED_CREDENTIALS_FILE", credsFile.Name()) + defer os.Unsetenv("AWS_SHARED_CREDENTIALS_FILE") + + // when + s, err := newSession(AWSSessionConfig{Profile: "profile2"}) + require.NoError(t, err) + creds, err := s.Config.Credentials.Get() + + // then + assert.NoError(t, err) + assert.Equal(t, "AKID2345", creds.AccessKeyID) + assert.Equal(t, "SECRET2", creds.SecretAccessKey) + }) + + t.Run("should respect env variables without profile", func(t *testing.T) { + // setup + os.Setenv("AWS_ACCESS_KEY_ID", "AKIAIOSFODNN7EXAMPLE") + os.Setenv("AWS_SECRET_ACCESS_KEY", "topsecret") + defer os.Unsetenv("AWS_ACCESS_KEY_ID") + defer os.Unsetenv("AWS_SECRET_ACCESS_KEY") + + // when + s, err := newSession(AWSSessionConfig{}) + require.NoError(t, err) + creds, err := s.Config.Credentials.Get() + + // then + assert.NoError(t, err) + assert.Equal(t, "AKIAIOSFODNN7EXAMPLE", creds.AccessKeyID) + assert.Equal(t, "topsecret", creds.SecretAccessKey) + }) +} + +func prepareCredentialsFile(t *testing.T) (*os.File, error) { + credsFile, err := os.CreateTemp("", "aws-*.creds") + require.NoError(t, err) + _, err = credsFile.WriteString("[profile1]\naws_access_key_id=AKID1234\naws_secret_access_key=SECRET1\n\n[profile2]\naws_access_key_id=AKID2345\naws_secret_access_key=SECRET2\n") + require.NoError(t, err) + err = credsFile.Close() + require.NoError(t, err) + return credsFile, err +} From 7ff4b3e8faf9d44b5f1e899da49024fa0bc11cce Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Sun, 9 Jun 2024 21:33:05 +0200 Subject: [PATCH 10/11] resolve SZUECS' findings --- provider/aws/aws.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 93669df18d..bd90a5c007 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -370,16 +370,11 @@ func (p *AWSProvider) zones(ctx context.Context) (map[string]*profiledZone, erro if err != nil { var awsErr awserr.Error if errors.As(err, &awsErr) { - switch awsErr.Code() { - case "AccessDenied": - log.Warnf("Skipping AWS profile %q due to missing permission: %v", profile, awsErr.Message()) + if awsErr.Code() == route53.ErrCodeThrottlingException { + log.Warnf("Skipping AWS profile %q due to provider side throttling: %v", profile, awsErr.Message()) continue - case "InvalidClientTokenId", "ExpiredToken", "SignatureDoesNotMatch": - log.Warnf("Skipping AWS profile %q due to credential issues: %v", profile, awsErr.Message()) - continue - default: - // noting to do here. Falling through to general error handling } + // nothing to do here. Falling through to general error handling } return nil, provider.NewSoftError(fmt.Errorf("failed to list hosted zones: %w", err)) } From 604a93670eefb9a2746929c62575617927a2b0ac Mon Sep 17 00:00:00 2001 From: Jan Roehrich Date: Mon, 10 Jun 2024 21:09:18 +0200 Subject: [PATCH 11/11] resolve SZUECS' findings --- provider/aws/aws.go | 2 +- provider/aws/aws_test.go | 40 ++++++++++++++++++++-------------------- provider/aws/session.go | 2 +- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index bd90a5c007..311ff7c52b 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -37,7 +37,7 @@ import ( ) const ( - DefaultAWSProfile = "default" + defaultAWSProfile = "default" recordTTL = 300 // From the experiments, it seems that the default MaxItems applied is 100, // and that, on the server side, there is a hard limit of 300 elements per page. diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 0b898266c0..418e05d09e 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -750,14 +750,14 @@ func TestAWSApplyChanges(t *testing.T) { ctx := tt.setup(provider) provider.zonesCache = &zonesListCache{duration: 0 * time.Minute} - counter := NewRoute53APICounter(provider.clients[DefaultAWSProfile]) - provider.clients[DefaultAWSProfile] = counter + counter := NewRoute53APICounter(provider.clients[defaultAWSProfile]) + provider.clients[defaultAWSProfile] = counter require.NoError(t, provider.ApplyChanges(ctx, changes)) assert.Equal(t, 1, counter.calls["ListHostedZonesPages"], tt.name) assert.Equal(t, tt.listRRSets, counter.calls["ListResourceRecordSetsPages"], tt.name) - validateRecords(t, listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ + validateRecords(t, listAWSRecords(t, provider.clients[defaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ { Name: aws.String("create-test.zone-1.ext-dns-test-2.teapot.zalan.do."), Type: aws.String(route53.RRTypeA), @@ -854,7 +854,7 @@ func TestAWSApplyChanges(t *testing.T) { ResourceRecords: []*route53.ResourceRecord{{Value: aws.String("10 mailhost1.foo.elb.amazonaws.com")}}, }, }) - validateRecords(t, listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ + validateRecords(t, listAWSRecords(t, provider.clients[defaultAWSProfile], "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do."), []*route53.ResourceRecordSet{ { Name: aws.String("create-test.zone-2.ext-dns-test-2.teapot.zalan.do."), Type: aws.String(route53.RRTypeA), @@ -1023,8 +1023,8 @@ func TestAWSApplyChangesDryRun(t *testing.T) { validateRecords(t, append( - listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), - listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do.")...), + listAWSRecords(t, provider.clients[defaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), + listAWSRecords(t, provider.clients[defaultAWSProfile], "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do.")...), originalRecords) } @@ -1066,21 +1066,21 @@ func TestAWSChangesByZones(t *testing.T) { zones := map[string]*profiledZone{ "foo-example-org": { - profile: DefaultAWSProfile, + profile: defaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("foo-example-org"), Name: aws.String("foo.example.org."), }, }, "bar-example-org": { - profile: DefaultAWSProfile, + profile: defaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("bar-example-org"), Name: aws.String("bar.example.org."), }, }, "bar-example-org-private": { - profile: DefaultAWSProfile, + profile: defaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("bar-example-org-private"), Name: aws.String("bar.example.org."), @@ -1088,7 +1088,7 @@ func TestAWSChangesByZones(t *testing.T) { }, }, "baz-example-org": { - profile: DefaultAWSProfile, + profile: defaultAWSProfile, zone: &route53.HostedZone{ Id: aws.String("baz-example-org"), Name: aws.String("baz.example.org."), @@ -1648,7 +1648,7 @@ func TestAWSCreateRecordsWithCNAME(t *testing.T) { Create: adjusted, })) - recordSets := listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") + recordSets := listAWSRecords(t, provider.clients[defaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") validateRecords(t, recordSets, []*route53.ResourceRecordSet{ { @@ -1712,7 +1712,7 @@ func TestAWSCreateRecordsWithALIAS(t *testing.T) { Create: adjusted, })) - recordSets := listAWSRecords(t, provider.clients[DefaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") + recordSets := listAWSRecords(t, provider.clients[defaultAWSProfile], "/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do.") validateRecords(t, recordSets, []*route53.ResourceRecordSet{ { @@ -1803,15 +1803,15 @@ func TestAWSCanonicalHostedZone(t *testing.T) { func TestAWSSuitableZones(t *testing.T) { zones := map[string]*profiledZone{ // Public domain - "example-org": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("example-org"), Name: aws.String("example.org.")}}, + "example-org": {profile: defaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("example-org"), Name: aws.String("example.org.")}}, // Public subdomain - "bar-example-org": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("bar-example-org"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(false)}}}, + "bar-example-org": {profile: defaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("bar-example-org"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(false)}}}, // Public subdomain - "longfoo-bar-example-org": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("longfoo-bar-example-org"), Name: aws.String("longfoo.bar.example.org.")}}, + "longfoo-bar-example-org": {profile: defaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("longfoo-bar-example-org"), Name: aws.String("longfoo.bar.example.org.")}}, // Private domain - "example-org-private": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("example-org-private"), Name: aws.String("example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}}, + "example-org-private": {profile: defaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("example-org-private"), Name: aws.String("example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}}, // Private subdomain - "bar-example-org-private": {profile: DefaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("bar-example-org-private"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}}, + "bar-example-org-private": {profile: defaultAWSProfile, zone: &route53.HostedZone{Id: aws.String("bar-example-org-private"), Name: aws.String("bar.example.org."), Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(true)}}}, } for _, tc := range []struct { @@ -1847,7 +1847,7 @@ func createAWSZone(t *testing.T, provider *AWSProvider, zone *route53.HostedZone HostedZoneConfig: zone.Config, } - if _, err := provider.clients[DefaultAWSProfile].CreateHostedZoneWithContext(context.Background(), params); err != nil { + if _, err := provider.clients[defaultAWSProfile].CreateHostedZoneWithContext(context.Background(), params); err != nil { require.EqualError(t, err, route53.ErrCodeHostedZoneAlreadyExists) } } @@ -1905,7 +1905,7 @@ func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilte client := NewRoute53APIStub(t) provider := &AWSProvider{ - clients: map[string]Route53API{DefaultAWSProfile: client}, + clients: map[string]Route53API{defaultAWSProfile: client}, batchChangeSize: defaultBatchChangeSize, batchChangeSizeBytes: defaultBatchChangeSizeBytes, batchChangeSizeValues: defaultBatchChangeSizeValues, @@ -1945,7 +1945,7 @@ func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilte Config: &route53.HostedZoneConfig{PrivateZone: aws.Bool(false)}, }) - setupZoneTags(provider.clients[DefaultAWSProfile].(*Route53APIStub)) + setupZoneTags(provider.clients[defaultAWSProfile].(*Route53APIStub)) setAWSRecords(t, provider, records) diff --git a/provider/aws/session.go b/provider/aws/session.go index e788ceda1f..da578b2927 100644 --- a/provider/aws/session.go +++ b/provider/aws/session.go @@ -66,7 +66,7 @@ func CreateSessions(cfg *externaldns.Config) map[string]*session.Session { if err != nil { logrus.Fatal(err) } - result[DefaultAWSProfile] = session + result[defaultAWSProfile] = session } else { for _, profile := range cfg.AWSProfiles { session, err := newSession(