fix(CLI): Apply overrides to chart values on install #15025 (#15181)

Running `linkerd upgrade` fails after running install with an external ca.  The issue presents at upgrade but the cause is due to install incorrectly initializing the chart values for identity, specifically it initializes the issuer versus querying for one that is already set.

```bash
linkerd install \
  --ha \
  --set proxy.metrics.hostnameLabels=true \
  --set identity.externalCA=true \
  --set identity.issuer.scheme=kubernetes.io/tls \
  --set policyValidator.externalSecret=true \
  --set policyValidator.injectCaFrom=cert-manager/webhook-ca \
  --set proxyInjector.externalSecret=true \
  --set proxyInjector.injectCaFrom=cert-manager/webhook-ca \
  --set profileValidator.externalSecret=true \
  --set profileValidator.injectCaFrom=cert-manager/webhook-ca \
  --ignore-cluster
```

The install process did not apply the '--set' command line options onto internal values state before attempting to initialize the issuer credentials.

 * The change applies the overrides to the values before initialize.
 * Add test code to cover initializeIssuerCredentials.
 * Track unit test run history in 'go-test.json' in the root directory when running in the justfile.
 * Ignore 'go-test.json.'
 * Add testify assertions as a go module.

Added unit test and ran through the integration test suite.

Fixes #15025

Author: raykroeker

.gitignore

@@ -14,6 +14,7 @@ web/app/yarn-error.log
 vendor
 **/*.swp
 **/charts/**/charts
+go-test.json
 package-lock.json
 .vscode
 **/coverage*

cli/cmd/install.go

@@ -303,6 +303,21 @@ func installControlPlane(ctx context.Context, k8sAPI *k8s.KubernetesAPI, w io.Wr
 		}
 	}
 
