Last active
November 26, 2022 19:36
-
-
Save ihcsim/8580c88f94fffc0c345fc852e4bb38ae to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment