Repository: linkerd/linkerd2
Pull request #5075 (2020-10-13 17:19:51)

Rename --addon-config flag to --config
The `--addon-config` flag allows users to supply a yaml config file which will override the Values used to install or upgrade Linkerd.  While this is useful for supplying config for addons, it can be used to configure any part of the Values struct.  Thus, we rename the flag to `--config`.

Signed-off-by: Alex Leong <alex@buoyant.io>

<!--  Thanks for sending a pull request!

If you already have a well-structured git commit message, chances are GitHub
set the title and description of this PR to the git commit message subject and
body, respectively. If so, you may delete these instructions and submit your PR.

If this is your first time, please read our contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md

The title and description of your Pull Request should match the git commit
subject and body, respectively. Git commit messages are structured as follows:

```
Subject

Problem

Solution

Validation

Fixes #[GitHub issue ID]

DCO Sign off
```

Example git commit message:

```
Introduce Pull Request Template

GitHub's community guidelines recommend a pull request template, the repo was
lacking one.

Introduce a `PULL_REQUEST_TEMPLATE.md` file.

Once merged, the
[Community profile checklist](https://github.com/linkerd/linkerd2/community)
should indicate the repo now provides a pull request template.

Fixes #3321

Signed-off-by: Jane Smith <jane.smith@example.com>
```

Note the git commit message subject becomes the pull request title.

For more details around git commits, see the section on Committing in our
contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md#committing
-->



Patch:
diff --git a/cli/cmd/options.go b/cli/cmd/options.go
index 4be54d5e0..25fea92d5 100644
--- a/cli/cmd/options.go
+++ b/cli/cmd/options.go
@@ -236,19 +236,14 @@ func makeAllStageFlags(defaults *l5dcharts.Values) ([]flag.Flag, *pflag.FlagSet)
 				return nil
 			}),
 
