Merge "kartongips: switch default diff behaviour to subset, nag users"

cluster/tools/kartongips/cmd/BUILD.bazel

@@ -19,6 +19,7 @@ go_library(
         "//cluster/tools/kartongips/utils:go_default_library",
         "@com_github_genuinetools_reg//registry:go_default_library",
         "@com_github_google_go_jsonnet//:go_default_library",
+        "@com_github_mattn_go_isatty//:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
         "@com_github_spf13_cobra//:go_default_library",
         "@io_k8s_apimachinery//pkg/api/meta:go_default_library",

cluster/tools/kartongips/cmd/diff.go

@@ -16,22 +16,54 @@
 package cmd
 
 import (
+	"fmt"
+	"os"
+
+	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
 
 	"code.hackerspace.pl/hscloud/cluster/tools/kartongips/pkg/kubecfg"
 )
 
 const (
+	// TODO(b/49): remove this flag
 	flagDiffStrategy = "diff-strategy"
 	flagOmitSecrets  = "omit-secrets"
 )
 
 func init() {
-	diffCmd.PersistentFlags().String(flagDiffStrategy, "all", "Diff strategy, all or subset.")
+	diffCmd.PersistentFlags().String(flagDiffStrategy, "", "Diff strategy - no op (will be removed soon)")
 	diffCmd.PersistentFlags().Bool(flagOmitSecrets, false, "hide secret details when showing diff")
 	RootCmd.AddCommand(diffCmd)
 }
 
+// nagStrategy nags the user about selecting strategy=subset explicitly -
+// either hint at not having to use it if subset is specified, or fail hard if
+// something else is set.
+//
+// TODO(b/49): remove this
+func nagStrategy(chosen string) {
+	if chosen == "" {
+		return
+	}
+	color := isatty.IsTerminal(os.Stdout.Fd())
+
+	if chosen == "subset" {
+		if color {
+			fmt.Fprintf(os.Stdout, "\x1b[92m")
+		}
+		fmt.Fprintf(os.Stdout, "--diff-strategy=subset is now the default behaviour of kartongips/kubecfg, no need to explicitly set it.\n")
+		fmt.Fprintf(os.Stdout, "Work on your muscle memory and fix your scripts! This flag will be removed soon (see: b.hswaw.net/49).\n")
+		if color {
+			fmt.Fprintf(os.Stdout, "\x1b[0m")
+		}
+		return
+	}
+
+	fmt.Fprintf(os.Stderr, "--diff-strategy is deprecated, the default behaviour is now 'subset' and all other modes of operation have been removed. See: b.hswaw.net/49.\n")
+	os.Exit(1)
+}
+
 var diffCmd = &cobra.Command{
 	Use:   "diff",
 	Short: "Display differences between server and local config",
@@ -42,10 +74,11 @@ var diffCmd = &cobra.Command{
 
 		c := kubecfg.DiffCmd{}
 
-		c.DiffStrategy, err = flags.GetString(flagDiffStrategy)
+		diffStrategy, err := flags.GetString(flagDiffStrategy)
 		if err != nil {
 			return err
 		}
+		nagStrategy(diffStrategy)
 
 		c.OmitSecrets, err = flags.GetBool(flagOmitSecrets)
 		if err != nil {
@@ -67,6 +100,8 @@ var diffCmd = &cobra.Command{
 			return err
 		}
 
-		return c.Run(cmd.Context(), objs, cmd.OutOrStdout())
+		err = c.Run(cmd.Context(), objs, cmd.OutOrStdout())
+		nagStrategy(diffStrategy)
+		return err
 	},
 }

cluster/tools/kartongips/pkg/kubecfg/diff.go

@@ -50,8 +50,6 @@ type DiffCmd struct {
 	Mapper           meta.RESTMapper
 	DefaultNamespace string
 	OmitSecrets      bool
-
-	DiffStrategy string
 }
 
 func (c DiffCmd) Run(ctx context.Context, apiObjects []*unstructured.Unstructured, out io.Writer) error {
@@ -89,10 +87,8 @@ func (c DiffCmd) Run(ctx context.Context, apiObjects []*unstructured.Unstructure
 		}
 
 		liveObjObject := liveObj.Object
-		if c.DiffStrategy == "subset" {
-			liveObjObject = removeMapFields(obj.Object, liveObjObject)
-			liveObjObject = removeClusterRoleAggregatedRules(liveObjObject)
-		}
+		liveObjObject = removeMapFields(obj.Object, liveObjObject)
+		liveObjObject = removeClusterRoleAggregatedRules(liveObjObject)
 
 		liveObjText, _ := json.MarshalIndent(liveObjObject, "", "  ")
 		objText, _ := json.MarshalIndent(obj.Object, "", "  ")