Add aws max change count flag (#596)

* Create `NewAWSProvider` with `AWSConfig` struct

Rather than calling `NewAWSProvider` with a list of objects, you will
now call it using a new `AWSConfig` struct. This allows for clearer
declarations of variables which becomes even  more important as more
variables are added.

* Add `aws-max-change-count` flag

Adding a new `aws-max-change-count` flag to override the default max
change count on the aws provider.

Included updated tests with a new `defaultMaxChangeCount` constant and
tests for setting the value as a flag and as an environment variable.

* Update CHANGELOG.md

Updating CHANGELOG.md with 'Add aws max change count flag' PR.

CHANGELOG.md

@@ -1,3 +1,5 @@
+  - AWS: Add a flag to override the default max change count (#596) @peterbale
+  
 ## v0.5.3 - 2018-06-15
 
   - Print a message if no hosted zones match (aws provider) (#592) @svend

main.go

@@ -95,7 +95,16 @@ func main() {
 	var p provider.Provider
 	switch cfg.Provider {
 	case "aws":
-		p, err = provider.NewAWSProvider(domainFilter, zoneIDFilter, zoneTypeFilter, cfg.AWSAssumeRole, cfg.DryRun)
+		p, err = provider.NewAWSProvider(
+			provider.AWSConfig{
+				DomainFilter:   domainFilter,
+				ZoneIDFilter:   zoneIDFilter,
+				ZoneTypeFilter: zoneTypeFilter,
+				MaxChangeCount: cfg.AWSMaxChangeCount,
+				AssumeRole:     cfg.AWSAssumeRole,
+				DryRun:         cfg.DryRun,
+			},
+		)
 	case "aws-sd":
 		// Check that only compatible Registry is used with AWS-SD
 		if cfg.Registry != "noop" && cfg.Registry != "aws-sd" {

pkg/apis/externaldns/types.go

@@ -52,6 +52,7 @@ type Config struct {
 	ZoneIDFilter             []string
 	AWSZoneType              string
 	AWSAssumeRole            string
+	AWSMaxChangeCount        int
 	AzureConfigFile          string
 	AzureResourceGroup       string
 	CloudflareProxied        bool
@@ -97,6 +98,7 @@ var defaultConfig = &Config{
 	DomainFilter:             []string{},
 	AWSZoneType:              "",
 	AWSAssumeRole:            "",
+	AWSMaxChangeCount:        4000,
 	AzureConfigFile:          "/etc/kubernetes/azure.json",
 	AzureResourceGroup:       "",
 	CloudflareProxied:        false,
@@ -176,6 +178,7 @@ func (cfg *Config) ParseFlags(args []string) error {
 	app.Flag("google-project", "When using the Google provider, current project is auto-detected, when running on GCP. Specify other project with this. Must be specified when running outside GCP.").Default(defaultConfig.GoogleProject).StringVar(&cfg.GoogleProject)
 	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-assume-role", "When using the AWS provider, 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-max-change-count", "When using the AWS provider, set the maximum number of changes that will be applied.").Default(strconv.Itoa(defaultConfig.AWSMaxChangeCount)).IntVar(&cfg.AWSMaxChangeCount)
 	app.Flag("azure-config-file", "When using the Azure provider, specify the Azure configuration file (required when --provider=azure").Default(defaultConfig.AzureConfigFile).StringVar(&cfg.AzureConfigFile)
 	app.Flag("azure-resource-group", "When using the Azure provider, override the Azure resource group to use (optional)").Default(defaultConfig.AzureResourceGroup).StringVar(&cfg.AzureResourceGroup)
 	app.Flag("cloudflare-proxied", "When using the Cloudflare provider, specify if the proxy mode must be enabled (default: disabled)").BoolVar(&cfg.CloudflareProxied)

pkg/apis/externaldns/types_test.go

@@ -41,6 +41,7 @@ var (
 		ZoneIDFilter:          []string{""},
 		AWSZoneType:           "",
 		AWSAssumeRole:         "",
+		AWSMaxChangeCount:     4000,
 		AzureConfigFile:       "/etc/kubernetes/azure.json",
 		AzureResourceGroup:    "",
 		CloudflareProxied:     false,
@@ -80,6 +81,7 @@ var (
 		ZoneIDFilter:          []string{"/hostedzone/ZTST1", "/hostedzone/ZTST2"},
 		AWSZoneType:           "private",
 		AWSAssumeRole:         "some-other-role",
+		AWSMaxChangeCount:     100,
 		AzureConfigFile:       "azure.json",
 		AzureResourceGroup:    "arg",
 		CloudflareProxied:     true,
@@ -155,6 +157,7 @@ func TestParseFlags(t *testing.T) {
 				"--zone-id-filter=/hostedzone/ZTST2",
 				"--aws-zone-type=private",
 				"--aws-assume-role=some-other-role",
+				"--aws-max-change-count=100",
 				"--policy=upsert-only",
 				"--registry=noop",
 				"--txt-owner-id=owner-1",
@@ -199,6 +202,7 @@ func TestParseFlags(t *testing.T) {
 				"EXTERNAL_DNS_ZONE_ID_FILTER":          "/hostedzone/ZTST1\n/hostedzone/ZTST2",
 				"EXTERNAL_DNS_AWS_ZONE_TYPE":           "private",
 				"EXTERNAL_DNS_AWS_ASSUME_ROLE":         "some-other-role",
+				"EXTERNAL_DNS_AWS_MAX_CHANGE_COUNT":    "100",
 				"EXTERNAL_DNS_POLICY":                  "upsert-only",
 				"EXTERNAL_DNS_REGISTRY":                "noop",
 				"EXTERNAL_DNS_TXT_OWNER_ID":            "owner-1",

provider/aws.go

@@ -33,7 +33,6 @@ import (
 const (
 	evaluateTargetHealth = true
 	recordTTL            = 300
-	maxChangeCount       = 4000
 )
 
 var (
@@ -86,8 +85,9 @@ type Route53API interface {
 
 // AWSProvider is an implementation of Provider for AWS Route53.
 type AWSProvider struct {
-	client Route53API
-	dryRun bool
+	client         Route53API
+	dryRun         bool
+	maxChangeCount int
 	// only consider hosted zones managing domains ending in this suffix
 	domainFilter DomainFilter
 	// filter hosted zones by id
@@ -96,8 +96,18 @@ type AWSProvider struct {
 	zoneTypeFilter ZoneTypeFilter
 }
 
+// AWSConfig contains configuration to create a new AWS provider.
+type AWSConfig struct {
+	DomainFilter   DomainFilter
+	ZoneIDFilter   ZoneIDFilter
+	ZoneTypeFilter ZoneTypeFilter
+	MaxChangeCount int
+	AssumeRole     string
+	DryRun         bool
+}
+
 // NewAWSProvider initializes a new AWS Route53 based Provider.
-func NewAWSProvider(domainFilter DomainFilter, zoneIDFilter ZoneIDFilter, zoneTypeFilter ZoneTypeFilter, assumeRole string, dryRun bool) (*AWSProvider, error) {
+func NewAWSProvider(awsConfig AWSConfig) (*AWSProvider, error) {
 	config := aws.NewConfig()
 
 	config.WithHTTPClient(
@@ -117,17 +127,18 @@ func NewAWSProvider(domainFilter DomainFilter, zoneIDFilter ZoneIDFilter, zoneTy
 		return nil, err
 	}
 
-	if assumeRole != "" {
-		log.Infof("Assuming role: %s", assumeRole)
-		session.Config.WithCredentials(stscreds.NewCredentials(session, assumeRole))
+	if awsConfig.AssumeRole != "" {
+		log.Infof("Assuming role: %s", awsConfig.AssumeRole)
+		session.Config.WithCredentials(stscreds.NewCredentials(session, awsConfig.AssumeRole))
 	}
 
 	provider := &AWSProvider{
 		client:         route53.New(session),
-		domainFilter:   domainFilter,
-		zoneIDFilter:   zoneIDFilter,
-		zoneTypeFilter: zoneTypeFilter,
-		dryRun:         dryRun,
+		domainFilter:   awsConfig.DomainFilter,
+		zoneIDFilter:   awsConfig.ZoneIDFilter,
+		zoneTypeFilter: awsConfig.ZoneTypeFilter,
+		maxChangeCount: awsConfig.MaxChangeCount,
+		dryRun:         awsConfig.DryRun,
 	}
 
 	return provider, nil
@@ -275,7 +286,7 @@ func (p *AWSProvider) submitChanges(changes []*route53.Change) error {
 	}
 
 	for z, cs := range changesByZone {
-		limCs := limitChangeSet(cs, maxChangeCount)
+		limCs := limitChangeSet(cs, p.maxChangeCount)
 
 		for _, c := range limCs {
 			log.Infof("Desired change: %s %s %s", *c.Action, *c.ResourceRecordSet.Name, *c.ResourceRecordSet.Type)

provider/aws_test.go

@@ -32,6 +32,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+const defaultMaxChangeCount = 4000
+
 // Compile time check for interface conformance
 var _ Route53API = &Route53APIStub{}
 
@@ -540,7 +542,7 @@ func TestAWSChangesByZones(t *testing.T) {
 func TestAWSsubmitChanges(t *testing.T) {
 	provider := newAWSProvider(t, NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), NewZoneIDFilter([]string{}), NewZoneTypeFilter(""), false, []*endpoint.Endpoint{})
 	const subnets = 16
-	const hosts = maxChangeCount / subnets
+	const hosts = defaultMaxChangeCount / subnets
 
 	endpoints := make([]*endpoint.Endpoint, 0)
 	for i := 0; i < subnets; i++ {
@@ -566,7 +568,7 @@ func TestAWSsubmitChanges(t *testing.T) {
 func TestAWSLimitChangeSet(t *testing.T) {
 	var cs []*route53.Change
 
-	for i := 1; i <= maxChangeCount; i += 2 {
+	for i := 1; i <= defaultMaxChangeCount; i += 2 {
 		cs = append(cs, &route53.Change{
 			Action: aws.String(route53.ChangeActionCreate),
 			ResourceRecordSet: &route53.ResourceRecordSet{
@@ -583,7 +585,7 @@ func TestAWSLimitChangeSet(t *testing.T) {
 		})
 	}
 
-	limCs := limitChangeSet(cs, maxChangeCount)
+	limCs := limitChangeSet(cs, defaultMaxChangeCount)
 
 	// sorting cs not needed as it should be returned as is
 	validateAWSChangeRecords(t, limCs, cs)
@@ -864,6 +866,7 @@ func newAWSProvider(t *testing.T, domainFilter DomainFilter, zoneIDFilter ZoneID
 
 	provider := &AWSProvider{
 		client:         client,
+		maxChangeCount: defaultMaxChangeCount,
 		domainFilter:   domainFilter,
 		zoneIDFilter:   zoneIDFilter,
 		zoneTypeFilter: zoneTypeFilter,