-		flag.NewStringFlag(allStageFlags, "addon-config", "",
-			"A path to a configuration file of add-ons. If add-on config already exists, this new config gets merged with the existing one (unless --addon-overwrite is used)",
+		flag.NewStringFlag(allStageFlags, "config", "",
+			"A path to a yaml configuration file. The fields in this file will override the values used to install or upgrade Linkerd.",
 			func(values *l5dcharts.Values, value string) error {
 				if value != "" {
 					data, err := ioutil.ReadFile(value)
 					if err != nil {
 						return err
 					}
-					addonConfigs := make(map[string]interface{})
-					err = yaml.Unmarshal(data, &addonConfigs)
-					if err != nil {
-						return err
-					}
 					err = yaml.Unmarshal(data, &values)
 					if err != nil {
 						return err
diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go
index 9d2b6134c..69427c79f 100644
--- a/cli/cmd/upgrade.go
+++ b/cli/cmd/upgrade.go
@@ -206,7 +206,7 @@ func makeUpgradeFlags() *pflag.FlagSet {
 	)
 	upgradeFlags.BoolVar(
 		&addOnOverwrite, "addon-overwrite", false,
-		"Overwrite (instead of merge) existing add-ons config with file in --addon-config (or reset to defaults if no new config is passed)",
+		"Overwrite add-on configuration instead of loading the existing config (or reset to defaults if no new config is specified)",
 	)
 	return upgradeFlags
 }
diff --git a/cli/cmd/upgrade_legacy.go b/cli/cmd/upgrade_legacy.go
index 02927d5a7..d43a56434 100644
--- a/cli/cmd/upgrade_legacy.go
+++ b/cli/cmd/upgrade_legacy.go
@@ -13,6 +13,7 @@ import (
 	"github.com/linkerd/linkerd2/pkg/issuercerts"
 	"github.com/linkerd/linkerd2/pkg/k8s"
 	"github.com/linkerd/linkerd2/pkg/version"
+	log "github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes"
@@ -75,6 +76,15 @@ func loadStoredValuesLegacy(ctx context.Context, k *k8s.KubernetesAPI) (*charts.
 				return nil, fmt.Errorf("values subpath not found in %s configmap", k8s.AddOnsConfigMapName)
 			}
 
+			// repair Add-On configs
+			repairedCm, err := repairAddOnConfig([]byte(cmData))
+			if err == nil {
+				// Update only if there is no error
+				cmData = string(repairedCm)
+			} else {
+				log.Warnf("add-on config repair failed: %s", err)
+			}
+
 			if err = yaml.Unmarshal([]byte(cmData), &values); err != nil {
 				return nil, err
 			}
@@ -84,6 +94,47 @@ func loadStoredValuesLegacy(ctx context.Context, k *k8s.KubernetesAPI) (*charts.
 	return values, nil
 }
 
+func repairAddOnConfig(rawValues []byte) ([]byte, error) {
+
+	var values map[string]interface{}
+	err := yaml.Unmarshal(rawValues, &values)
+	if err != nil {
+		return nil, err
+	}
+
+	// Grafana Depreciation Fix
+	// Convert into Map instead of Values, as the latter returns with empty values
+	if grafana, err := healthcheck.GetMap(values, "grafana"); err == nil {
+		image, err := healthcheck.GetMap(grafana, "image")
+		if err == nil {
+			// Remove image.name tag if only name is present and set to the older image tag
+			if val, err := healthcheck.GetString(image, "name"); err == nil && val == "gcr.io/linkerd-io/grafana" {
+				delete(image, "name")
+			}
+
+			// Remove image tag if its a empty map
+			if len(image) == 0 {
+				delete(grafana, "image")
+			}
+		}
+
+		// Handle removal of grafana.name field
+		name, err := healthcheck.GetString(grafana, "name")
+		if err == nil {
+			// If default, remove it as its no longer needed
+			if name == "linkerd-grafana" {
+				delete(grafana, "name")
+			}
+		}
+
+	}
+	rawValues, err = yaml.Marshal(values)
+	if err != nil {
+		return nil, err
+	}
+	return rawValues, nil
+}
+
 func setFlagsFromInstall(flags *pflag.FlagSet, installFlags []*pb.Install_Flag) {
 	for _, i := range installFlags {
 		if f := flags.Lookup(i.GetName()); f != nil && !f.Changed {
diff --git a/pkg/healthcheck/healthcheck_addons.go b/pkg/healthcheck/healthcheck_addons.go
index ea7c0b3ea..1fe027ef3 100644
--- a/pkg/healthcheck/healthcheck_addons.go
+++ b/pkg/healthcheck/healthcheck_addons.go
@@ -103,7 +103,7 @@ func (hc *HealthChecker) addOnCategories() []category {
 					warning:     true,
 					check: func(ctx context.Context) error {
 						if grafana, ok := hc.addOns[l5dcharts.GrafanaAddOn]; ok {
-							name, err := getString(grafana, "name")
+							name, err := GetString(grafana, "name")
 							if err != nil && !errors.Is(err, errorKeyNotFound) {
 								return err
 							}
@@ -123,7 +123,7 @@ func (hc *HealthChecker) addOnCategories() []category {
 					warning:     true,
 					check: func(ctx context.Context) error {
 						if grafana, ok := hc.addOns[l5dcharts.GrafanaAddOn]; ok {
-							name, err := getString(grafana, "name")
+							name, err := GetString(grafana, "name")
 							if err != nil && !errors.Is(err, errorKeyNotFound) {
 								return err
 							}
@@ -172,9 +172,9 @@ func (hc *HealthChecker) addOnCategories() []category {
 					check: func(ctx context.Context) error {
 						if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
 
-							collector, mapError := getMap(tracing, "collector")
+							collector, mapError := GetMap(tracing, "collector")
 
-							collectorName, keyError := getString(collector, "name")
+							collectorName, keyError := GetString(collector, "name")
 
 							if errors.Is(mapError, errorKeyNotFound) || errors.Is(keyError, errorKeyNotFound) {
 								// default name of collector instance
@@ -198,9 +198,9 @@ func (hc *HealthChecker) addOnCategories() []category {
 					warning:     true,
 					check: func(ctx context.Context) error {
 						if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
-							jaeger, mapError := getMap(tracing, "jaeger")
+							jaeger, mapError := GetMap(tracing, "jaeger")
 
-							jaegerName, keyError := getString(jaeger, "name")
+							jaegerName, keyError := GetString(jaeger, "name")
 
 							if errors.Is(mapError, errorKeyNotFound) || errors.Is(keyError, errorKeyNotFound) {
 								// default name of jaeger instance
@@ -224,9 +224,9 @@ func (hc *HealthChecker) addOnCategories() []category {
 					warning:     true,
 					check: func(ctx context.Context) error {
 						if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok {
-							collector, mapError := getMap(tracing, "collector")
+							collector, mapError := GetMap(tracing, "collector")
 
-							collectorName, keyError := getString(collector, "name")
+							collectorName, keyError := GetString(collector, "name")
 
 							if errors.Is(mapError, errorKeyNotFound) || errors.Is(keyError, errorKeyNotFound) {
 								// default name of collector instance
@@ -342,7 +342,8 @@ func (hc *HealthChecker) checkForAddOnCM(ctx context.Context) (string, error) {
 	return values, nil
 }
 
-func getString(i interface{}, k string) (string, error) {
+// GetString returns a String with the given key if present
+func GetString(i interface{}, k string) (string, error) {
 	m, ok := i.(map[string]interface{})
 	if !ok {
 		return "", errors.New("config value is not a map")
@@ -361,7 +362,8 @@ func getString(i interface{}, k string) (string, error) {
 	return res, nil
 }
 
-func getMap(i interface{}, k string) (map[string]interface{}, error) {
+// GetMap returns a Map with the given Key if Present
+func GetMap(i interface{}, k string) (map[string]interface{}, error) {
 	m, ok := i.(map[string]interface{})
 	if !ok {
 		return nil, errors.New("config value is not a map")


Test patch:
diff --git a/cli/cmd/upgrade_test.go b/cli/cmd/upgrade_test.go
index b547c112a..6cdb6886f 100644
--- a/cli/cmd/upgrade_test.go
+++ b/cli/cmd/upgrade_test.go
@@ -282,7 +282,7 @@ func TestUpgradeTracingAddon(t *testing.T) {
 
 	install := renderInstall(t, installOpts)
 
-	flagSet.Set("addon-config", filepath.Join("testdata", "addon_config.yaml"))
+	flagSet.Set("config", filepath.Join("testdata", "addon_config.yaml"))
 
 	upgrade, err := renderUpgrade(install.String(), upgradeOpts)
 	if err != nil {
@@ -331,7 +331,7 @@ func TestUpgradeOverwriteTracingAddon(t *testing.T) {
 
 	install := renderInstall(t, installOpts)
 
-	flagSet.Set("addon-config", filepath.Join("testdata", "addon_config_overwrite.yaml"))
+	flagSet.Set("config", filepath.Join("testdata", "addon_config_overwrite.yaml"))
 	flagSet.Set("trace-collector", "overwrite-collector")
 	flagSet.Set("trace-collector-svc-account", "overwrite-collector.default")
 
@@ -511,7 +511,7 @@ func TestUpgradeEnableAddon(t *testing.T) {
 
 	install := renderInstall(t, installOpts)
 
-	flagSet.Set("addon-config", filepath.Join("testdata", "grafana_enabled.yaml"))
+	flagSet.Set("config", filepath.Join("testdata", "grafana_enabled.yaml"))
 
 	upgrade, err := renderUpgrade(install.String(), upgradeOpts)
 	if err != nil {
@@ -563,7 +563,7 @@ func TestUpgradeRemoveAddonKeys(t *testing.T) {
 
 	install := renderInstall(t, installOpts)
 
-	flagSet.Set("addon-config", filepath.Join("testdata", "grafana_enabled.yaml"))
+	flagSet.Set("config", filepath.Join("testdata", "grafana_enabled.yaml"))
 
 	upgrade, err := renderUpgrade(install.String(), upgradeOpts)
 	if err != nil {
@@ -597,7 +597,7 @@ func TestUpgradeOverwriteRemoveAddonKeys(t *testing.T) {
 
 	install := renderInstall(t, installOpts)
 
-	flagSet.Set("addon-config", filepath.Join("testdata", "grafana_enabled.yaml"))
+	flagSet.Set("config", filepath.Join("testdata", "grafana_enabled.yaml"))
 	flagSet.Set("addon-override", "true")
 
 	upgrade, err := renderUpgrade(install.String(), upgradeOpts)
diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go
index 3c6cee08b..85b6f4b35 100644
--- a/pkg/healthcheck/healthcheck_test.go
+++ b/pkg/healthcheck/healthcheck_test.go
@@ -3567,7 +3567,7 @@ func TestGetString(t *testing.T) {
 	for i, tc := range testCases {
 		tc := tc //pin
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
-			ans, err := getString(tc.i, tc.k)
+			ans, err := GetString(tc.i, tc.k)
 
 			if ans != tc.expected {
 				t.Logf("Expected value: %s\n", tc.expected)