plugin/route53: make refresh frequency adjustable (#3083)

the current update frequency for the refresh loop in the route 53 plugin is hard-coded
to 1 minute. aws rate-limits the number of api requests so less frequent record refreshes
can help when reaching those limits depending upon your individual scenarios. this pull
adds a configuration option to the route53 plugin to adjust the refresh frequency.

thanks for getting my last pull released so quickly. this is the last local change that
i have been running and would love to get it contributed back to the project.

Signed-off-by: Matt Kulka <mkulka@parchment.com>

5 files changed, 65 insertions, 6 deletions

diff --git a/plugin/route53/README.md b/plugin/route53/README.md
index d492ae913..e704ced60 100644
--- a/plugin/route53/README.md
+++ b/plugin/route53/README.md
@@ -18,6 +18,7 @@ route53 [ZONE:HOSTED_ZONE_ID...] {
     aws_access_key [AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY]
     credentials PROFILE [FILENAME]
     fallthrough [ZONES...]
+    refresh DURATION
 }
 ~~~
 
@@ -48,6 +49,14 @@ route53 [ZONE:HOSTED_ZONE_ID...] {
 *   **ZONES** zones it should be authoritative for. If empty, the zones from the configuration
     block.
 
+*   `refresh` can be used to control how long between record retrievals from Route 53. It requires
+    a duration string as a parameter to specify the duration between update cycles. Each update
+    cycle may result in many AWS API calls depending on how many domains use this plugin and how
+    many records are in each. Adjusting the update frequency may help reduce the potential of API
+    rate-limiting imposed by AWS.
+
+*   **DURATION** A duration string. Defaults to `1m`. If units are unspecified, seconds are assumed.
+
 ## Examples
 
 Enable route53 with implicit AWS credentials and resolve CNAMEs via 10.0.0.1:
@@ -86,3 +95,12 @@ Enable route53 with multiple hosted zones with the same domain:
     route53 example.org.:Z1Z2Z3Z4DZ5Z6Z7 example.org.:Z93A52145678156
 }
 ~~~
+
+Enable route53 and refresh records every 3 minutes
+~~~ txt
+. {
+    route53 example.org.:Z1Z2Z3Z4DZ5Z6Z7 {
+      refresh 3m
+    }
+}
+~~~
diff --git a/plugin/route53/route53.go b/plugin/route53/route53.go
index 0f7c2b1d2..346defa66 100644
--- a/plugin/route53/route53.go
+++ b/plugin/route53/route53.go
@@ -31,6 +31,7 @@ type Route53 struct {
 	zoneNames []string
 	client    route53iface.Route53API
 	upstream  *upstream.Upstream
+	refresh   time.Duration
 
 	zMu   sync.RWMutex
 	zones zones
@@ -49,7 +50,7 @@ type zones map[string][]*zone
 // exist, and returns a new *Route53. In addition to this, upstream is passed
 // for doing recursive queries against CNAMEs.
 // Returns error if it cannot verify any given domain name/zone id pair.
-func New(ctx context.Context, c route53iface.Route53API, keys map[string][]string, up *upstream.Upstream) (*Route53, error) {
+func New(ctx context.Context, c route53iface.Route53API, keys map[string][]string, up *upstream.Upstream, refresh time.Duration) (*Route53, error) {
 	zones := make(map[string][]*zone, len(keys))
 	zoneNames := make([]string, 0, len(keys))
 	for dns, hostedZoneIDs := range keys {
@@ -72,6 +73,7 @@ func New(ctx context.Context, c route53iface.Route53API, keys map[string][]strin
 		zoneNames: zoneNames,
 		zones:     zones,
 		upstream:  up,
+		refresh:   refresh,
 	}, nil
 }
 
@@ -87,7 +89,7 @@ func (h *Route53) Run(ctx context.Context) error {
 			case <-ctx.Done():
 				log.Infof("Breaking out of Route53 update loop: %v", ctx.Err())
 				return
-			case <-time.After(1 * time.Minute):
+			case <-time.After(h.refresh):
 				if err := h.updateZones(ctx); err != nil && ctx.Err() == nil /* Don't log error if ctx expired. */ {
 					log.Errorf("Failed to update zones: %v", err)
 				}
@@ -248,7 +250,7 @@ func (h *Route53) updateZones(ctx context.Context) error {
 				newZ.Upstream = h.upstream
 				in := &route53.ListResourceRecordSetsInput{
 					HostedZoneId: aws.String(hostedZone.id),
-					MaxItems: aws.String("1000"),
+					MaxItems:     aws.String("1000"),
 				}
 				err = h.client.ListResourceRecordSetsPagesWithContext(ctx, in,
 					func(out *route53.ListResourceRecordSetsOutput, last bool) bool {
diff --git a/plugin/route53/route53_test.go b/plugin/route53/route53_test.go
index 5657e7c7a..64ea90d6a 100644
--- a/plugin/route53/route53_test.go
+++ b/plugin/route53/route53_test.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	"github.com/coredns/coredns/plugin/pkg/fall"
@@ -79,7 +80,7 @@ func (fakeRoute53) ListResourceRecordSetsPagesWithContext(_ aws.Context, in *rou
 func TestRoute53(t *testing.T) {
 	ctx := context.Background()
 
-	r, err := New(ctx, fakeRoute53{}, map[string][]string{"bad.": {"0987654321"}}, &upstream.Upstream{})
+	r, err := New(ctx, fakeRoute53{}, map[string][]string{"bad.": {"0987654321"}}, &upstream.Upstream{}, time.Duration(1) * time.Minute)
 	if err != nil {
 		t.Fatalf("Failed to create Route53: %v", err)
 	}
@@ -87,7 +88,7 @@ func TestRoute53(t *testing.T) {
 		t.Fatalf("Expected errors for zone bad.")
 	}
 
-	r, err = New(ctx, fakeRoute53{}, map[string][]string{"org.": {"1357986420", "1234567890"}, "gov.": {"Z098765432", "1234567890"}}, &upstream.Upstream{})
+	r, err = New(ctx, fakeRoute53{}, map[string][]string{"org.": {"1357986420", "1234567890"}, "gov.": {"Z098765432", "1234567890"}}, &upstream.Upstream{}, time.Duration(90) * time.Second)
 	if err != nil {
 		t.Fatalf("Failed to create Route53: %v", err)
 	}
diff --git a/plugin/route53/setup.go b/plugin/route53/setup.go
index 1872dce4e..918b0e56a 100644
--- a/plugin/route53/setup.go
+++ b/plugin/route53/setup.go
@@ -2,7 +2,10 @@ package route53
 
 import (
 	"context"
+	"fmt"
+	"strconv"
 	"strings"
+	"time"
 
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin"
@@ -53,6 +56,8 @@ func setup(c *caddy.Controller, f func(*credentials.Credentials) route53iface.Ro
 
 		up := upstream.New()
 
+		refresh := time.Duration(1) * time.Minute // default update frequency to 1 minute
+
 		args := c.RemainingArgs()
 
 		for i := 0; i < len(args); i++ {
@@ -98,6 +103,23 @@ func setup(c *caddy.Controller, f func(*credentials.Credentials) route53iface.Ro
 				}
 			case "fallthrough":
 				fall.SetZonesFromArgs(c.RemainingArgs())
+			case "refresh":
+				if c.NextArg() {
+					refreshStr := c.Val()
+					_, err := strconv.Atoi(refreshStr)
+					if err == nil {
+						refreshStr = fmt.Sprintf("%ss", c.Val())
+					}
+					refresh, err = time.ParseDuration(refreshStr)
+					if err != nil {
+						return c.Errf("Unable to parse duration: '%v'", err)
+					}
+					if refresh <= 0 {
+						return c.Errf("refresh interval must be greater than 0: %s", refreshStr)
+					}
+				} else {
+					return c.ArgErr()
+				}
 			default:
 				return c.Errf("unknown property '%s'", c.Val())
 			}
@@ -107,7 +129,7 @@ func setup(c *caddy.Controller, f func(*credentials.Credentials) route53iface.Ro
 		})
 		client := f(credentials.NewChainCredentials(providers))
 		ctx := context.Background()
-		h, err := New(ctx, client, keys, up)
+		h, err := New(ctx, client, keys, up, refresh)
 		if err != nil {
 			return c.Errf("failed to create Route53 plugin: %v", err)
 		}
diff --git a/plugin/route53/setup_test.go b/plugin/route53/setup_test.go
index 3e827eb58..998285e46 100644
--- a/plugin/route53/setup_test.go
+++ b/plugin/route53/setup_test.go
@@ -51,6 +51,22 @@ func TestSetupRoute53(t *testing.T) {
 		{`route53 example.org:12345678 example.org:12345678 {
 	}`, true},
 
+		{`route53 example.org:12345678 {
+	refresh 90
+}`, false},
+		{`route53 example.org:12345678 {
+	refresh 5m
+}`, false},
+		{`route53 example.org:12345678 {
+	refresh
+}`, true},
+		{`route53 example.org:12345678 {
+	refresh foo
+}`, true},
+		{`route53 example.org:12345678 {
+	refresh -1m
+}`, true},
+
 		{`route53 example.org {
 	}`, true},
 	}