From 2821bdd6c4ab882962f55e16e25352d26003425b Mon Sep 17 00:00:00 2001 From: Chris O'Haver Date: Fri, 26 Apr 2019 10:32:53 -0400 Subject: [PATCH] kubernetes/migration: Add new default plugins/options (#155) * add new defaults * i think this works better --- kubernetes/migration/migrate.go | 56 ++++++++++++++++++++++------ kubernetes/migration/migrate_test.go | 55 ++++++++++++++++++++++----- kubernetes/migration/notice.go | 1 + kubernetes/migration/versions.go | 48 +++++++++++++++++++++++- 4 files changed, 136 insertions(+), 24 deletions(-) diff --git a/kubernetes/migration/migrate.go b/kubernetes/migration/migrate.go index 6c59ce5..510aae4 100644 --- a/kubernetes/migration/migrate.go +++ b/kubernetes/migration/migrate.go @@ -101,6 +101,7 @@ func getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, status string) } // Migrate returns version of the Corefile migrated to toCoreDNSVersion, or an error if it cannot. +// TODO: add newdefault bool parameter? func Migrate(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string, deprecations bool) (string, error) { err := validateVersions(fromCoreDNSVersion, toCoreDNSVersion) if err != nil { @@ -161,19 +162,50 @@ func Migrate(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string, deprecati } newOpts = append(newOpts, o) } - newPlugs = append(newPlugs, - &corefile.Plugin{ - Name: p.Name, - Args: p.Args, - Options: newOpts, - }) + newPlug := &corefile.Plugin{ + Name: p.Name, + Args: p.Args, + Options: newOpts, + } + CheckForNewOptions: + for name, vo := range Versions[v].plugins[p.Name].options { + if vo.status != newdefault { + continue + } + for _, o := range p.Options { + if name == o.Name { + continue CheckForNewOptions + } + } + newPlug, err = vo.add(newPlug) + if err != nil { + return "", err + } + } + + newPlugs = append(newPlugs, newPlug) } - newSrvs = append(newSrvs, - &corefile.Server{ - DomPorts: s.DomPorts, - Plugins: newPlugs, - }, - ) + newSrv := &corefile.Server{ + DomPorts: s.DomPorts, + Plugins: newPlugs, + } + CheckForNewPlugins: + for name, vp := range Versions[v].plugins { + if vp.status != newdefault { + continue + } + for _, p := range s.Plugins { + if name == p.Name { + continue CheckForNewPlugins + } + } + newSrv, err = vp.add(newSrv) + if err != nil { + return "", err + } + } + + newSrvs = append(newSrvs, newSrv) } cf = corefile.Corefile{Servers: newSrvs} if v == toCoreDNSVersion { diff --git a/kubernetes/migration/migrate_test.go b/kubernetes/migration/migrate_test.go index 05988ff..1c5d3e4 100644 --- a/kubernetes/migration/migrate_test.go +++ b/kubernetes/migration/migrate_test.go @@ -15,6 +15,9 @@ func TestMigrate(t *testing.T) { }{ { name: "Remove invalid proxy option", + fromVersion: "1.1.3", + toVersion: "1.2.6", + deprecations: true, startCorefile: `.:53 { errors health @@ -34,11 +37,6 @@ func TestMigrate(t *testing.T) { loadbalance } `, - - fromVersion: "1.1.3", - toVersion: "1.2.6", - deprecations: true, - expectedCorefile: `.:53 { errors health @@ -59,6 +57,9 @@ func TestMigrate(t *testing.T) { }, { name: "Migrate from proxy to forward and handle Kubernetes deprecations", + fromVersion: "1.3.1", + toVersion: "1.5.0", + deprecations: true, startCorefile: `.:53 { errors health @@ -76,11 +77,6 @@ func TestMigrate(t *testing.T) { loadbalance } `, - - fromVersion: "1.3.1", - toVersion: "1.5.0", - deprecations: true, - expectedCorefile: `.:53 { errors health @@ -95,9 +91,48 @@ func TestMigrate(t *testing.T) { loop reload loadbalance + ready } `, }, + { + name: "add missing loop and ready plugins", + fromVersion: "1.1.3", + toVersion: "1.5.0", + deprecations: true, + startCorefile: `.:53 { + errors + health + kubernetes cluster.local in-addr.arpa ip6.arpa { + pods insecure + upstream + fallthrough in-addr.arpa ip6.arpa + } + prometheus :9153 + proxy . /etc/resolv.conf + cache 30 + reload + loadbalance +} +`, + expectedCorefile: `.:53 { + errors + health + kubernetes cluster.local in-addr.arpa ip6.arpa { + pods insecure + fallthrough in-addr.arpa ip6.arpa + } + prometheus :9153 + forward . /etc/resolv.conf + cache 30 + reload + loadbalance + loop + ready +} +`, + }, + } for _, testCase := range testCases { diff --git a/kubernetes/migration/notice.go b/kubernetes/migration/notice.go index f5bb0dd..0de165b 100644 --- a/kubernetes/migration/notice.go +++ b/kubernetes/migration/notice.go @@ -38,4 +38,5 @@ const ( ignored = "ignored" // plugin/option is ignored by CoreDNS removed = "removed" // plugin/option has been removed from CoreDNS unsupported = "unsupported" // plugin/option is not supported by the migration tool + newdefault = "newdefault" // plugin/option was added to the default corefile ) diff --git a/kubernetes/migration/versions.go b/kubernetes/migration/versions.go index 6fdcc00..d024239 100644 --- a/kubernetes/migration/versions.go +++ b/kubernetes/migration/versions.go @@ -6,6 +6,7 @@ import ( type plugin struct { status string + add serverActionFn replacedBy string additional string action pluginActionFn @@ -14,6 +15,7 @@ type plugin struct { type option struct { status string + add pluginActionFn replacedBy string additional string action optionActionFn @@ -34,6 +36,7 @@ type release struct { defaultConf string } +type serverActionFn func(server *corefile.Server) (*corefile.Server, error) type pluginActionFn func(*corefile.Plugin) (*corefile.Plugin, error) type optionActionFn func(*corefile.Option) (*corefile.Option, error) @@ -45,6 +48,36 @@ func renamePlugin(p *corefile.Plugin, to string) (*corefile.Plugin, error) { return p, nil } +func addToServerBlocksWithPlugins(sb *corefile.Server, newPlugin *corefile.Plugin, with []string) (*corefile.Server, error) { + if len(with) == 0 { + // add to all blocks + sb.Plugins = append(sb.Plugins, newPlugin) + return sb, nil + } + for _, p := range sb.Plugins { + for _, w := range with { + if w == p.Name { + // add to this block + sb.Plugins = append(sb.Plugins, newPlugin) + return sb, nil + } + } + } + return sb, nil +} + +func addToKubernetesServerBlocks(sb *corefile.Server, newPlugin *corefile.Plugin) (*corefile.Server, error) { + return addToServerBlocksWithPlugins(sb, newPlugin, []string{"kubernetes"}) +} + +func addToForwardingServerBlocks(sb *corefile.Server, newPlugin *corefile.Plugin) (*corefile.Server, error) { + return addToServerBlocksWithPlugins(sb, newPlugin, []string{"forward", "proxy"}) +} + +func addToAllServerBlocks(sb *corefile.Server, newPlugin *corefile.Plugin) (*corefile.Server, error) { + return addToServerBlocksWithPlugins(sb, newPlugin, []string{}) +} + var Versions = map[string]release{ "1.5.0": { dockerImageID: "7987f0908caf", @@ -59,7 +92,13 @@ var Versions = map[string]release{ "class": {}, }, }, - "health": {}, + "health": {}, + "ready": { + status: newdefault, + add: func(c *corefile.Server) (*corefile.Server, error) { + return addToKubernetesServerBlocks(c, &corefile.Plugin{Name: "ready"}) + }, + }, "autopath": {}, "kubernetes": { options: map[string]option{ @@ -808,7 +847,12 @@ var Versions = map[string]release{ "prefetch": {}, }, }, - "loop": {}, + "loop": { + status: newdefault, + add: func(s *corefile.Server) (*corefile.Server, error) { + return addToForwardingServerBlocks(s, &corefile.Plugin{Name: "loop"}) + }, + }, "reload": {}, "loadbalance": {}, },