diff --git a/CHANGELOG.md b/CHANGELOG.md index e16c207ae..591519eea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ + - Graceful handling of misconfigure password for dyn provider (#470) @jvassev - Don't log sensitive data on start (#463) @jvassev - Google: Improve logging to help trace misconfigurations (#388) @stealthybox - AWS: In addition to the one best public hosted zone, records will be added to all matching private hosted zones (#356) @coreypobrien diff --git a/main.go b/main.go index 1a6d86bca..db6c2a94d 100644 --- a/main.go +++ b/main.go @@ -121,13 +121,14 @@ func main() { case "dyn": p, err = provider.NewDynProvider( provider.DynConfig{ - DomainFilter: domainFilter, - ZoneIDFilter: zoneIDFilter, - DryRun: cfg.DryRun, - CustomerName: cfg.DynCustomerName, - Username: cfg.DynUsername, - Password: cfg.DynPassword, - AppVersion: externaldns.Version, + DomainFilter: domainFilter, + ZoneIDFilter: zoneIDFilter, + DryRun: cfg.DryRun, + CustomerName: cfg.DynCustomerName, + Username: cfg.DynUsername, + Password: cfg.DynPassword, + MinTTLSeconds: cfg.DynMinTTLSeconds, + AppVersion: externaldns.Version, }, ) case "inmemory": diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 69d67f99c..d9036dda5 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -61,6 +61,7 @@ type Config struct { DynCustomerName string DynUsername string DynPassword string + DynMinTTLSeconds int InMemoryZones []string Policy string Registry string @@ -172,6 +173,8 @@ func (cfg *Config) ParseFlags(args []string) error { app.Flag("dyn-customer-name", "When using the Dyn provider, specify the Customer Name").Default("").StringVar(&cfg.DynCustomerName) app.Flag("dyn-username", "When using the Dyn provider, specify the Username").Default("").StringVar(&cfg.DynUsername) app.Flag("dyn-password", "When using the Dyn provider, specify the pasword").Default("").StringVar(&cfg.DynPassword) + app.Flag("dyn-min-ttl", "Minimal TTL (in seconds) for records. This value will be used if the provided TTL for a service/ingress is lower than this.").IntVar(&cfg.DynMinTTLSeconds) + app.Flag("inmemory-zone", "Provide a list of pre-configured zones for the inmemory provider; specify multiple times for multiple zones (optional)").Default("").StringsVar(&cfg.InMemoryZones) // Flags related to policies diff --git a/pkg/apis/externaldns/types_test.go b/pkg/apis/externaldns/types_test.go index 5e3b6a586..1cfb0d97c 100644 --- a/pkg/apis/externaldns/types_test.go +++ b/pkg/apis/externaldns/types_test.go @@ -18,6 +18,7 @@ package externaldns import ( "os" + "strings" "testing" "time" @@ -222,3 +223,15 @@ func restoreEnv(t *testing.T, originalEnv map[string]string) { require.NoError(t, os.Setenv(k, v)) } } + +func TestPasswordsNotLogged(t *testing.T) { + cfg := Config{ + DynPassword: "dyn-pass", + InfobloxWapiPassword: "infoblox-pass", + } + + s := cfg.String() + + assert.False(t, strings.Contains(s, "dyn-pass")) + assert.False(t, strings.Contains(s, "infoblox-pass")) +} diff --git a/pkg/apis/externaldns/validation/validation.go b/pkg/apis/externaldns/validation/validation.go index 4918e4c49..cdbe6ef4e 100644 --- a/pkg/apis/externaldns/validation/validation.go +++ b/pkg/apis/externaldns/validation/validation.go @@ -52,5 +52,18 @@ func ValidateConfig(cfg *externaldns.Config) error { return errors.New("no Infoblox WAPI password specified") } } + + if cfg.Provider == "dyn" { + if cfg.DynUsername == "" { + return errors.New("no Dyn username specified") + } + if cfg.DynCustomerName == "" { + return errors.New("no Dyn customer name specified") + } + + if cfg.DynMinTTLSeconds < 0 { + return errors.New("TTL specified for Dyn is negative") + } + } return nil } diff --git a/pkg/apis/externaldns/validation/validation_test.go b/pkg/apis/externaldns/validation/validation_test.go index 9c048a838..c50073ae4 100644 --- a/pkg/apis/externaldns/validation/validation_test.go +++ b/pkg/apis/externaldns/validation/validation_test.go @@ -63,3 +63,56 @@ func newValidConfig(t *testing.T) *externaldns.Config { return cfg } + +func addRequiredFieldsForDyn(cfg *externaldns.Config) { + cfg.LogFormat = "json" + cfg.Sources = []string{"ingress"} + cfg.Provider = "dyn" +} + +func TestValidateBadDynConfig(t *testing.T) { + badConfigs := []*externaldns.Config{ + {}, + { + // only username + DynUsername: "test", + }, + { + // only customer name + DynCustomerName: "test", + }, + { + // negative timeout + DynUsername: "test", + DynCustomerName: "test", + DynMinTTLSeconds: -1, + }, + } + + for _, cfg := range badConfigs { + addRequiredFieldsForDyn(cfg) + err := ValidateConfig(cfg) + assert.NotNil(t, err, "Configuration %+v should NOT have passed validation", cfg) + } +} + +func TestValidateGoodDynConfig(t *testing.T) { + goodConfigs := []*externaldns.Config{ + { + DynUsername: "test", + DynCustomerName: "test", + DynMinTTLSeconds: 600, + }, + { + DynUsername: "test", + DynCustomerName: "test", + DynMinTTLSeconds: 0, + }, + } + + for _, cfg := range goodConfigs { + addRequiredFieldsForDyn(cfg) + err := ValidateConfig(cfg) + assert.Nil(t, err, "Configuration should be valid, got this error instead", err) + } +} diff --git a/provider/dyn.go b/provider/dyn.go index 725e4b872..038b622f6 100644 --- a/provider/dyn.go +++ b/provider/dyn.go @@ -19,6 +19,7 @@ package provider import ( "fmt" "os" + "strconv" "strings" "time" @@ -37,6 +38,12 @@ const ( // may be made configurable in the future but 20K records seems like enough for a few zones cacheMaxSize = 20000 + // two consecutive bad logins happen at least this many seconds appart + // While it is easy to get the username right, misconfiguring the password + // can get account blocked. Exit(1) is not a good solution + // as k8s will restart the pod and another login attempt will be made + badLoginMinIntervalSeconds = 30 * 60 + // this prefix must be stripped from resource links before feeding them to dynect.Client.Do() restAPIPrefix = "/REST/" ) @@ -60,17 +67,21 @@ func (c *cache) Put(link string, ep *endpoint.Endpoint) { c.contents[link] = &entry{ ep: ep, - expires: int64(time.Now().Unix()) + int64(ep.RecordTTL), + expires: unixNow() + int64(ep.RecordTTL), } } +func unixNow() int64 { + return int64(time.Now().Unix()) +} + func (c *cache) Get(link string) *endpoint.Endpoint { result, ok := c.contents[link] if !ok { return nil } - now := int64(time.Now().Unix()) + now := unixNow() if result.expires < now { delete(c.contents, link) @@ -82,20 +93,22 @@ func (c *cache) Get(link string) *endpoint.Endpoint { // DynConfig hold connection parameters to dyn.com and interanl state type DynConfig struct { - DomainFilter DomainFilter - ZoneIDFilter ZoneIDFilter - DryRun bool - CustomerName string - Username string - Password string - AppVersion string - DynVersion string + DomainFilter DomainFilter + ZoneIDFilter ZoneIDFilter + DryRun bool + CustomerName string + Username string + Password string + MinTTLSeconds int + AppVersion string + DynVersion string } // DynProvider is the actual interface impl. type dynProviderState struct { DynConfig - Cache *cache + Cache *cache + LastLoginErrorTime int64 } // ZoneChange is missing from dynect: https://help.dyn.com/get-zone-changeset-api/ @@ -166,13 +179,17 @@ func filterAndFixLinks(links []string, filter DomainFilter) []string { return result } -func fixMissingTTL(ttl endpoint.TTL) string { +func fixMissingTTL(ttl endpoint.TTL, minTTLSeconds int) string { i := dynDefaultTTL if ttl.IsConfigured() { - i = int(ttl) + if int(ttl) < minTTLSeconds { + i = minTTLSeconds + } else { + i = int(ttl) + } } - return fmt.Sprintf("%d", i) + return strconv.Itoa(i) } // merge produces a singe list of records that can be used as a replacement. @@ -320,7 +337,6 @@ func (d *dynProviderState) buildLinkToRecord(ep *endpoint.Endpoint) string { } if matchingZone == "" { - fmt.Printf("no zone") // no matching zone, ignore return "" } @@ -337,6 +353,12 @@ func (d *dynProviderState) buildLinkToRecord(ep *endpoint.Endpoint) string { // This method also stores the DynAPI version. // Don't user the dynect.Client.Login() func (d *dynProviderState) login() (*dynect.Client, error) { + if d.LastLoginErrorTime != 0 { + secondsSinceLastError := unixNow() - d.LastLoginErrorTime + if secondsSinceLastError < badLoginMinIntervalSeconds { + return nil, fmt.Errorf("will not attempt an API call as the last login failure occurred just %ds ago", secondsSinceLastError) + } + } client := dynect.NewClient(d.CustomerName) var req = dynect.LoginBlock{ @@ -348,9 +370,11 @@ func (d *dynProviderState) login() (*dynect.Client, error) { err := client.Do("POST", "Session", req, &resp) if err != nil { + d.LastLoginErrorTime = unixNow() return nil, err } + d.LastLoginErrorTime = 0 client.Token = resp.Data.Token // this is the only change from the original @@ -371,7 +395,7 @@ func (d *dynProviderState) buildRecordRequest(ep *endpoint.Endpoint) (string, *d } record := dynect.RecordRequest{ - TTL: fixMissingTTL(ep.RecordTTL), + TTL: fixMissingTTL(ep.RecordTTL, d.MinTTLSeconds), RData: *endpointToRecord(ep), } return link, &record diff --git a/provider/dyn_test.go b/provider/dyn_test.go index 6ae0e831e..851c4c721 100644 --- a/provider/dyn_test.go +++ b/provider/dyn_test.go @@ -255,10 +255,13 @@ func TestDyn_filterAndFixLinks(t *testing.T) { } func TestDyn_fixMissingTTL(t *testing.T) { - assert.Equal(t, fmt.Sprintf("%v", dynDefaultTTL), fixMissingTTL(endpoint.TTL(0))) + assert.Equal(t, fmt.Sprintf("%v", dynDefaultTTL), fixMissingTTL(endpoint.TTL(0), 0)) // nothing to fix - assert.Equal(t, "111", fixMissingTTL(endpoint.TTL(111))) + assert.Equal(t, "111", fixMissingTTL(endpoint.TTL(111), 25)) + + // apply min TTL + assert.Equal(t, "1992", fixMissingTTL(endpoint.TTL(111), 1992)) } func TestDyn_cachePut(t *testing.T) {