kubernetes/migration: Add new default plugins/options (#155)

* add new defaults

* i think this works better

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,
+				newPlug := &corefile.Plugin{
-					&corefile.Plugin{
 					Name:    p.Name,
 					Args:    p.Args,
 					Options: newOpts,
-					})
 				}
-			newSrvs = append(newSrvs,
+			CheckForNewOptions:
-				&corefile.Server{
+				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)
+			}
+			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 {

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 {

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
 )

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",
@@ -60,6 +93,12 @@ var Versions = map[string]release{
 				},
 			},
 			"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": {},
 		},