kubernetes/migration: support simple downgrade cases (#172)

* support basic downgrades

* add unit test for validDownMigration

* add no-op test for downward migration

* document

kubernetes/migration/README.md

@@ -30,15 +30,22 @@ any new default plugins that would be added in a migration.  Notices
 
 `Migrate(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string, deprecations bool) (string, error)`
 
-Migrate returns a migrated version of the Corefile, or an error if it cannot. It will:
+Migrate returns a migrated version of the Corefile, or an error if it cannot. The _to_ version 
+must be >= the _from_ version. It will:
   * replace/convert any plugins/options that have replacements (e.g. _proxy_ -> _forward_)
   * return an error if replacable plugins/options cannot be converted (e.g. proxy _options_ not available in _forward_)
   * remove plugins/options that do not have replacements (e.g. kubernetes `upstream`)
-  * add in any new default plugins where applicable if they are not already present (e.g. adding _loop_ plugin when it was added).
+  * add in any new default plugins where applicable if they are not already present.
-    This will have to be case by case, and could potentially get complicated.
   * If deprecations is true, deprecated plugins/options will be migrated as soon as they are deprecated.
   * If deprecations is false, deprecated plugins/options will be migrated only once they become removed or ignored.
 
+`MigrateDown(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string) (string, error)`
+
+MigrateDown returns a downward migrated version of the Corefile, or an error if it cannot. The _to_ version 
+must be <= the _from_ version.
+  * It will handle the removal of plugins and options that no longer exist in the destination 
+    version when downgrading.
+  * It will not restore plugins/options that might have been removed or altered during an upward migration. 
 
 `Unsupported(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string) ([]Notice, error)`
 

kubernetes/migration/migrate.go

@@ -29,7 +29,7 @@ func getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, status string)
 	if fromCoreDNSVersion == toCoreDNSVersion {
 		return nil, nil
 	}
-	err := validateVersions(fromCoreDNSVersion, toCoreDNSVersion)
+	err := validUpMigration(fromCoreDNSVersion, toCoreDNSVersion)
 	if err != nil {
 		return nil, err
 	}
@@ -125,14 +125,15 @@ func getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, status string)
 	return notices, nil
 }
 
