Handle grafana add-on config repair (#5059)

* Handle grafana add-on config repair

Fixes #5014

In Grafana Add-On, Default fields i.e `grafana.image.name`, `grafana.name`
have been removed from `linkerd-config-addons` after `2.8.1`. Only
overriden values are stored in `linkerd-config-addons` as of now.
Hence, `grafana.image.name` has to be removed from
`linkerd-config-addons` unless they are overriden so that updates
to it can take place especially the move from `gcr` to `ghcr`.

This also removes `grafana.name` field if they are set to default, as
its removed.

This problem will not occur again even if we update default values, as
default values are not stored in `linekrd-config-addons` anymore for all
add-ons.

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

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 {

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")

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)