Config file and env var for istioctl --istioNamespace, --xds-address, and --cert-dir (istio#25280)

* Config file and env var for some istioctl CLI options

* ISTIOCONFIG variable for overriding default istioctl configuration

* 'prefer-experimental' option for commands with regular and experimental variants

* Defaults for --xds-san and --insecure

* Column for 'istioctl x config list' that lets users tell defaults from configured values

* Fail if user supplies invalid ISTIOCONFIG env var

* Don't fail if config file does not exist

* Initialize defaults in tests

* Show origin of config var; move defaulting close to command so tests work

* Use Istio RegisterXXXVar for environment overrides to istioctl

* Allow user to make XDS-based proxy-status the default with env or config setting

* Added release note

* Sort 'x config list' output

* release note fix

Author: esnible

Diff for: istioctl/cmd/config.go

@@ -0,0 +1,140 @@
+// Copyright Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cmd
+
+import (
+	"fmt"
+	"io"
+	"sort"
+	"text/tabwriter"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+
+	"istio.io/istio/pilot/pkg/serviceregistry/kube/controller"
+	"istio.io/pkg/env"
+)
+
+var (
+	// settableFlags are the flags used to istioctl
+	settableFlags = map[string]interface{}{
+		"istioNamespace":      env.RegisterStringVar("ISTIOCTL_ISTIONAMESPACE", controller.IstioNamespace, "istioctl --istioNamespace override"),
+		"xds-address":         env.RegisterStringVar("ISTIOCTL_XDS_ADDRESS", "", "istioctl --xds-address override"),
+		"xds-port":            env.RegisterIntVar("ISTIOCTL_XDS_PORT", 15012, "istioctl --xds-port override"),
+		"xds-san":             env.RegisterStringVar("ISTIOCTL_XDS_SAN", "", "istioctl --xds-san override"),
+		"cert-dir":            env.RegisterStringVar("ISTIOCTL_CERT_DIR", "", "istioctl --cert-dir override"),
+		"insecure":            env.RegisterBoolVar("ISTIOCTL_INSECURE", false, "istioctl --insecure override"),
+		"prefer-experimental": env.RegisterBoolVar("ISTIOCTL_PREFER_EXPERIMENTAL", false, "istioctl should use experimental subcommand variants"),
+	}
+)
+
+// configCmd represents the config subcommand command
+func configCmd() *cobra.Command {
+	configCmd := &cobra.Command{
+		Use:   "config SUBCOMMAND",
+		Short: "Gonfigure istioctl defaults",
+		Args:  cobra.NoArgs,
+		Example: `
+# list configuration parameters
+istioctl config list
+`,
+	}
+	configCmd.AddCommand(listCommand())
+	return configCmd
+}
+
+func listCommand() *cobra.Command {
+	listCmd := &cobra.Command{
+		Use:   "list",
+		Short: "List istio configurable defaults",
+		Args:  cobra.ExactArgs(0),
+		RunE: func(c *cobra.Command, _ []string) error {
+			scope.Debugf("Config file %q", IstioConfig)
+			return runList(c.OutOrStdout())
+		},
+	}
+	return listCmd
+}
+
+func runList(writer io.Writer) error {
+	// Sort flag names
+	keys := make([]string, len(settableFlags))
+	i := 0
+	for key := range settableFlags {
+		keys[i] = key
+		i++
+	}
+	sort.Strings(keys)
+
+	w := new(tabwriter.Writer).Init(writer, 0, 8, 5, ' ', 0)
+	fmt.Fprintf(w, "FLAG\tVALUE\tFROM\n")
+	for _, flag := range keys {
+		v := settableFlags[flag]
+		fmt.Fprintf(w, "%s\t%s\t%v\n", flag, viper.GetString(flag), configSource(flag, v))
+	}
+	return w.Flush()
+}
+
+func configSource(flag string, v interface{}) string {
+	// Environment variables have high precedence in Viper
+	if isVarSet(v) {
+		return "$" + getVarVar(v).Name
+	}
+
+	if viper.InConfig(flag) {
+		return IstioConfig
+	}
+
+	return "default"
+}
+
+func getVarVar(v interface{}) env.Var {
+	switch ev := v.(type) {
+	case env.StringVar:
+		return ev.Var
+	case env.BoolVar:
+		return ev.Var
+	case env.IntVar:
+		return ev.Var
+	case env.DurationVar:
+		return ev.Var
+	case env.FloatVar:
+		return ev.Var
+	default:
+		panic(fmt.Sprintf("Unexpected environment var type %v", v))
+	}
+}
+
+func isVarSet(v interface{}) bool {
+	switch ev := v.(type) {
+	case env.StringVar:
+		_, ok := ev.Lookup()
+		return ok
+	case env.BoolVar:
+		_, ok := ev.Lookup()
+		return ok
+	case env.IntVar:
+		_, ok := ev.Lookup()
+		return ok
+	case env.DurationVar:
+		_, ok := ev.Lookup()
+		return ok
+	case env.FloatVar:
+		_, ok := ev.Lookup()
+		return ok
+	default:
+		panic(fmt.Sprintf("Unexpected environment var type %v", v))
+	}
+}

Diff for: istioctl/cmd/config_test.go

@@ -0,0 +1,52 @@
+// Copyright Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cmd
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+func TestConfigList(t *testing.T) {
+
+	cases := []testCase{
+		{ // case 0
+			args:           strings.Split("experimental config get istioNamespace", " "),
+			expectedRegexp: regexp.MustCompile("Usage:\n  istioctl experimental config"),
+			wantException:  false,
+		},
+		{ // case 1
+			args: strings.Split("experimental config list", " "),
+			expectedOutput: `FLAG                    VALUE            FROM
+cert-dir                                 default
+insecure                                 default
+istioNamespace          istio-system     default
+prefer-experimental                      default
+xds-address                              default
+xds-port                15012            default
+xds-san                                  default
+`,
+			wantException: false,
+		},
+	}
+
+	for i, c := range cases {
+		t.Run(fmt.Sprintf("case %d %s", i, strings.Join(c.args, " ")), func(t *testing.T) {
+			verifyOutput(t, c)
+		})
+	}
+}

Diff for: istioctl/cmd/istioctl/main.go

@@ -15,6 +15,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 
 	// import all known client auth plugins
@@ -24,6 +25,12 @@ import (
 )
 
 func main() {
+	if err := cmd.ConfigAndEnvProcessing(); err != nil {
+		fmt.Fprintf(os.Stderr, "Could not initialize: %v\n", err)
+		exitCode := cmd.GetExitCode(err)
+		os.Exit(exitCode)
+	}
+
 	rootCmd := cmd.GetRootCmd(os.Args[1:])
 
 	if err := rootCmd.Execute(); err != nil {

Diff for: istioctl/cmd/root.go

@@ -17,9 +17,12 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"path/filepath"
+	"strings"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/cobra/doc"
+	"github.com/spf13/viper"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/clientcmd"
 
@@ -30,6 +33,7 @@ import (
 	"istio.io/istio/pilot/pkg/serviceregistry/kube/controller"
 	"istio.io/istio/pkg/cmd"
 	"istio.io/pkg/collateral"
+	"istio.io/pkg/env"
 	"istio.io/pkg/log"
 )
 
@@ -42,7 +46,16 @@ func (c CommandParseError) Error() string {
 	return c.e.Error()
 }
 
+const (
+	// Location to read istioctl defaults from
+	defaultIstioctlConfig = "$HOME/.istioctl/config.yaml"
+)
+
 var (
+	// IstioConfig is the name of the istioctl config file (if any)
+	IstioConfig = env.RegisterStringVar("ISTIOCONFIG", defaultIstioctlConfig,
+		"Default values for istioctl flags").Get()
+
 	kubeconfig       string
 	configContext    string
 	namespace        string
@@ -56,6 +69,9 @@ var (
 	kubeClient = newKubeClient
 
 	loggingOptions = defaultLogOptions()
+
+	// scope is for dev logging.  Warning: log levels are not set by --log_output_level until command is Run().
+	scope = log.RegisterScope("cli", "istioctl", 0)
 )
 
 func defaultLogOptions() *log.Options {
@@ -74,6 +90,34 @@ func defaultLogOptions() *log.Options {
 	return o
 }
 
+// ConfigAndEnvProcessing uses spf13/viper for overriding CLI parameters
+func ConfigAndEnvProcessing() error {
+	configPath := filepath.Dir(IstioConfig)
+	baseName := filepath.Base(IstioConfig)
+	configType := filepath.Ext(IstioConfig)
+	configName := baseName[0 : len(baseName)-len(configType)]
+	if configType != "" {
+		configType = configType[1:]
+	}
+
+	// Allow users to override some variables through $HOME/.istioctl/config.yaml
+	// and environment variables.
+	viper.SetEnvPrefix("ISTIOCTL")
+	viper.AutomaticEnv()
+	viper.AllowEmptyEnv(true) // So we can say ISTIOCTL_CERT_DIR="" to suppress certs
+	viper.SetConfigName(configName)
+	viper.SetConfigType(configType)
+	viper.AddConfigPath(configPath)
+	viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
+	err := viper.ReadInConfig()
+	// Ignore errors reading the configuration unless the file is explicitly customized
+	if IstioConfig != defaultIstioctlConfig {
+		return err
+	}
+
+	return nil
+}
+
 // GetRootCmd returns the root of the cobra command-tree.
 func GetRootCmd(args []string) *cobra.Command {
 	rootCmd := &cobra.Command{
@@ -95,7 +139,8 @@ debug and diagnose their Istio mesh.
 	rootCmd.PersistentFlags().StringVar(&configContext, "context", "",
 		"The name of the kubeconfig context to use")
 
-	rootCmd.PersistentFlags().StringVarP(&istioNamespace, "istioNamespace", "i", controller.IstioNamespace,
+	viper.SetDefault("istioNamespace", controller.IstioNamespace)
+	rootCmd.PersistentFlags().StringVarP(&istioNamespace, "istioNamespace", "i", viper.GetString("istioNamespace"),
 		"Istio system namespace")
 
 	rootCmd.PersistentFlags().StringVarP(&namespace, "namespace", "n", v1.NamespaceAll,
@@ -111,7 +156,6 @@ debug and diagnose their Istio mesh.
 
 	cmd.AddFlags(rootCmd)
 
-	rootCmd.AddCommand(newVersionCommand())
 	rootCmd.AddCommand(register())
 	rootCmd.AddCommand(deregisterCmd)
 	rootCmd.AddCommand(injectCommand())
@@ -127,12 +171,40 @@ debug and diagnose their Istio mesh.
 		Short:   "Experimental commands that may be modified or deprecated",
 	}
 
+	xdsBasedTroubleshooting := []*cobra.Command{
+		xdsVersionCommand(),
+		xdsStatusCommand(),
+	}
+	debugBasedTroubleshooting := []*cobra.Command{
+		newVersionCommand(),
+		statusCommand(),
+	}
+	var debugCmdAttachmentPoint *cobra.Command
+	if viper.GetBool("PREFER-EXPERIMENTAL") {
+		legacyCmd := &cobra.Command{
+			Use:   "legacy",
+			Short: "Legacy command variants",
+		}
+		rootCmd.AddCommand(legacyCmd)
+		for _, c := range xdsBasedTroubleshooting {
+			rootCmd.AddCommand(c)
+		}
+		debugCmdAttachmentPoint = legacyCmd
+	} else {
+		debugCmdAttachmentPoint = rootCmd
+	}
+	for _, c := range xdsBasedTroubleshooting {
+		experimentalCmd.AddCommand(c)
+	}
+	for _, c := range debugBasedTroubleshooting {
+		debugCmdAttachmentPoint.AddCommand(c)
+	}
+
 	rootCmd.AddCommand(experimentalCmd)
 	rootCmd.AddCommand(proxyConfig())
 
 	rootCmd.AddCommand(convertIngress())
 	rootCmd.AddCommand(dashboard())
-	rootCmd.AddCommand(statusCommand())
 	rootCmd.AddCommand(Analyze())
 
 	rootCmd.AddCommand(install.NewVerifyCommand())
@@ -150,9 +222,7 @@ debug and diagnose their Istio mesh.
 	experimentalCmd.AddCommand(vmBootstrapCommand())
 	experimentalCmd.AddCommand(waitCmd())
 	experimentalCmd.AddCommand(mesh.UninstallCmd(loggingOptions))
-
-	experimentalCmd.AddCommand(xdsVersionCommand())
-	experimentalCmd.AddCommand(xdsStatusCommand())
+	experimentalCmd.AddCommand(configCmd())
 
 	postInstallCmd.AddCommand(Webhook())
 	experimentalCmd.AddCommand(postInstallCmd)