-// Migrate returns the Corefile converted to toCoreDNSVersion, or an error if it cannot.
+// Migrate returns the Corefile converted to toCoreDNSVersion, or an error if it cannot.  This function only accepts
+// a forward migration, where the destination version is => the start version.
 // If deprecations is true, deprecated plugins/options will be migrated as soon as they are deprecated.
 // If deprecations is false, deprecated plugins/options will be migrated only once they become removed or ignored.
 func Migrate(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string, deprecations bool) (string, error) {
 	if fromCoreDNSVersion == toCoreDNSVersion {
 		return corefileStr, nil
 	}
-	err := validateVersions(fromCoreDNSVersion, toCoreDNSVersion)
+	err := validUpMigration(fromCoreDNSVersion, toCoreDNSVersion)
 	if err != nil {
 		return "", err
 	}
@@ -254,6 +255,89 @@ func Migrate(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string, deprecati
 	return cf.ToString(), nil
 }
 
+// MigrateDown returns the Corefile converted to toCoreDNSVersion, or an error if it cannot. This function only accepts
+// a downward migration, where the destination version is <= the start version.
+func MigrateDown(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string) (string, error) {
+	if fromCoreDNSVersion == toCoreDNSVersion {
+		return corefileStr, nil
+	}
+	err := validDownMigration(fromCoreDNSVersion, toCoreDNSVersion)
+	if err != nil {
+		return "", err
+	}
+	cf, err := corefile.New(corefileStr)
+	if err != nil {
+		return "", err
+	}
+	v := fromCoreDNSVersion
+	for {
+		newSrvs := []*corefile.Server{}
+		for _, s := range cf.Servers {
+			newPlugs := []*corefile.Plugin{}
+			for _, p := range s.Plugins {
+				vp, present := Versions[v].plugins[p.Name]
+				if !present {
+					newPlugs = append(newPlugs, p)
+					continue
+				}
+				if vp.downAction == nil {
+					newPlugs = append(newPlugs, p)
+					continue
+				}
+				p, err := vp.downAction(p)
+				if err != nil {
+					return "", err
+				}
+				if p == nil {
+					// remove plugin, skip options processing
+					continue
+				}
+
+				newOpts := []*corefile.Option{}
+				for _, o := range p.Options {
+					vo, present := Versions[v].plugins[p.Name].options[o.Name]
+					if !present {
+						newOpts = append(newOpts, o)
+						continue
+					}
+					if vo.downAction == nil {
+						newOpts = append(newOpts, o)
+						continue
+					}
+					o, err := vo.downAction(o)
+					if err != nil {
+						return "", err
+					}
+					if o == nil {
+						// remove option
+						continue
+					}
+					newOpts = append(newOpts, o)
+				}
+				newPlug := &corefile.Plugin{
+					Name:    p.Name,
+					Args:    p.Args,
+					Options: newOpts,
+				}
+				newPlugs = append(newPlugs, newPlug)
+			}
+			newSrv := &corefile.Server{
+				DomPorts: s.DomPorts,
+				Plugins:  newPlugs,
+			}
+			newSrvs = append(newSrvs, newSrv)
+		}
+
+		cf = &corefile.Corefile{Servers: newSrvs}
+
+		if v == toCoreDNSVersion {
+			break
+		}
+		v = Versions[v].priorVersion
+	}
+	return cf.ToString(), nil
+}
+
 // Default returns true if the Corefile is the default for a given version of Kubernetes.
 // Or, if k8sVersion is empty, Default returns true if the Corefile is the default for any version of Kubernetes.
 func Default(k8sVersion, corefileStr string) bool {
@@ -330,7 +414,7 @@ func validateVersion(fromCoreDNSVersion string) error {
 	return nil
 }
 
-func validateVersions(fromCoreDNSVersion, toCoreDNSVersion string) error {
+func validUpMigration(fromCoreDNSVersion, toCoreDNSVersion string) error {
 	err := validateVersion(fromCoreDNSVersion)
 	if err != nil {
 		return err
@@ -341,5 +425,19 @@ func validateVersions(fromCoreDNSVersion, toCoreDNSVersion string) error {
 		}
 		return nil
 	}
-	return fmt.Errorf("cannot migrate to '%v' from '%v'", toCoreDNSVersion, fromCoreDNSVersion)
+	return fmt.Errorf("cannot migrate up to '%v' from '%v'", toCoreDNSVersion, fromCoreDNSVersion)
+}
+
+func validDownMigration(fromCoreDNSVersion, toCoreDNSVersion string) error {
+	err := validateVersion(fromCoreDNSVersion)
+	if err != nil {
+		return err
+	}
+	for prior := Versions[fromCoreDNSVersion].priorVersion; prior != ""; prior = Versions[prior].priorVersion {
+		if prior != toCoreDNSVersion {
+			continue
+		}
+		return nil
+	}
+	return fmt.Errorf("cannot migrate down to '%v' from '%v'", toCoreDNSVersion, fromCoreDNSVersion)
 }

kubernetes/migration/migrate_test.go

@@ -242,6 +242,108 @@ mystub-2.example.org {
 	}
 }
 
+func TestMigrateDown(t *testing.T) {
+	testCases := []struct {
+		name             string
+		fromVersion      string
+		toVersion        string
+		deprecations     bool
+		startCorefile    string
+		expectedCorefile string
+	}{
+		{
+			name:        "from 1.5.0 to 1.1.3",
+			fromVersion: "1.5.0",
+			toVersion:   "1.1.3",
+			startCorefile: `.:53 {
+    errors
+    health
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+        pods insecure
+        upstream
+        fallthrough in-addr.arpa ip6.arpa
+    }
+    prometheus :9153
+    forward . /etc/resolv.conf
+    cache 30
+    loop
+    reload
+    loadbalance
+}
+`,
+			expectedCorefile: `.:53 {
+    errors
+    health
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+        pods insecure
+        upstream
+        fallthrough in-addr.arpa ip6.arpa
+    }
+    prometheus :9153
+    forward . /etc/resolv.conf
+    cache 30
+    reload
+    loadbalance
+}
+`,
+		},
+		{
+			name:         "no-op same version migration",
+			fromVersion:  "1.3.1",
+			toVersion:    "1.3.1",
+			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
+    loop
+    reload
+    loadbalance
+}
+`,
+			expectedCorefile: `.: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
+    loop
+    reload
+    loadbalance
+}
+`,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+
+			result, err := MigrateDown(testCase.fromVersion, testCase.toVersion, testCase.startCorefile)
+
+			if err != nil {
+				t.Errorf("%v", err)
+			}
+
+			if result != testCase.expectedCorefile {
+				t.Errorf("expected -> got diffs:\n%v", diff.LineDiff(testCase.expectedCorefile, result))
+			}
+		})
+	}
+}
+
 func TestDeprecated(t *testing.T) {
 	startCorefile := `.:53 {
     errors
@@ -418,7 +520,7 @@ stubzone.org:53 {
 	}
 }
 
-func TestValidateVersions(t *testing.T) {
+func TestValidUpMigration(t *testing.T) {
 	testCases := []struct {
 		from      string
 		to        string
@@ -433,7 +535,33 @@ func TestValidateVersions(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		err := validateVersions(tc.from, tc.to)
+		err := validUpMigration(tc.from, tc.to)
+
+		if !tc.shouldErr && err != nil {
+			t.Errorf("expected '%v' to '%v' to be valid versions.", tc.from, tc.to)
+		}
+		if tc.shouldErr && err == nil {
+			t.Errorf("expected '%v' to '%v' to be invalid versions.", tc.from, tc.to)
+		}
+	}
+}
+
+func TestValidDownMigration(t *testing.T) {
+	testCases := []struct {
+		from      string
+		to        string
+		shouldErr bool
+	}{
+		{"1.3.1", "1.3.1", true},
+		{"1.3.1", "1.5.0", true},
+		{"1.5.0", "1.3.1", false},
+		{"banana", "1.5.0", true},
+		{"1.3.1", "apple", true},
+		{"banana", "apple", true},
+	}
+
+	for _, tc := range testCases {
+		err := validDownMigration(tc.from, tc.to)
 
 		if !tc.shouldErr && err != nil {
 			t.Errorf("expected '%v' to '%v' to be valid versions.", tc.from, tc.to)

kubernetes/migration/versions.go

@@ -12,19 +12,22 @@ type plugin struct {
 	options    map[string]option
 	action     pluginActionFn // action affecting this plugin only
 	add        serverActionFn // action to add a new plugin to the server block
+	downAction pluginActionFn // downgrade action affecting this plugin only
 }
 
 type option struct {
 	status     string
 	replacedBy string
 	additional string
-	action     optionActionFn // take action affecting this option only
+	action     optionActionFn // action affecting this option only
-	add        pluginActionFn // take action to add the option to the plugin
+	add        pluginActionFn // action to add the option to the plugin
+	downAction optionActionFn // downgrade action affecting this option only
 }
 
 type release struct {
 	k8sRelease     string
 	nextVersion    string
+	priorVersion   string
 	dockerImageSHA string
 	plugins        map[string]plugin // list of plugins with deprecation status and migration actions
 
@@ -88,6 +91,8 @@ func addToAllServerBlocks(sb *corefile.Server, newPlugin *corefile.Plugin) (*cor
 
 var Versions = map[string]release{
 	"1.5.0": {
+		priorVersion:   "1.4.0",
+		k8sRelease:     "1.15",
 		dockerImageSHA: "e83beb5e43f8513fa735e77ffc5859640baea30a882a11cc75c4c3244a737d3c",
 		plugins: map[string]plugin{
 			"errors": {
@@ -106,6 +111,7 @@ var Versions = map[string]release{
 				add: func(c *corefile.Server) (*corefile.Server, error) {
 					return addToKubernetesServerBlocks(c, &corefile.Plugin{Name: "ready"})
 				},
+				downAction: removePlugin,
 			},
 			"autopath": {},
 			"kubernetes": {
@@ -176,6 +182,7 @@ var Versions = map[string]release{
 	},
 	"1.4.0": {
 		nextVersion:    "1.5.0",
+		priorVersion:   "1.3.1",
 		dockerImageSHA: "70a92e9f6fc604f9b629ca331b6135287244a86612f550941193ec7e12759417",
 		plugins: map[string]plugin{
 			"errors": {
@@ -255,6 +262,7 @@ var Versions = map[string]release{
 	},
 	"1.3.1": {
 		nextVersion:    "1.4.0",
+		priorVersion:   "1.3.0",
 		k8sRelease:     "1.14",
 		dockerImageSHA: "02382353821b12c21b062c59184e227e001079bb13ebd01f9d3270ba0fcbf1e4",
 		defaultConf: `.:53 {
@@ -351,6 +359,7 @@ var Versions = map[string]release{
 	},
 	"1.3.0": {
 		nextVersion:    "1.3.1",
+		priorVersion:   "1.2.6",
 		dockerImageSHA: "e030773c7fee285435ed7fc7623532ee54c4c1c4911fb24d95cd0170a8a768bc",
 		plugins: map[string]plugin{
 			"errors": {
@@ -384,6 +393,7 @@ var Versions = map[string]release{
 				},
 			},
 			"k8s_external": {
+				downAction: removePlugin,
 				options: map[string]option{
 					"apex": {},
 					"ttl":  {},
@@ -428,6 +438,7 @@ var Versions = map[string]release{
 	},
 	"1.2.6": {
 		nextVersion:    "1.3.0",
+		priorVersion:   "1.2.5",
 		k8sRelease:     "1.13",
 		dockerImageSHA: "81936728011c0df9404cb70b95c17bbc8af922ec9a70d0561a5d01fefa6ffa51",
 		defaultConf: `.:53 {
@@ -515,6 +526,7 @@ var Versions = map[string]release{
 	},
 	"1.2.5": {
 		nextVersion:    "1.2.6",
+		priorVersion:   "1.2.4",
 		dockerImageSHA: "33c8da20b887ae12433ec5c40bfddefbbfa233d5ce11fb067122e68af30291d6",
 		plugins: map[string]plugin{
 			"errors": {},
@@ -582,6 +594,7 @@ var Versions = map[string]release{
 	},
 	"1.2.4": {
 		nextVersion:    "1.2.5",
+		priorVersion:   "1.2.3",
 		dockerImageSHA: "a0d40ad961a714c699ee7b61b77441d165f6252f9fb84ac625d04a8d8554c0ec",
 		plugins: map[string]plugin{
 			"errors": {},
@@ -649,6 +662,7 @@ var Versions = map[string]release{
 	},
 	"1.2.3": {
 		nextVersion:    "1.2.4",
+		priorVersion:   "1.2.2",
 		dockerImageSHA: "12f3cab301c826978fac736fd40aca21ac023102fd7f4aa6b4341ae9ba89e90e",
 		plugins: map[string]plugin{
 			"errors": {},
@@ -716,6 +730,7 @@ var Versions = map[string]release{
 	},
 	"1.2.2": {
 		nextVersion:    "1.2.3",
+		priorVersion:   "1.2.1",
 		k8sRelease:     "1.12",
 		dockerImageSHA: "3e2be1cec87aca0b74b7668bbe8c02964a95a402e45ceb51b2252629d608d03a",
 		defaultConf: `.:53 {
@@ -798,6 +813,7 @@ var Versions = map[string]release{
 	},
 	"1.2.1": {
 		nextVersion:    "1.2.2",
+		priorVersion:   "1.2.0",
 		dockerImageSHA: "fb129c6a7c8912bc6d9cc4505e1f9007c5565ceb1aa6369750e60cc79771a244",
 		plugins: map[string]plugin{
 			"errors": {},
@@ -862,6 +878,7 @@ var Versions = map[string]release{
 				add: func(s *corefile.Server) (*corefile.Server, error) {
 					return addToForwardingServerBlocks(s, &corefile.Plugin{Name: "loop"})
 				},
+				downAction: removePlugin,
 			},
 			"reload":      {},
 			"loadbalance": {},
@@ -869,6 +886,7 @@ var Versions = map[string]release{
 	},
 	"1.2.0": {
 		nextVersion:    "1.2.1",
+		priorVersion:   "1.1.4",
 		dockerImageSHA: "ae69a32f8cc29a3e2af9628b6473f24d3e977950a2cb62ce8911478a61215471",
 		plugins: map[string]plugin{
 			"errors": {},
@@ -937,6 +955,7 @@ var Versions = map[string]release{
 	},
 	"1.1.4": {
 		nextVersion:    "1.2.0",
+		priorVersion:   "1.1.3",
 		dockerImageSHA: "463c7021141dd3bfd4a75812f4b735ef6aadc0253a128f15ffe16422abe56e50",
 		plugins: map[string]plugin{
 			"errors": {},