https://github.com/linkerd/linkerd2/pull/3552

pr-3552.diff

diff --git a/controller/cmd/identity/main.go b/controller/cmd/identity/main.go
index a6b65098..aee02021 100644
--- a/controller/cmd/identity/main.go
+++ b/controller/cmd/identity/main.go
@@ -1,6 +1,7 @@
 package identity
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"net"
@@ -94,12 +95,16 @@ func Main(args []string) {
 		}
 	}
 
-	expectedName := fmt.Sprintf("identity.%s.%s", controllerNS, trustDomain)
-	watcher := idctl.NewFsCredsWatcher(*issuerPath, keyName, crtName, expectedName, trustAnchors, validity)
+	watcher := idctl.NewFsCredsWatcher(*issuerPath)
 
-	if err := watcher.StartWatching(); err != nil {
-		log.Fatalf("Failed to start creds watcher: %s", err)
-	}
+	// need to handle this
+	errChan := make(chan error)
+	go func() {
+		// context.Background() for code review purposes only
+		if err := watcher.StartWatching(context.Background()); err != nil {
+			errChan <- err
+		}
+	}()
 
 	k8s, err := k8s.NewAPI(*kubeConfigPath, "", "", 0)
 	if err != nil {
@@ -110,7 +115,11 @@ func Main(args []string) {
 		log.Fatalf("Failed to initialize identity service: %s", err)
 	}
 
-	svc := identity.NewService(v, watcher.Creds())
+	expectedName := fmt.Sprintf("identity.%s.%s", controllerNS, trustDomain)
+	svc := identity.NewService(v)
+	go func() {
+		svc.Run(*issuerPath, keyName, crtName, expectedName, trustAnchors, validity, watcher.EventChan, watcher.ErrorChan)
+	}()
 
 	go admin.StartServer(*adminAddr)
 	lis, err := net.Listen("tcp", *addr)
diff --git a/controller/identity/creds_watcher.go b/controller/identity/creds_watcher.go
index 31b71558..60711149 100644
--- a/controller/identity/creds_watcher.go
+++ b/controller/identity/creds_watcher.go
@@ -1,12 +1,10 @@
 package identity
 
 import (
-	"crypto/x509"
-	"fmt"
+	"context"
 	"path/filepath"
 
 	"github.com/fsnotify/fsnotify"
-	"github.com/linkerd/linkerd2/pkg/tls"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -14,84 +12,56 @@ const dataDirectoryLnName = "..data"
 
 // FsCredsWatcher is used to monitor tls credentials on the filesystem
 type FsCredsWatcher struct {
-	issuerPath, keyName, crtName, expectedName string
-	roots                                      *x509.CertPool
-	validity                                   tls.Validity
-	issuerChan                                 chan tls.Issuer
+	issuerPath string
+	EventChan  chan struct{}
+	ErrorChan  chan error
 }
 
 // NewFsCredsWatcher constructs a FsCredsWatcher instance
-func NewFsCredsWatcher(issuerPath, keyName, crtName, expectedName string, roots *x509.CertPool, validity tls.Validity) *FsCredsWatcher {
-	ch := make(chan tls.Issuer, 100)
-	return &FsCredsWatcher{issuerPath, keyName, crtName, expectedName, roots, validity, ch}
-}
-
-// Creds gives back a chan from which new issuers can be read
-func (fscw *FsCredsWatcher) Creds() <-chan tls.Issuer {
-	return fscw.issuerChan
-}
-
-func (fscw *FsCredsWatcher) loadCredentials() (*tls.CA, error) {
-	creds, err := tls.ReadPEMCreds(
-		filepath.Join(fscw.issuerPath, fscw.keyName),
-		filepath.Join(fscw.issuerPath, fscw.crtName),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read CA from %s: %s", fscw.issuerPath, err)
+func NewFsCredsWatcher(issuerPath string) *FsCredsWatcher {
+	return &FsCredsWatcher{
+		issuerPath,
+		make(chan struct{}),
+		make(chan error),
 	}
-
-	if err := creds.Crt.Verify(fscw.roots, fscw.expectedName); err != nil {
-		return nil, fmt.Errorf("failed to verify issuer credentials for '%s' with trust anchors: %s", fscw.expectedName, err)
-	}
-
-	log.Infof("Loaded issuer cert:\nCert: %s\nKey: %s", creds.EncodeCertificatePEM(), creds.EncodePrivateKeyPEM())
-	return tls.NewCA(*creds, fscw.validity), nil
 }
 
 // StartWatching starts watching the filesystem for cert updates
-func (fscw *FsCredsWatcher) StartWatching() error {
+func (fscw *FsCredsWatcher) StartWatching(ctx context.Context) error {
 	watcher, err := fsnotify.NewWatcher()
 	if err != nil {
 		return err
 	}
+	defer watcher.Close()
 
-	go func() {
-		for {
-			select {
-			case event, ok := <-watcher.Events:
-				if !ok {
-					return
-				}
-				log.Debugf("Received event: %v", event)
-				// Watching the folder for create events as this indicates
-				// that the secret has been updated.
-				if event.Op&fsnotify.Create == fsnotify.Create &&
-					event.Name == filepath.Join(fscw.issuerPath, dataDirectoryLnName) {
-					log.Debugf("Reloading issuer certificate")
-					newCa, err := fscw.loadCredentials()
-					if err != nil {
-						log.Fatalf("Problem reloading %s", err)
-					}
-					fscw.issuerChan <- newCa
-				}
-			case err, ok := <-watcher.Errors:
-				if !ok {
-					return
-				}
-				log.Warnf("Error while watching %s: %s", fscw.issuerPath, err)
-			}
-		}
-	}()
-
-	err = watcher.Add(fscw.issuerPath)
-	if err != nil {
+	// no point of proceeding if we fail to watch this
+	if err := watcher.Add(fscw.issuerPath); err != nil {
 		log.Fatal(err)
 	}
 
-	initialCredentials, err := fscw.loadCredentials()
-	if err != nil {
-		return fmt.Errorf("failed to read initial credentials: %s", err)
+LOOP:
+	for {
+		select {
+		case event := <-watcher.Events:
+			log.Debugf("Received event: %v", event)
+			// Watching the folder for create events as this indicates
+			// that the secret has been updated.
+			if event.Op&fsnotify.Create == fsnotify.Create &&
+				event.Name == filepath.Join(fscw.issuerPath, dataDirectoryLnName) {
+				log.Debugf("Reloading issuer certificate")
+				fscw.EventChan <- struct{}{}
+			}
+		case err := <-watcher.Errors:
+			fscw.ErrorChan <- err
+			log.Warnf("Error while watching %s: %s", fscw.issuerPath, err)
+			break LOOP
+		case <-ctx.Done():
+			if err := ctx.Err(); err != nil {
+				fscw.ErrorChan <- err
+			}
+			break LOOP
+		}
 	}
-	fscw.issuerChan <- initialCredentials
+
 	return nil
 }
diff --git a/pkg/identity/service.go b/pkg/identity/service.go
index 232c4a5c..1394f572 100644
--- a/pkg/identity/service.go
+++ b/pkg/identity/service.go
@@ -5,6 +5,7 @@ import (
 	"crypto/x509"
 	"errors"
 	"fmt"
+	"path/filepath"
 	"sync"
 	"time"
 
@@ -59,21 +60,37 @@ type (
 )
 
 // NewService creates a new identity service.
-func NewService(v Validator, issuerChan <-chan tls.Issuer) *Service {
-
+func NewService(v Validator) *Service {
 	var issuerMutex = &sync.Mutex{}
-	service := &Service{v, nil, issuerMutex}
-
-	go func() {
-		for is := range issuerChan {
-			newIssuer := is
-			issuerMutex.Lock()
-			service.issuer = &newIssuer
-			log.Debug("Issuer has been updated")
-			issuerMutex.Unlock()
+	return &Service{v, nil, issuerMutex}
+}
+
+func (svc *Service) Run(issuerPath, keyName, crtName, expectedName string, trustAnchors *x509.CertPool, validity tls.Validity, issuerEvent <-chan struct{}, issuerError <-chan error) {
+	for {
+		select {
+		case <-issuerEvent:
+			loadCredentials(issuerPath, keyName, crtName, expectedName, trustAnchors, validity)
+		case <-issuerError:
+			// hmmmm...
 		}
-	}()
-	return service
+	}
+}
+
+func loadCredentials(issuerPath, keyName, crtName, expectedName string, trustAnchors *x509.CertPool, validity tls.Validity) (*tls.CA, error) {
+	creds, err := tls.ReadPEMCreds(
+		filepath.Join(issuerPath, keyName),
+		filepath.Join(issuerPath, crtName),
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read CA from %s: %s", issuerPath, err)
+	}
+
+	if err := creds.Crt.Verify(trustAnchors, expectedName); err != nil {
+		return nil, fmt.Errorf("failed to verify issuer credentials for '%s' with trust anchors: %s", expectedName, err)
+	}
+
+	log.Infof("Loaded issuer cert:\nCert: %s\nKey: %s", creds.EncodeCertificatePEM(), creds.EncodePrivateKeyPEM())
+	return tls.NewCA(*creds, validity), nil
 }
 
 // Register registers an identity service implementation in the provided gRPC

remove-flag.diff

diff --git a/cli/cmd/install.go b/cli/cmd/install.go
index f8d74a76..103db3e7 100644
--- a/cli/cmd/install.go
+++ b/cli/cmd/install.go
@@ -23,6 +23,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -67,7 +68,6 @@ type (
 		clockSkewAllowance time.Duration
 
 		trustPEMFile, crtPEMFile, keyPEMFile string
-		identityExternalIssuer               bool
 	}
 )
 
@@ -198,10 +198,9 @@ func newInstallOptionsWithDefaults() (*installOptions, error) {
 			enableExternalProfiles: defaults.Proxy.EnableExternalProfiles,
 		},
 		identityOptions: &installIdentityOptions{
-			trustDomain:            defaults.Identity.TrustDomain,
-			issuanceLifetime:       issuanceLifetime,
-			clockSkewAllowance:     clockSkewAllowance,
-			identityExternalIssuer: defaults.Identity.Issuer.External,
+			trustDomain:        defaults.Identity.TrustDomain,
+			issuanceLifetime:   issuanceLifetime,
+			clockSkewAllowance: clockSkewAllowance,
 		},
 
 		generateUUID: func() string {
@@ -455,10 +454,6 @@ func (options *installOptions) recordableFlagSet() *pflag.FlagSet {
 		&options.omitWebhookSideEffects, "omit-webhook-side-effects", options.omitWebhookSideEffects,
 		"Omit the sideEffects flag in the webhook manifests, This flag must be provided during install or upgrade for Kubernetes versions pre 1.12",
 	)
-	flags.BoolVar(
-		&options.identityOptions.identityExternalIssuer, "identity-external-issuer", options.identityOptions.identityExternalIssuer,
-		"Whether to use an external identity issuer (default false)",
-	)
 
 	flags.StringVarP(&options.controlPlaneVersion, "control-plane-version", "", options.controlPlaneVersion, "(Development) Tag to be used for the control plane component images")
 	flags.MarkHidden("control-plane-version")
@@ -545,10 +540,6 @@ func (options *installOptions) recordFlags(flags *pflag.FlagSet) {
 }
 
 func (options *installOptions) validate() error {
-	if options.ignoreCluster && options.identityOptions.identityExternalIssuer {
-		return errors.New("--ignore-cluster is not supported when --identity-external-issuer=true")
-	}
-
 	if options.controlPlaneVersion != "" && !alphaNumDashDot.MatchString(options.controlPlaneVersion) {
 		return fmt.Errorf("%s is not a valid version", options.controlPlaneVersion)
 	}
@@ -930,34 +921,18 @@ func (idopts *installIdentityOptions) validate() error {
 		}
 	}
 
-	if idopts.identityExternalIssuer {
-
-		if idopts.crtPEMFile != "" {
-			return errors.New("--identity-issuer-certificate-file must not be specified if --identity-external-issuer=true")
+	if idopts.trustPEMFile != "" || idopts.crtPEMFile != "" || idopts.keyPEMFile != "" {
+		if idopts.trustPEMFile == "" {
+			return errors.New("a trust anchors file must be specified if other credentials are provided")
 		}
-
-		if idopts.keyPEMFile != "" {
-			return errors.New("--identity-issuer-key-file must not be specified if --identity-external-issuer=true")
+		if idopts.crtPEMFile == "" {
+			return errors.New("a certificate file must be specified if other credentials are provided")
 		}
-
-		if idopts.trustPEMFile != "" {
-			return errors.New("--identity-trust-anchors-file must not be specified if --identity-external-issuer=true")
+		if idopts.keyPEMFile == "" {
+			return errors.New("a private key file must be specified if other credentials are provided")
 		}
-
-	} else {
-		if idopts.trustPEMFile != "" || idopts.crtPEMFile != "" || idopts.keyPEMFile != "" {
-			if idopts.trustPEMFile == "" {
-				return errors.New("a trust anchors file must be specified if other credentials are provided")
-			}
-			if idopts.crtPEMFile == "" {
-				return errors.New("a certificate file must be specified if other credentials are provided")
-			}
-			if idopts.keyPEMFile == "" {
-				return errors.New("a private key file must be specified if other credentials are provided")
-			}
-			if err := checkFiles([]string{idopts.trustPEMFile, idopts.crtPEMFile, idopts.keyPEMFile}); err != nil {
-				return err
-			}
+		if err := checkFiles([]string{idopts.trustPEMFile, idopts.crtPEMFile, idopts.keyPEMFile}); err != nil {
+			return err
 		}
 	}
 
@@ -973,9 +948,15 @@ func (idopts *installIdentityOptions) validateAndBuild() (*charts.Identity, erro
 		return nil, err
 	}
 
-	if idopts.identityExternalIssuer {
-		return idopts.readExternallyManaged()
-	} else if idopts.trustPEMFile != "" && idopts.crtPEMFile != "" && idopts.keyPEMFile != "" {
+	if identity, err := idopts.readExternallyManaged(); identity != nil {
+		return identity, nil
+	} else {
+		if !kerrors.IsNotFound(err) {
+			return nil, err
+		}
+	}
+
+	if idopts.trustPEMFile != "" && idopts.crtPEMFile != "" && idopts.keyPEMFile != "" {
 		return idopts.readValues()
 	} else {
 		return idopts.genValues()
@@ -996,7 +977,6 @@ func (idopts *installIdentityOptions) genValues() (*charts.Identity, error) {
 		TrustDomain:     idopts.trustDomain,
 		TrustAnchorsPEM: root.Cred.Crt.EncodeCertificatePEM(),
 		Issuer: &charts.Issuer{
-			External:            false,
 			ClockSkewAllowance:  idopts.clockSkewAllowance.String(),
 			IssuanceLifetime:    idopts.issuanceLifetime.String(),
 			CrtExpiry:           root.Cred.Crt.Certificate.NotAfter,
@@ -1026,24 +1006,22 @@ func loadExternalIssuerData() (*externalIssuerData, error) {
 		return nil, err
 	}
 
-	keyMissingError := "key %s containing the %s needs to exist in secret %s if --identity-external-issuer=true"
-
-	anchors, ok := secret.Data[consts.IdentityIssuerTrustAnchorsNameExternal]
-	if !ok {
-		return nil, fmt.Errorf(keyMissingError, consts.IdentityIssuerTrustAnchorsNameExternal, "trust anchors", consts.IdentityIssuerSecretName)
+	if secret.Type != corev1.SecretTypeTLS {
+		return nil, errors.New("")
 	}
 
-	crt, ok := secret.Data[consts.IdentityIssuerCrtNameExternal]
+	keyMissingError := "key %s containing the %s needs to exist in secret %s"
+	crt, ok := secret.Data[corev1.TLSCertKey]
 	if !ok {
 		return nil, fmt.Errorf(keyMissingError, consts.IdentityIssuerCrtNameExternal, "issuer certificate", consts.IdentityIssuerSecretName)
 	}
 
-	key, ok := secret.Data[consts.IdentityIssuerKeyNameExternal]
+	key, ok := secret.Data[corev1.TLSPrivateKeyKey]
 	if !ok {
 		return nil, fmt.Errorf(keyMissingError, consts.IdentityIssuerKeyNameExternal, "issuer key", consts.IdentityIssuerSecretName)
 	}
 
-	return &externalIssuerData{string(anchors), string(crt), string(key)}, nil
+	return &externalIssuerData{"", string(crt), string(key)}, nil
 }
 
 func (idopts *installIdentityOptions) verifyCred(creds *tls.Cred, trustAnchors string) error {
@@ -1059,7 +1037,6 @@ func (idopts *installIdentityOptions) verifyCred(creds *tls.Cred, trustAnchors s
 }
 
 func (idopts *installIdentityOptions) readExternallyManaged() (*charts.Identity, error) {
-
 	externalIssuerData, err := loadExternalIssuerData()
 	if err != nil {
 		return nil, err
diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go
index 29c4118c..7dfd8e90 100644
--- a/cli/cmd/upgrade.go
+++ b/cli/cmd/upgrade.go
@@ -14,6 +14,7 @@ import (
 	"github.com/linkerd/linkerd2/pkg/version"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -192,13 +193,6 @@ func upgradeRunE(options *upgradeOptions, stage string, flags *pflag.FlagSet) er
 	return nil
 }
 
-func isIssuerExternal(flags *pflag.FlagSet) bool {
-	if f := flags.Lookup("identity-external-issuer"); f != nil {
-		return f.Value.String() == "true"
-	}
-	return false
-}
-
 func (options *upgradeOptions) validateAndBuild(stage string, k kubernetes.Interface, flags *pflag.FlagSet) (*charts.Values, *pb.All, error) {
 	if err := options.validate(); err != nil {
 		return nil, nil, err
@@ -253,7 +247,7 @@ func (options *upgradeOptions) validateAndBuild(stage string, k kubernetes.Inter
 		}
 		configs.GetGlobal().IdentityContext = toIdentityContext(identity)
 	} else {
-		identity, err = fetchIdentityValues(k, idctx, isIssuerExternal(flags))
+		identity, err = fetchIdentityValues(k, idctx)
 		if err != nil {
 			return nil, nil, fmt.Errorf("unable to fetch the existing issuer credentials from Kubernetes: %s", err)
 		}
@@ -350,12 +344,12 @@ func fetchTLSSecret(k kubernetes.Interface, webhook string, options *upgradeOpti
 //
 // This bypasses the public API so that we can access secrets and validate
 // permissions.
-func fetchIdentityValues(k kubernetes.Interface, idctx *pb.IdentityContext, externalIssuer bool) (*charts.Identity, error) {
+func fetchIdentityValues(k kubernetes.Interface, idctx *pb.IdentityContext) (*charts.Identity, error) {
 	if idctx == nil {
 		return nil, nil
 	}
 
-	keyPEM, crtPEM, expiry, err := fetchIssuer(k, idctx.GetTrustAnchorsPem(), externalIssuer)
+	keyPEM, crtPEM, expiry, err := fetchIssuer(k, idctx.GetTrustAnchorsPem())
 	if err != nil {
 		return nil, err
 	}
@@ -364,7 +358,6 @@ func fetchIdentityValues(k kubernetes.Interface, idctx *pb.IdentityContext, exte
 		TrustDomain:     idctx.GetTrustDomain(),
 		TrustAnchorsPEM: idctx.GetTrustAnchorsPem(),
 		Issuer: &charts.Issuer{
-			External:            externalIssuer,
 			ClockSkewAllowance:  idctx.GetClockSkewAllowance().String(),
 			IssuanceLifetime:    idctx.GetIssuanceLifetime().String(),
 			CrtExpiry:           expiry,
@@ -377,10 +370,7 @@ func fetchIdentityValues(k kubernetes.Interface, idctx *pb.IdentityContext, exte
 	}, nil
 }
 
-func fetchIssuer(k kubernetes.Interface, trustPEM string, externalIssuer bool) (string, string, time.Time, error) {
-	crtName := k8s.IdentityIssuerCrtName
-	keyName := k8s.IdentityIssuerKeyName
-
+func fetchIssuer(k kubernetes.Interface, trustPEM string) (string, string, time.Time, error) {
 	roots, err := tls.DecodePEMCertPool(trustPEM)
 	if err != nil {
 		return "", "", time.Time{}, err
@@ -392,9 +382,12 @@ func fetchIssuer(k kubernetes.Interface, trustPEM string, externalIssuer bool) (
 	if err != nil {
 		return "", "", time.Time{}, err
 	}
-	if externalIssuer {
-		crtName = k8s.IdentityIssuerCrtNameExternal
-		keyName = k8s.IdentityIssuerKeyNameExternal
+
+	crtName := k8s.IdentityIssuerCrtName
+	keyName := k8s.IdentityIssuerKeyName
+	if secret.Type != corev1.SecretTypeTLS {
+		crtName = corev1.TLSCertKey
+		keyName = corev1.TLSPrivateKeyKey
 	}
 
 	keyPEM := string(secret.Data[keyName])
diff --git a/pkg/charts/values.go b/pkg/charts/values.go
index 25d94249..c662510e 100644
--- a/pkg/charts/values.go
+++ b/pkg/charts/values.go
@@ -156,7 +156,6 @@ type (
 
 	// Issuer has the Helm variables of the identity issuer
 	Issuer struct {
-		External            bool
 		ClockSkewAllowance  string
 		IssuanceLifetime    string
 		CrtExpiryAnnotation string