Apply overrides to chart values on install #15025

.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