From 709b1365631760cbf667a929a47719fdae002bc9 Mon Sep 17 00:00:00 2001 From: Sandeep Rajan Date: Mon, 13 May 2019 16:19:36 -0400 Subject: [PATCH] Alter existing "deprecated" function to return all migration notices (#159) --- kubernetes/corefile-tool/README.md | 10 +---- kubernetes/corefile-tool/cmd/deprecated.go | 4 +- kubernetes/corefile-tool/cmd/removed.go | 52 ---------------------- kubernetes/corefile-tool/cmd/root.go | 1 - kubernetes/migration/migrate.go | 23 ++++------ kubernetes/migration/migrate_test.go | 50 +++------------------ kubernetes/migration/notice.go | 1 + 7 files changed, 19 insertions(+), 122 deletions(-) delete mode 100644 kubernetes/corefile-tool/cmd/removed.go diff --git a/kubernetes/corefile-tool/README.md b/kubernetes/corefile-tool/README.md index 9eba929..3bf8483 100644 --- a/kubernetes/corefile-tool/README.md +++ b/kubernetes/corefile-tool/README.md @@ -13,7 +13,6 @@ Usage: corefile-tool deprecated --from --to --corefile corefile-tool migrate --from --to --corefile [--deprecations ] corefile-tool released --dockerImageId - corefile-tool removed --from --to --corefile corefile-tool unsupported --from --to --corefile corefile-tool validversions ``` @@ -25,14 +24,12 @@ The following operations are supported: - `default`: returns true if the Corefile is the default for the given version of Kubernetes. If `--k8sversion` is not specified, then this will return true if the Corefile is the default for any version of Kubernetes supported by the tool. -- `deprecated`: returns a list of plugins/options in the Corefile that have been deprecated. +- `deprecated`: returns a list of plugins/options in the Corefile that have been deprecated, removed, ignored or is a new default plugin/option. - `migrate`: updates your CoreDNS corefile to be compatible with the `-to` version. Setting the `--deprecations` flag to `true` will migrate plugins/options as soon as they are announced as deprecated. Setting the `--deprecations` flag to `false` will migrate plugins/options only once they are removed (or made a no-op). The default is `false`. - `released`: determines if the `--dockerImageID` was an official CoreDNS release or not. Only official releases of CoreDNS are supported by the tool. -- `removed`: returns a list plugins/options in the Corefile that have been removed from CoreDNS. - - `unsupported`: returns a list of plugins/options in the Corefile that are not supported by the migration tool (but may still be valid in CoreDNS). - `validversions`: Shows the list of CoreDNS versions supported by the this tool. @@ -57,11 +54,6 @@ corefile-tool deprecated --from 1.4.0 --to 1.5.0 --corefile /path/to/Corefile corefile-tool unsupported --from 1.4.0 --to 1.5.0 --corefile /path/to/Corefile ``` -```bash -# See removed plugins CoreDNS from v1.4.0 to v1.5.0. -corefile-tool removed --from 1.4.0 --to 1.5.0 --corefile /path/to/Corefile -``` - ```bash # Migrate CoreDNS from v1.4.0 to v1.5.0 and also migrate all the deprecations # that are present in the current Corefile. diff --git a/kubernetes/corefile-tool/cmd/deprecated.go b/kubernetes/corefile-tool/cmd/deprecated.go index 00a59e3..b9cbad5 100644 --- a/kubernetes/corefile-tool/cmd/deprecated.go +++ b/kubernetes/corefile-tool/cmd/deprecated.go @@ -12,8 +12,8 @@ import ( func NewDeprecatedCmd() *cobra.Command { deprecatedCmd := &cobra.Command{ Use: "deprecated", - Short: "Deprecated returns a list of deprecated plugins or directives present in the Corefile.", - Example: `# See deprecated plugins CoreDNS from v1.4.0 to v1.5.0. + Short: "Deprecated returns a list of deprecated, removed, ignored and new default plugins or directives present in the Corefile.", + Example: `# See deprecated, removed, ignored and new default plugins CoreDNS from v1.4.0 to v1.5.0. corefile-tool deprecated --from 1.4.0 --to 1.5.0 --corefile /path/to/Corefile`, RunE: func(cmd *cobra.Command, args []string) error { from, _ := cmd.Flags().GetString("from") diff --git a/kubernetes/corefile-tool/cmd/removed.go b/kubernetes/corefile-tool/cmd/removed.go deleted file mode 100644 index ab19c37..0000000 --- a/kubernetes/corefile-tool/cmd/removed.go +++ /dev/null @@ -1,52 +0,0 @@ -package cmd - -import ( - "fmt" - - "github.com/coredns/deployment/kubernetes/migration" - - "github.com/spf13/cobra" -) - -// NewRemovedCmd represents the removed command -func NewRemovedCmd() *cobra.Command { - removedCmd := &cobra.Command{ - Use: "removed", - Short: "Removed returns a list of removed plugins or directives present in the Corefile.", - Example: `# See removed plugins CoreDNS from v1.4.0 to v1.5.0. -corefile-tool removed --from 1.4.0 --to 1.5.0 --corefile /path/to/Corefile`, - RunE: func(cmd *cobra.Command, args []string) error { - from, _ := cmd.Flags().GetString("from") - to, _ := cmd.Flags().GetString("to") - corefile, _ := cmd.Flags().GetString("corefile") - removed, err := removedCorefileFromPath(from, to, corefile) - if err != nil { - return fmt.Errorf("error while listing deprecated plugins: %v \n", err) - } - for _, rem := range removed { - fmt.Println(rem.ToString()) - } - return nil - }, - - } - removedCmd.Flags().String("from", "", "Required: The version you are migrating from. ") - removedCmd.MarkFlagRequired("from") - removedCmd.Flags().String("to", "", "Required: The version you are migrating to.") - removedCmd.MarkFlagRequired("to") - removedCmd.Flags().String("corefile", "", "Required: The path where your Corefile is located.") - removedCmd.MarkFlagRequired("corefile") - - return removedCmd -} - -// removedCorefileFromPath takes the path where the Corefile is located and returns the plugins or directives -// that have been removed. -func removedCorefileFromPath(fromCoreDNSVersion, toCoreDNSVersion, corefilePath string) ([]migration.Notice, error) { - fileBytes, err := getCorefileFromPath(corefilePath) - if err != nil { - return nil, err - } - corefileStr := string(fileBytes) - return migration.Removed(fromCoreDNSVersion, toCoreDNSVersion, corefileStr) -} diff --git a/kubernetes/corefile-tool/cmd/root.go b/kubernetes/corefile-tool/cmd/root.go index 64dbb04..d8040ef 100644 --- a/kubernetes/corefile-tool/cmd/root.go +++ b/kubernetes/corefile-tool/cmd/root.go @@ -26,7 +26,6 @@ func CorefileTool() *cobra.Command { `), } - rootCmd.AddCommand(NewRemovedCmd()) rootCmd.AddCommand(NewMigrateCmd()) rootCmd.AddCommand(NewDefaultCmd()) rootCmd.AddCommand(NewDeprecatedCmd()) diff --git a/kubernetes/migration/migrate.go b/kubernetes/migration/migrate.go index d6e25ee..9f6d415 100644 --- a/kubernetes/migration/migrate.go +++ b/kubernetes/migration/migrate.go @@ -12,18 +12,11 @@ import ( "github.com/coredns/deployment/kubernetes/migration/corefile" ) -// Deprecated returns a list of deprecated plugins or directives present in the Corefile. Each Notice returned is a -// warning, e.g. "plugin 'foo' is deprecated." An empty list returned means there are no deprecated plugins/options -// present in the Corefile. +// Deprecated returns a list of deprecated, removed, ignored and new default plugins or directives present in the Corefile. +// Each Notice returned is a warning, e.g. "plugin 'foo' is deprecated." An empty list returned means there are no +// deprecated, removed, ignored or new default plugins/options present in the Corefile. func Deprecated(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string) ([]Notice, error) { - return getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, deprecated) -} - -// Removed returns a list of removed plugins or directives present in the Corefile. Each Notice returned is a warning, -// e.g. "plugin 'foo' is no longer supported." An empty list returned means there are no removed plugins/options -// present in the Corefile. -func Removed(fromCoreDNSVersion, toCoreDNSVersion, corefileStr string) ([]Notice, error) { - return getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, removed) + return getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, all) } // Unsupported returns a list of plugins that are not recognized/supported by the migration tool (but may still be valid in CoreDNS). @@ -57,10 +50,10 @@ func getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, status string) if !present { continue } - if vp.status == status { + if vp.status != "" { notices = append(notices, Notice{ Plugin: p.Name, - Severity: status, + Severity: vp.status, Version: v, ReplacedBy: vp.replacedBy, Additional: vp.additional, @@ -86,8 +79,8 @@ func getStatus(fromCoreDNSVersion, toCoreDNSVersion, corefileStr, status string) if !present { continue } - if vo.status == status { - notices = append(notices, Notice{Plugin: p.Name, Option: o.Name, Severity: status, Version: v}) + if vo.status != "" { + notices = append(notices, Notice{Plugin: p.Name, Option: o.Name, Severity: vo.status, Version: v}) continue } } diff --git a/kubernetes/migration/migrate_test.go b/kubernetes/migration/migrate_test.go index 1c5d3e4..93a44ca 100644 --- a/kubernetes/migration/migrate_test.go +++ b/kubernetes/migration/migrate_test.go @@ -14,7 +14,7 @@ func TestMigrate(t *testing.T) { expectedCorefile string }{ { - name: "Remove invalid proxy option", + name: "Remove invalid proxy option", fromVersion: "1.1.3", toVersion: "1.2.6", deprecations: true, @@ -56,7 +56,7 @@ func TestMigrate(t *testing.T) { `, }, { - name: "Migrate from proxy to forward and handle Kubernetes deprecations", + name: "Migrate from proxy to forward and handle Kubernetes deprecations", fromVersion: "1.3.1", toVersion: "1.5.0", deprecations: true, @@ -96,7 +96,7 @@ func TestMigrate(t *testing.T) { `, }, { - name: "add missing loop and ready plugins", + name: "add missing loop and ready plugins", fromVersion: "1.1.3", toVersion: "1.5.0", deprecations: true, @@ -132,7 +132,6 @@ func TestMigrate(t *testing.T) { } `, }, - } for _, testCase := range testCases { @@ -155,6 +154,7 @@ func TestDeprecated(t *testing.T) { startCorefile := `.:53 { errors health + ready kubernetes cluster.local in-addr.arpa ip6.arpa { pods insecure upstream @@ -172,48 +172,12 @@ func TestDeprecated(t *testing.T) { expected := []Notice{ {Plugin: "kubernetes", Option: "upstream", Severity: deprecated, Version: "1.4.0"}, {Plugin: "proxy", Severity: deprecated, ReplacedBy: "forward", Version: "1.4.0"}, - } - - result, err := Deprecated("1.3.1", "1.5.0", startCorefile) - - if err != nil { - t.Fatal(err) - } - - if len(result) != len(expected) { - t.Fatalf("expected to find %v deprecations; got %v", len(expected), len(result)) - } - - for i, dep := range expected { - if result[i].ToString() != dep.ToString() { - t.Errorf("expected to get '%v'; got '%v'", dep.ToString(), result[i].ToString()) - } - } -} - -func TestRemoved(t *testing.T) { - 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 -} -` - - expected := []Notice{ + {Plugin: "ready", Severity: newdefault, Version: "1.5.0"}, + {Option: "upstream", Plugin: "kubernetes", Severity: ignored, Version: "1.5.0"}, {Plugin: "proxy", Severity: removed, ReplacedBy: "forward", Version: "1.5.0"}, } - result, err := Removed("1.3.1", "1.5.0", startCorefile) + result, err := Deprecated("1.3.1", "1.5.0", startCorefile) if err != nil { t.Fatal(err) diff --git a/kubernetes/migration/notice.go b/kubernetes/migration/notice.go index 59f17e2..c14f199 100644 --- a/kubernetes/migration/notice.go +++ b/kubernetes/migration/notice.go @@ -41,4 +41,5 @@ const ( 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 + all = "all" // all plugin/option that are deprecated, ignored, removed and new defaults. )