+	// in order to correctly initialize the issuer credentials the overrides
+	// (from above) need to be set/applied to the values themselves
+	// specifically identity issuer scheme, and trust values
+	//
+	// marshal+unmarshal here will only apply specific overrides to values and
+	// will not wipe out values that are not set.
+	data, err := yaml.Marshal(valuesOverrides)
+	if err != nil {
+		return err
+	}
+	err = yaml.Unmarshal(data, values)
+	if err != nil {
+		return err
+	}
+
 	err = initializeIssuerCredentials(ctx, k8sAPI, values)
 	if err != nil {
 		return err

cli/cmd/install_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -16,6 +17,7 @@ import (
 	"github.com/linkerd/linkerd2/pkg/tls"
 	"helm.sh/helm/v3/pkg/cli/values"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 const (
@@ -314,6 +316,240 @@ func TestRender(t *testing.T) {
 	}
 }
 
+// TestOverrideIssuer calls install control plane with the goal of testing
+// options overrides for initialize issuer credentials.
+func TestOverrideIssuer(t *testing.T) {
+	removeIssuerCrt := func() (*charts.Values, error) {
+		t.Helper()
+		values, err := testInstallOptionsFakeCerts()
+		if err != nil {
+			return nil, err
+		}
+		values.Identity.Issuer.TLS.CrtPEM = ""
+		return values, nil
+	}
+	removeIssuerKey := func() (*charts.Values, error) {
+		t.Helper()
+		values, err := testInstallOptionsFakeCerts()
+		if err != nil {
+			return nil, err
+		}
+		values.Identity.Issuer.TLS.KeyPEM = ""
+		return values, nil
+	}
+	removeTrustAnchor := func() (*charts.Values, error) {
+		t.Helper()
+		values, err := testInstallOptionsFakeCerts()
+		if err != nil {
+			return nil, err
+		}
+		values.IdentityTrustAnchorsPEM = ""
+		return values, nil
+	}
+	read := func(filename string) []byte {
+		t.Helper()
+		data, err := os.ReadFile(path.Join("testdata", filename))
+		if err != nil {
+			t.Fatalf("cannot read filename=%s err=%v", filename, err)
+		}
+		return data
+	}
+	// newK8S returns a test implementation of the k8s API; after setting the
+	// issuer trust anchor and tls crt+key as a secret.
+	newK8S := func() *k8s.KubernetesAPI {
+		t.Helper()
+		api, err := k8s.NewFakeAPI()
+		if err != nil {
+			t.Fatalf("cannot create new fake-api from manifests err=%v", err)
+		}
+		_, err = api.CoreV1().Secrets(controlPlaneNamespace).Create(context.Background(),
+			&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      k8s.IdentityIssuerSecretName,
+					Namespace: controlPlaneNamespace,
+				},
+				Data: map[string][]byte{
+					k8s.IdentityIssuerTrustAnchorsNameExternal: read("valid-trust-anchors.pem"),
+					corev1.TLSCertKey:                          read("valid-crt.pem"),
+					corev1.TLSPrivateKeyKey:                    read("valid-key.pem"),
+				}}, metav1.CreateOptions{})
+		if err != nil {
+			t.Fatalf("cannot create secret for new-k8s-api err=%v", err)
+		}
+		return api
+	}
+	controlPlaneNamespace = defaultLinkerdNamespace
+	for i, test := range []struct {
+		options                values.Options
+		values                 func() (*charts.Values, error)
+		k8sAPI                 *k8s.KubernetesAPI
+		expErr                 string
+		expIdentityTrustAnchor bool
+		expIssuerCrt           bool
+		expIssuerKey           bool
+		expIssuerName          string
+	}{
+		{
+			// no options; no certs in values -> generated anchor; key + crt
+			options:                values.Options{},
+			values:                 testInstallValuesNoCertsNoHA,
+			k8sAPI:                 nil,
+			expIdentityTrustAnchor: true,
+			expIssuerKey:           true,
+			expIssuerCrt:           true,
+			expIssuerName: fmt.Sprintf("identity.%s.%s",
+				controlPlaneNamespace, "test-override-issuer"),
+		},
+		{
+			// no options; fake certs in values -> fake certs untouched
+			options:                values.Options{},
+			values:                 testInstallOptionsFakeCerts,
+			k8sAPI:                 nil,
+			expIdentityTrustAnchor: true,
+			expIssuerKey:           true,
+			expIssuerCrt:           true,
+			expIssuerName:          "identity.linkerd.cluster.local",
+		},
+		{
+			// issuer scheme in options; no certs in values; nil k8s api ->
+			// error trying to call k8s
+			options: values.Options{
+				Values: []string{"identity.issuer.scheme=kubernetes.io/tls"},
+			},
+			values:                 testInstallValuesNoCertsNoHA,
+			k8sAPI:                 nil,
+			expErr:                 "--ignore-cluster is not supported when --identity-external-issuer=true",
+			expIdentityTrustAnchor: false,
+			expIssuerKey:           false,
+			expIssuerCrt:           false,
+			expIssuerName:          "",
+		},
+		{
+			// issuer scheme in options; no certs in values; fake k8s api ->
+			// trust anchor is set
+			options: values.Options{
+				Values: []string{"identity.issuer.scheme=kubernetes.io/tls"},
+			},
+			values:                 testInstallValuesNoCertsNoHA,
+			k8sAPI:                 newK8S(),
+			expErr:                 "",
+			expIdentityTrustAnchor: true,
+			expIssuerKey:           false,
+			expIssuerCrt:           false,
+			expIssuerName:          "identity.linkerd.cluster.local",
+		},
+		{
+			// no options; fake certs in values; remove trust anchor -> err
+			options:                values.Options{},
+			values:                 removeTrustAnchor,
+			k8sAPI:                 nil,
+			expErr:                 "a trust anchors file must be specified if other credentials are provided",
+			expIdentityTrustAnchor: false,
+			expIssuerCrt:           true,
+			expIssuerKey:           true,
+			expIssuerName:          "identity.linkerd.cluster.local",
+		},
+		{
+			// no options; fake certs in values; remove issuer crt -> err
+			options:                values.Options{},
+			values:                 removeIssuerCrt,
+			k8sAPI:                 nil,
+			expErr:                 "a certificate file must be specified if other credentials are provided",
+			expIdentityTrustAnchor: true,
+			expIssuerCrt:           false,
+			expIssuerName:          "identity.linkerd.cluster.local",
+			expIssuerKey:           true,
+		},
+		{
+			// no options; fake certs in values; remove issuer key -> err
+			options:                values.Options{},
+			values:                 removeIssuerKey,
+			k8sAPI:                 nil,
+			expErr:                 "a private key file must be specified if other credentials are provided",
+			expIdentityTrustAnchor: true,
+			expIssuerCrt:           true,
+			expIssuerName:          "identity.linkerd.cluster.local",
+			expIssuerKey:           false,
+		},
+	} {
+		values, err := test.values()
+		if err != nil {
+			t.Fatalf("%02d/test install options failed with an error err=%v", i, err)
+		}
+		values.IdentityTrustDomain = "test-override-issuer"
+		// ensure the install options created above meet expectations (we are
+		// testing the override not the values)
+		if values.Identity.Issuer.Scheme != k8s.IdentityIssuerSchemeLinkerd {
+			t.Fatalf("%02d/identity issuer scheme is incorrect: %s != %s", i,
+				k8s.IdentityIssuerSchemeLinkerd, values.Identity.Issuer.Scheme)
+		}
+		var buf bytes.Buffer
+		err = installControlPlane(context.Background(), test.k8sAPI, &buf, values, nil, test.options, "yaml")
+		if test.expErr != "" {
+			if test.expErr != err.Error() {
+				t.Fatalf("%02d/install control plane returned incorrect error %s<>%v", i, test.expErr, err)
+			}
+		} else {
+			if err != nil {
+				t.Fatalf("%02d/install control plane failed with an error=%v", i, err)
+			}
+		}
+		if test.expIdentityTrustAnchor {
+			if values.IdentityTrustAnchorsPEM == "" {
+				t.Fatalf("%02d/identity trust-anchors-pem is empty", i)
+			}
+			crt, err := tls.DecodePEMCrt(values.IdentityTrustAnchorsPEM)
+			if err != nil {
+				t.Fatalf("%02d/generated identity-trust-anchors-pem cannot be decoded", i)
+			}
+			if crt == nil {
+				t.Fatalf("%02d/generated identity-trust-anchors-pem cannot be decoded (nil)", i)
+			}
+			if crt.Certificate == nil {
+				t.Fatalf("%02d/generated identity-trust-anchors-pem certificate is invalid", i)
+			}
+			if test.expIssuerName != crt.Certificate.Issuer.CommonName {
+				t.Fatalf("%02d/generated identity-trust-anchors-pem certificate common-name is incorrect %s<>%s", i,
+					test.expIssuerName,
+					crt.Certificate.Issuer.CommonName)
+			}
+		} else if values.IdentityTrustAnchorsPEM != "" {
+			t.Fatalf("%02d/identity was incorrectly set pem=%s", i, values.IdentityTrustAnchorsPEM)
+		}
+		if test.expIssuerCrt {
+			if values.Identity.Issuer.TLS.CrtPEM == "" {
+				t.Fatalf("%02d/generated identity-issuer-tls-crt-pem is empty", i)
+			}
+			crt, err := tls.DecodePEMCrt(values.Identity.Issuer.TLS.CrtPEM)
+			if err != nil {
+				t.Fatalf("%02d/generated identity-issuer-tls-crt-pem cannot be decoded err=%v", i, err)
+			}
+			if crt == nil {
+				t.Fatalf("%02d/generated identity-issuer-tls-crt-pem cannot be decoded (nil)", i)
+			}
+			if crt.Certificate == nil {
+				t.Fatalf("%02d/generated identity-issuer-tls-crt-pem certificate is invalid (nil)", i)
+			}
+		} else if values.Identity.Issuer.TLS.CrtPEM != "" {
+			t.Fatalf("%02d/identity issuer crt was incorrectly set pem=%s", i, values.Identity.Issuer.TLS.CrtPEM)
+		}
+		if test.expIssuerKey {
+			if values.Identity.Issuer.TLS.KeyPEM == "" {
+				t.Fatalf("%02d/generated identity-issuer-tls-key-pem is empty", i)
+			}
+			key, err := tls.DecodePEMKey(values.Identity.Issuer.TLS.KeyPEM)
+			if err != nil {
+				t.Fatalf("%02d/generated identity-issuer-tls-key-pem cannot be decoded err=%v", i, err)
+			}
+			if key == nil {
+				t.Fatalf("%02d/generated identity-issuer-tls-key-pem cannot be decoded (nil)", i)
+			}
+		} else if values.Identity.Issuer.TLS.KeyPEM != "" {
+			t.Fatalf("%02d/identity issuer tls key was incorrectly set pem=%s", i, values.Identity.Issuer.TLS.KeyPEM)
+		}
+	}
+}
+
 func TestIgnoreCluster(t *testing.T) {
 	defaultValues, err := testInstallOptions()
 	if err != nil {
@@ -550,6 +786,10 @@ func testInstallOptionsNoCerts(ha bool) (*charts.Values, error) {
 	return values, nil
 }
 
+func testInstallValuesNoCertsNoHA() (*charts.Values, error) {
+	return testInstallOptionsNoCerts(false)
+}
+
 func testInstallValues() (*charts.Values, error) {
 	values, err := charts.NewValues()
 	if err != nil {

justfile

@@ -20,7 +20,7 @@ go-lint *flags:
     golangci-lint run {{ flags }}
 
 go-test:
-    LINKERD_TEST_PRETTY_DIFF=1 gotestsum -- -race -v -mod=readonly --timeout 10m ./...
+    LINKERD_TEST_PRETTY_DIFF=1 gotestsum --jsonfile go-test.json -- -race -v -mod=readonly --timeout 10m ./...
 
 ##
 ## Rust