From cefb94aa8d3dfa450502c974ac29f3696228b7d5 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Thu, 3 Dec 2020 14:39:52 -0800 Subject: [PATCH 01/14] Working POC with the leader-elector container Uses a leader-elector sidecar to ensure only one injector replica generates the CA and cert+key. The other replicas pick up the cert+key from a k8s secret for use in their TLS listeners. The leader-elector sidecars coordinate using the annotations of a k8s Endpoint object, which is why those extra permissions were added to the role in the deployment yaml. Build a dev image: `make image VERSION=dev` Deploy: `kubectl apply -k deploy/ -n vault` --- deploy/injector-deployment.yaml | 15 ++++++++-- deploy/injector-extras.yaml | 13 +++++++++ deploy/injector-rbac.yaml | 33 ++++++++++++++++++++++ deploy/kustomization.yaml | 1 + helper/cert/notify.go | 1 + helper/cert/source_gen.go | 49 ++++++++++++++++++++++++++++++++- leader/leader.go | 40 +++++++++++++++++++++++++++ subcommand/injector/command.go | 22 +++++++++++++-- 8 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 deploy/injector-extras.yaml create mode 100644 leader/leader.go diff --git a/deploy/injector-deployment.yaml b/deploy/injector-deployment.yaml index c6e9fd65..979837c5 100644 --- a/deploy/injector-deployment.yaml +++ b/deploy/injector-deployment.yaml @@ -7,7 +7,7 @@ metadata: app.kubernetes.io/name: vault-injector app.kubernetes.io/instance: vault spec: - replicas: 1 + replicas: 2 selector: matchLabels: app.kubernetes.io/name: vault-injector @@ -20,8 +20,19 @@ spec: spec: serviceAccountName: "vault-injector" containers: + - name: leader-elector + image: gcr.io/google_containers/leader-elector:0.4 + args: + - --election=injector-leader + - --election-namespace=$(NAMESPACE) + - --http=0.0.0.0:4040 + env: + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace - name: sidecar-injector - image: "hashicorp/vault-k8s:0.6.0" + image: "hashicorp/vault-k8s:dev" imagePullPolicy: IfNotPresent env: - name: NAMESPACE diff --git a/deploy/injector-extras.yaml b/deploy/injector-extras.yaml new file mode 100644 index 00000000..5315caf5 --- /dev/null +++ b/deploy/injector-extras.yaml @@ -0,0 +1,13 @@ +# These are created here so they can be cleaned up easily. The endpoints +# especially, since if they're left around the leader won't expire for about a +# minute. +--- +apiVersion: v1 +kind: Endpoints +metadata: + name: injector-leader +--- +apiVersion: v1 +kind: Secret +metadata: + name: certs diff --git a/deploy/injector-rbac.yaml b/deploy/injector-rbac.yaml index 21499d09..31cf431c 100644 --- a/deploy/injector-rbac.yaml +++ b/deploy/injector-rbac.yaml @@ -38,3 +38,36 @@ subjects: - kind: ServiceAccount name: vault-injector namespace: vault +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: vault-injector-role + labels: + app.kubernetes.io/name: vault-injector + app.kubernetes.io/instance: vault +rules: +- apiGroups: [""] + resources: ["endpoints", "secrets"] + verbs: + - "create" + - "get" + - "watch" + - "list" + - "update" +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: vault-injector-rolebinding + labels: + app.kubernetes.io/name: vault-injector + app.kubernetes.io/instance: vault +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: vault-injector-role +subjects: +- kind: ServiceAccount + name: vault-injector + namespace: vault diff --git a/deploy/kustomization.yaml b/deploy/kustomization.yaml index 066a6d68..f8c8ba19 100644 --- a/deploy/kustomization.yaml +++ b/deploy/kustomization.yaml @@ -5,3 +5,4 @@ resources: - injector-mutating-webhook.yaml - injector-rbac.yaml - injector-service.yaml +- injector-extras.yaml diff --git a/helper/cert/notify.go b/helper/cert/notify.go index 214eb90c..45beafc1 100644 --- a/helper/cert/notify.go +++ b/helper/cert/notify.go @@ -59,6 +59,7 @@ func (n *Notify) Run() { if last.Equal(&next) { continue } + log.Println("[DEBUG] notify.run: sending new cert") last = &next // Send the certificate out, but in case it hangs, because // the certificates aren't being pulled off the channel quickly diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index e5ffce81..40e32861 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -17,6 +17,11 @@ import ( "strings" "sync" "time" + + "github.com/hashicorp/vault-k8s/leader" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" ) // GenSource generates a self-signed CA and certificate pair. @@ -43,6 +48,9 @@ type GenSource struct { caCert []byte caCertTemplate *x509.Certificate caSigner crypto.Signer + + K8sClient *kubernetes.Clientset + // TODO(tvoran): add secret informer here } // Certificate implements source @@ -51,6 +59,17 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro defer s.mu.Unlock() var result Bundle + // For followers, run different function here that reads bundle from Secret, + // and returns that in the result. That will flow through the existing + // notify channel structure, testing if it's the same cert as last, etc. + leaderCheck, err := leader.IsLeader() + if err != nil { + return result, err + } + if !leaderCheck { + return s.getBundleFromSecret() + } + // If we have no CA, generate it for the first time. if len(s.caCert) == 0 { if err := s.generateCA(); err != nil { @@ -92,17 +111,45 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro if err == nil { result.Cert = []byte(cert) result.Key = []byte(key) + + _, err = s.K8sClient.CoreV1().Secrets("vault").Update(&v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "certs", + }, + Data: map[string][]byte{ + "ca": result.CACert, + "cert": result.Cert, + "key": result.Key, + }, + }) + if err != nil { + return result, fmt.Errorf("failed to create secret: %s", err) + } } return result, err } +func (s *GenSource) getBundleFromSecret() (Bundle, error) { + var bundle Bundle + // will this work without knowing the namespace? ...nope. + secret, err := s.K8sClient.CoreV1().Secrets("vault").Get("certs", metav1.GetOptions{}) + if err != nil { + return bundle, fmt.Errorf("failed to get secret: %s", err) + } + bundle.CACert = secret.Data["ca"] + bundle.Cert = secret.Data["cert"] + bundle.Key = secret.Data["key"] + + return bundle, nil +} + func (s *GenSource) expiry() time.Duration { if s.Expiry > 0 { return s.Expiry } - return 24 * time.Hour + return 1 * time.Minute } func (s *GenSource) expiryWithin() time.Duration { diff --git a/leader/leader.go b/leader/leader.go new file mode 100644 index 00000000..fc8d8fc8 --- /dev/null +++ b/leader/leader.go @@ -0,0 +1,40 @@ +package leader + +import ( + "encoding/json" + "io/ioutil" + "net/http" + "os" +) + +type leaderResponse struct { + Name string `json:"name"` +} + +// IsLeader returns whether this host is the leader +func IsLeader() (bool, error) { + resp, err := http.Get("http://localhost:4040/") + if err != nil { + return false, err + } + defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return false, err + } + leaderResp := &leaderResponse{} + err = json.Unmarshal(body, leaderResp) + if err != nil { + return false, err + } + hostname, err := os.Hostname() + if err != nil { + return false, err + } + if leaderResp.Name == hostname { + // log.Printf("[DEBUG] I'm the leader! %s", hostname) + return true, nil + } + // log.Printf("[DEBUG] I'm not the leader: %s, %s", leaderResp.Name, hostname) + return false, nil +} diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index ee228fce..74ece743 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/go-hclog" agentInject "github.com/hashicorp/vault-k8s/agent-inject" "github.com/hashicorp/vault-k8s/helper/cert" + "github.com/hashicorp/vault-k8s/leader" "github.com/mitchellh/cli" "github.com/prometheus/client_golang/prometheus/promhttp" "k8s.io/apimachinery/pkg/types" @@ -87,8 +88,9 @@ func (c *Command) Run(args []string) int { // Determine where to source the certificates from var certSource cert.Source = &cert.GenSource{ - Name: "Agent Inject", - Hosts: strings.Split(c.flagAutoHosts, ","), + Name: "Agent Inject", + Hosts: strings.Split(c.flagAutoHosts, ","), + K8sClient: clientset, } if c.flagCertFile != "" { certSource = &cert.DiskSource{ @@ -193,10 +195,14 @@ func (c *Command) getCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, clientset *kubernetes.Clientset) { var bundle cert.Bundle + var updateReceived bool for { select { case bundle = <-ch: c.UI.Output("Updated certificate bundle received. Updating certs...") + // this keeps certWatcher from updating the k8s API every second, + // even when there's been no bundle update on the channel + updateReceived = true // Bundle is updated, set it up case <-time.After(1 * time.Second): @@ -216,8 +222,15 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client continue } + // Only the leader should do the caBundle patching in k8s API + isLeader, err := leader.IsLeader() + if err != nil { + c.UI.Error(fmt.Sprintf("error checking leader: %s", err)) + continue + } + // If there is a MWC name set, then update the CA bundle. - if c.flagAutoName != "" && len(bundle.CACert) > 0 { + if isLeader && updateReceived && c.flagAutoName != "" && len(bundle.CACert) > 0 { // The CA Bundle value must be base64 encoded value := base64.StdEncoding.EncodeToString(bundle.CACert) @@ -235,6 +248,9 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client err)) continue } + c.UI.Output("[DEBUG] sent new caBundle to mutating webhook config") + + updateReceived = false } // Update the certificate From 636d1a8c4e16e0ca20d74fb0aa0896e81200ddbc Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Tue, 8 Dec 2020 16:06:30 -0800 Subject: [PATCH 02/14] Set certs Secret namespace from env --- helper/cert/source_gen.go | 12 ++++++------ subcommand/injector/command.go | 10 ++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index 40e32861..ebb56e9e 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -50,6 +50,7 @@ type GenSource struct { caSigner crypto.Signer K8sClient *kubernetes.Clientset + Namespace string // TODO(tvoran): add secret informer here } @@ -59,13 +60,13 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro defer s.mu.Unlock() var result Bundle - // For followers, run different function here that reads bundle from Secret, - // and returns that in the result. That will flow through the existing - // notify channel structure, testing if it's the same cert as last, etc. leaderCheck, err := leader.IsLeader() if err != nil { return result, err } + // For followers, run different function here that reads bundle from Secret, + // and returns that in the result. That will flow through the existing + // notify channel structure, testing if it's the same cert as last, etc. if !leaderCheck { return s.getBundleFromSecret() } @@ -112,7 +113,7 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro result.Cert = []byte(cert) result.Key = []byte(key) - _, err = s.K8sClient.CoreV1().Secrets("vault").Update(&v1.Secret{ + _, err = s.K8sClient.CoreV1().Secrets(s.Namespace).Update(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "certs", }, @@ -132,8 +133,7 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro func (s *GenSource) getBundleFromSecret() (Bundle, error) { var bundle Bundle - // will this work without knowing the namespace? ...nope. - secret, err := s.K8sClient.CoreV1().Secrets("vault").Get("certs", metav1.GetOptions{}) + secret, err := s.K8sClient.CoreV1().Secrets(s.Namespace).Get("certs", metav1.GetOptions{}) if err != nil { return bundle, fmt.Errorf("failed to get secret: %s", err) } diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index 74ece743..e679c1bf 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -91,6 +91,7 @@ func (c *Command) Run(args []string) int { Name: "Agent Inject", Hosts: strings.Split(c.flagAutoHosts, ","), K8sClient: clientset, + Namespace: getNamespace(), } if c.flagCertFile != "" { certSource = &cert.DiskSource{ @@ -177,6 +178,15 @@ func (c *Command) Run(args []string) int { return 0 } +func getNamespace() string { + namespace := os.Getenv("NAMESPACE") + if len(namespace) > 0 { + return namespace + } + + return "default" +} + func (c *Command) handleReady(rw http.ResponseWriter, req *http.Request) { // Always ready at this point. The main readiness check is whether // there is a TLS certificate. If we reached this point it means we From d6ffcec0c9ed6bcb2d42ddad9486ce347799b6a2 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Sun, 13 Dec 2020 14:16:19 -0900 Subject: [PATCH 03/14] flag to control leader elector usage and informer Added command-line and env option to control leader-elector usage. Using Secrets informer for followers to ensure retrieving timely cert updates without overloading the k8s api. --- agent-inject/agent/agent.go | 5 ++- helper/cert/source_gen.go | 77 ++++++++++++++++++++++------------ subcommand/injector/command.go | 26 ++++++++++-- subcommand/injector/flags.go | 12 ++++++ 4 files changed, 88 insertions(+), 32 deletions(-) diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go index 8e7297da..9edc7c7c 100644 --- a/agent-inject/agent/agent.go +++ b/agent-inject/agent/agent.go @@ -26,6 +26,7 @@ const ( DefaultAgentCacheEnable = "false" DefaultAgentCacheUseAutoAuthToken = "true" DefaultAgentCacheListenerPort = "8200" + DefaultAgentUseLeaderElector = false ) // Agent is the top level structure holding all the @@ -123,8 +124,8 @@ type Agent struct { // SetSecurityContext controls whether the injected containers have a // SecurityContext set. SetSecurityContext bool - - // ExtraSecret is the Kubernetes secret to mount as a volume in the Vault agent container + + // ExtraSecret is the Kubernetes secret to mount as a volume in the Vault agent container // which can be referenced by the Agent config for secrets. Mounted at /vault/custom/ ExtraSecret string } diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index ebb56e9e..cf78176d 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -20,10 +20,16 @@ import ( "github.com/hashicorp/vault-k8s/leader" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + informerv1 "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" ) +// Name of the k8s Secret used to share the caBundle between leader and +// followers +const certSecretName = "certs" + // GenSource generates a self-signed CA and certificate pair. // // This generator is stateful. On the first run (last == nil to Certificate), @@ -49,9 +55,10 @@ type GenSource struct { caCertTemplate *x509.Certificate caSigner crypto.Signer - K8sClient *kubernetes.Clientset - Namespace string - // TODO(tvoran): add secret informer here + K8sClient *kubernetes.Clientset + Namespace string + SecretsCache informerv1.SecretInformer + LeaderElectorEnabled bool } // Certificate implements source @@ -60,15 +67,17 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro defer s.mu.Unlock() var result Bundle - leaderCheck, err := leader.IsLeader() - if err != nil { - return result, err - } - // For followers, run different function here that reads bundle from Secret, - // and returns that in the result. That will flow through the existing - // notify channel structure, testing if it's the same cert as last, etc. - if !leaderCheck { - return s.getBundleFromSecret() + if s.LeaderElectorEnabled { + leaderCheck, err := leader.IsLeader() + if err != nil { + return result, err + } + // For followers, run different function here that reads bundle from Secret, + // and returns that in the result. That will flow through the existing + // notify channel structure, testing if it's the same cert as last, etc. + if !leaderCheck { + return s.getBundleFromSecret() + } } // If we have no CA, generate it for the first time. @@ -113,27 +122,43 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro result.Cert = []byte(cert) result.Key = []byte(key) - _, err = s.K8sClient.CoreV1().Secrets(s.Namespace).Update(&v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "certs", - }, - Data: map[string][]byte{ - "ca": result.CACert, - "cert": result.Cert, - "key": result.Key, - }, - }) - if err != nil { - return result, fmt.Errorf("failed to create secret: %s", err) + if s.LeaderElectorEnabled { + if err := s.updateSecret(result); err != nil { + return result, fmt.Errorf("failed to update Secret: %s", err) + } } } return result, err } +func (s *GenSource) updateSecret(bundle Bundle) error { + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: certSecretName, + }, + Data: map[string][]byte{ + "ca": bundle.CACert, + "cert": bundle.Cert, + "key": bundle.Key, + }, + } + // Attempt updating the Secret first, and if it doesn't exist, fallback to + // create + _, err := s.K8sClient.CoreV1().Secrets(s.Namespace).Update(secret) + if errors.IsNotFound(err) { + _, err = s.K8sClient.CoreV1().Secrets(s.Namespace).Create(secret) + } + if err != nil { + return err + } + return nil +} + func (s *GenSource) getBundleFromSecret() (Bundle, error) { var bundle Bundle - secret, err := s.K8sClient.CoreV1().Secrets(s.Namespace).Get("certs", metav1.GetOptions{}) + + secret, err := s.SecretsCache.Lister().Secrets(s.Namespace).Get(certSecretName) if err != nil { return bundle, fmt.Errorf("failed to get secret: %s", err) } @@ -149,7 +174,7 @@ func (s *GenSource) expiry() time.Duration { return s.Expiry } - return 1 * time.Minute + return 24 * time.Hour } func (s *GenSource) expiryWithin() time.Duration { diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index e679c1bf..1a0f3001 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -22,8 +22,11 @@ import ( "github.com/mitchellh/cli" "github.com/prometheus/client_golang/prometheus/promhttp" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/informers" + informerv1 "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/cache" ) type Command struct { @@ -45,6 +48,7 @@ type Command struct { flagRunAsSameUser bool // Run Vault agent as the User (uid) of the first application container flagSetSecurityContext bool // Set SecurityContext in injected containers flagTelemetryPath string // Path under which to expose metrics + flagUseLeaderElector bool // Use leader elector code flagSet *flag.FlagSet @@ -86,12 +90,26 @@ func (c *Command) Run(args []string) int { return 1 } + namespace := getNamespace() + var secrets informerv1.SecretInformer + if c.flagUseLeaderElector { + c.UI.Info("using leader elector logic") + factory := informers.NewSharedInformerFactoryWithOptions(clientset, 0, informers.WithNamespace(namespace)) + secrets = factory.Core().V1().Secrets() + go secrets.Informer().Run(ctx.Done()) + if !cache.WaitForCacheSync(ctx.Done(), secrets.Informer().HasSynced) { + c.UI.Error("timeout syncing Secrets informer") + return 1 + } + } // Determine where to source the certificates from var certSource cert.Source = &cert.GenSource{ - Name: "Agent Inject", - Hosts: strings.Split(c.flagAutoHosts, ","), - K8sClient: clientset, - Namespace: getNamespace(), + Name: "Agent Inject", + Hosts: strings.Split(c.flagAutoHosts, ","), + K8sClient: clientset, + Namespace: namespace, + SecretsCache: secrets, + LeaderElectorEnabled: c.flagUseLeaderElector, } if c.flagCertFile != "" { certSource = &cert.DiskSource{ diff --git a/subcommand/injector/flags.go b/subcommand/injector/flags.go index a975fd0c..6dbdfd04 100644 --- a/subcommand/injector/flags.go +++ b/subcommand/injector/flags.go @@ -68,6 +68,9 @@ type Specification struct { // TelemetryPath is the AGENT_INJECT_TELEMETRY_PATH environment variable. TelemetryPath string `split_words:"true"` + + // UseLeaderElector is the AGENT_INJECT_USE_LEADER_ELECTOR + UseLeaderElector string `split_words:"true"` } func (c *Command) init() { @@ -105,6 +108,8 @@ func (c *Command) init() { fmt.Sprintf("Set SecurityContext in injected containers. Defaults to %v.", agent.DefaultAgentSetSecurityContext)) c.flagSet.StringVar(&c.flagTelemetryPath, "telemetry-path", "", "Path under which to expose metrics") + c.flagSet.BoolVar(&c.flagUseLeaderElector, "use-leader-elector", agent.DefaultAgentUseLeaderElector, + fmt.Sprintf("Use leader elector to coordinate multiple replicas when updating CA and Certs with auto-tls")) c.help = flags.Usage(help, c.flagSet) } @@ -211,5 +216,12 @@ func (c *Command) parseEnvs() error { c.flagTelemetryPath = envs.TelemetryPath } + if envs.UseLeaderElector != "" { + c.flagUseLeaderElector, err = strconv.ParseBool(envs.UseLeaderElector) + if err != nil { + return err + } + } + return nil } From e28d446b9d3d840d50e4569c3160567c0ea476da Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Sun, 13 Dec 2020 14:37:29 -0900 Subject: [PATCH 04/14] Updating deployment yaml Added ttl and health checks to leader-elector. Added AGENT_INJECT_USE_LEADER_ELECTOR env option to deployment. --- deploy/injector-deployment.yaml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/deploy/injector-deployment.yaml b/deploy/injector-deployment.yaml index 979837c5..f5c9532a 100644 --- a/deploy/injector-deployment.yaml +++ b/deploy/injector-deployment.yaml @@ -26,11 +26,32 @@ spec: - --election=injector-leader - --election-namespace=$(NAMESPACE) - --http=0.0.0.0:4040 + - --ttl=60s env: - name: NAMESPACE valueFrom: fieldRef: fieldPath: metadata.namespace + livenessProbe: + httpGet: + path: / + port: 4040 + scheme: HTTP + failureThreshold: 2 + initialDelaySeconds: 1 + periodSeconds: 2 + successThreshold: 1 + timeoutSeconds: 5 + readinessProbe: + httpGet: + path: / + port: 4040 + scheme: HTTP + failureThreshold: 2 + initialDelaySeconds: 2 + periodSeconds: 2 + successThreshold: 1 + timeoutSeconds: 5 - name: sidecar-injector image: "hashicorp/vault-k8s:dev" imagePullPolicy: IfNotPresent @@ -53,6 +74,8 @@ spec: value: vault-agent-injector-cfg - name: AGENT_INJECT_TLS_AUTO_HOSTS value: "vault-agent-injector-svc,vault-agent-injector-svc.$(NAMESPACE),vault-agent-injector-svc.$(NAMESPACE).svc" + - name: AGENT_INJECT_USE_LEADER_ELECTOR + value: "true" args: - agent-inject - 2>&1 From 4d951ed6c0095b84bdbece1f018382b5979bb1bd Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Sun, 13 Dec 2020 18:09:12 -0900 Subject: [PATCH 05/14] updating cert name and flag test --- deploy/{injector-extras.yaml => injector-leader-extras.yaml} | 4 ++-- helper/cert/source_gen.go | 4 ++-- subcommand/injector/flags_test.go | 2 ++ 3 files changed, 6 insertions(+), 4 deletions(-) rename deploy/{injector-extras.yaml => injector-leader-extras.yaml} (79%) diff --git a/deploy/injector-extras.yaml b/deploy/injector-leader-extras.yaml similarity index 79% rename from deploy/injector-extras.yaml rename to deploy/injector-leader-extras.yaml index 5315caf5..af286beb 100644 --- a/deploy/injector-extras.yaml +++ b/deploy/injector-leader-extras.yaml @@ -5,9 +5,9 @@ apiVersion: v1 kind: Endpoints metadata: - name: injector-leader + name: vault-agent-injector-leader --- apiVersion: v1 kind: Secret metadata: - name: certs + name: vault-injector-leader diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index cf78176d..089e828c 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -28,7 +28,7 @@ import ( // Name of the k8s Secret used to share the caBundle between leader and // followers -const certSecretName = "certs" +const certSecretName = "vault-injector-certs" // GenSource generates a self-signed CA and certificate pair. // @@ -55,7 +55,7 @@ type GenSource struct { caCertTemplate *x509.Certificate caSigner crypto.Signer - K8sClient *kubernetes.Clientset + K8sClient kubernetes.Interface Namespace string SecretsCache informerv1.SecretInformer LeaderElectorEnabled bool diff --git a/subcommand/injector/flags_test.go b/subcommand/injector/flags_test.go index 5be9e0a3..f281f650 100644 --- a/subcommand/injector/flags_test.go +++ b/subcommand/injector/flags_test.go @@ -157,6 +157,8 @@ func TestCommandEnvBools(t *testing.T) { {env: "AGENT_INJECT_RUN_AS_SAME_USER", value: false, cmdPtr: &cmd.flagRunAsSameUser}, {env: "AGENT_INJECT_SET_SECURITY_CONTEXT", value: true, cmdPtr: &cmd.flagSetSecurityContext}, {env: "AGENT_INJECT_SET_SECURITY_CONTEXT", value: false, cmdPtr: &cmd.flagSetSecurityContext}, + {env: "AGENT_INJECT_USE_LEADER_ELECTOR", value: true, cmdPtr: &cmd.flagUseLeaderElector}, + {env: "AGENT_INJECT_USE_LEADER_ELECTOR", value: false, cmdPtr: &cmd.flagUseLeaderElector}, } for _, tt := range tests { From db55f5793c02a133d84d6ba04e6dd219f2be49bb Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Mon, 14 Dec 2020 22:20:43 -0900 Subject: [PATCH 06/14] leader/follower unit tests for source.Certificate() Slightly refactored the leader package so it's more testable --- go.mod | 3 + go.sum | 12 ++++ helper/cert/source_gen.go | 14 ++-- helper/cert/source_gen_test.go | 114 +++++++++++++++++++++++++++++++++ leader/leader.go | 31 +++++++-- subcommand/injector/command.go | 29 +++++---- 6 files changed, 179 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index a073f194..e96d6036 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,8 @@ go 1.12 require ( github.com/armon/go-radix v1.0.0 // indirect + github.com/evanphx/json-patch v4.9.0+incompatible // indirect + github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect github.com/google/btree v1.0.0 // indirect github.com/hashicorp/consul v1.5.0 github.com/hashicorp/go-hclog v0.9.2 @@ -31,6 +33,7 @@ require ( k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible k8s.io/klog v1.0.0 // indirect + k8s.io/kube-openapi v0.0.0-20190228160746-b3a7cee44a30 // indirect k8s.io/utils v0.0.0-20191030222137-2b95a09bc58d // indirect sigs.k8s.io/yaml v1.1.0 // indirect ) diff --git a/go.sum b/go.sum index 3097d400..516dd45f 100644 --- a/go.sum +++ b/go.sum @@ -65,10 +65,13 @@ github.com/duosecurity/duo_api_golang v0.0.0-20190308151101-6c680f768e74 h1:2MIh github.com/duosecurity/duo_api_golang v0.0.0-20190308151101-6c680f768e74/go.mod h1:UqXY1lYT/ERa4OEAywUqdok1T4RCRdArkhic1Opuavo= github.com/elazarl/go-bindata-assetfs v0.0.0-20160803192304-e1a2a7ec64b0/go.mod h1:v+YaWX3bdea5J/mo8dSETolEo7R71Vk1u8bnjau5yw4= github.com/envoyproxy/go-control-plane v0.0.0-20180919002855-2137d9196328/go.mod h1:SBwIajubJHhxtWwsL9s8ss4safvEdbitLhGGK48rN6g= +github.com/evanphx/json-patch v4.9.0+incompatible h1:kLcOMZeuLAJvL2BPWLMIj5oaZQobrkAqrL+WFZwQses= +github.com/evanphx/json-patch v4.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/structs v0.0.0-20180123065059-ebf56d35bba7/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= +github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= @@ -93,6 +96,8 @@ github.com/gogo/protobuf v1.1.1 h1:72R+M5VuhED/KujmZVcIquuo8mBgX4oVda//DQb3PXo= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= +github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e h1:1r7pUrabqp18hOBcwBwiTsbnFeTZHV9eER/QT5JVZxY= +github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:tluoj9z5200jBnyusfRPU2LqT6J+DAorxEvtC7LHB+E= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM= @@ -222,6 +227,7 @@ github.com/hashicorp/vic v1.5.1-0.20190403131502-bbfe86ec9443/go.mod h1:bEpDU35n github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= +github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/imdario/mergo v0.3.6 h1:xTNEAn+kxVO7dTZGu0CegyqKZmoWFI0rF8UxjlB2d28= github.com/imdario/mergo v0.3.6/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA= @@ -297,8 +303,10 @@ github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRW github.com/nicolai86/scaleway-sdk v1.10.2-0.20180628010248-798f60e20bb2/go.mod h1:TLb2Sg7HQcgGdloNxkrmtgDNR9uVYF3lfdFIN4Ro6Sk= github.com/oklog/run v0.0.0-20180308005104-6934b124db28/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA= github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA= +github.com/onsi/ginkgo v1.6.0 h1:Ix8l273rp3QzYgXSR+c8d1fTG7UPgYkOSELPhiY/YGw= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/gomega v1.4.1/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= +github.com/onsi/gomega v1.4.2 h1:3mYCb7aPxS/RU7TI1y4rkEn1oKmPRjNJLNEXgw7MH2I= github.com/onsi/gomega v1.4.2/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= @@ -479,6 +487,7 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKWaSkCsqBpgog8nAV2xsGOxlo= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= @@ -487,6 +496,7 @@ gopkg.in/mgo.v2 v2.0.0-20160818020120-3f83fa500528/go.mod h1:yeKp02qBN3iKW1OzL3M gopkg.in/ory-am/dockertest.v3 v3.3.4/go.mod h1:s9mmoLkaGeAh97qygnNj4xWkiN7e1SKekYC6CovU+ek= gopkg.in/square/go-jose.v2 v2.3.1 h1:SK5KegNXmKmqE342YYN2qPHEnUYeoMiXXl1poUlI+o4= gopkg.in/square/go-jose.v2 v2.3.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= +gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= @@ -515,6 +525,8 @@ k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible/go.mod h1:7v k8s.io/klog v0.3.0/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8= k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I= +k8s.io/kube-openapi v0.0.0-20190228160746-b3a7cee44a30 h1:TRb4wNWoBVrH9plmkp2q86FIDppkbrEXdXlxU3a3BMI= +k8s.io/kube-openapi v0.0.0-20190228160746-b3a7cee44a30/go.mod h1:BXM9ceUBTj2QnfH2MK1odQs778ajze1RxcmP6S8RVVc= k8s.io/utils v0.0.0-20191030222137-2b95a09bc58d h1:1P0iBJsBzxRmR+dIFnM+Iu4aLxnoa7lBqozW/0uHbT8= k8s.io/utils v0.0.0-20191030222137-2b95a09bc58d/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs= diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index 089e828c..4a3ea66d 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -55,10 +55,10 @@ type GenSource struct { caCertTemplate *x509.Certificate caSigner crypto.Signer - K8sClient kubernetes.Interface - Namespace string - SecretsCache informerv1.SecretInformer - LeaderElectorEnabled bool + K8sClient kubernetes.Interface + Namespace string + SecretsCache informerv1.SecretInformer + LeaderElector *leader.LeaderElector } // Certificate implements source @@ -67,8 +67,8 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro defer s.mu.Unlock() var result Bundle - if s.LeaderElectorEnabled { - leaderCheck, err := leader.IsLeader() + if s.LeaderElector != nil { + leaderCheck, err := s.LeaderElector.IsLeader() if err != nil { return result, err } @@ -122,7 +122,7 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro result.Cert = []byte(cert) result.Key = []byte(key) - if s.LeaderElectorEnabled { + if s.LeaderElector != nil { if err := s.updateSecret(result); err != nil { return result, fmt.Errorf("failed to update Secret: %s", err) } diff --git a/helper/cert/source_gen_test.go b/helper/cert/source_gen_test.go index 6b7303bb..15837189 100644 --- a/helper/cert/source_gen_test.go +++ b/helper/cert/source_gen_test.go @@ -2,14 +2,22 @@ package cert import ( "context" + "encoding/json" "io/ioutil" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" "testing" "time" + "github.com/hashicorp/vault-k8s/leader" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" ) // hasOpenSSL is used to determine if the openssl CLI exists for unit tests. @@ -116,3 +124,109 @@ func testBundleVerify(t *testing.T, bundle *Bundle) { t.Log(string(output)) require.NoError(err) } + +func TestGenSource_leader(t *testing.T) { + + if !hasOpenSSL { + t.Skip("openssl not found") + return + } + + // Generate the bundle + source := testGenSource() + + // Setup test leader service returning this host as the leader + ts := testLeaderServer(t, testGetHostname(t)) + defer ts.Close() + source.LeaderElector = leader.NewWithURL(ts.URL) + + source.Namespace = "default" + source.K8sClient = fake.NewSimpleClientset() + bundle, err := source.Certificate(context.Background(), nil) + require.NoError(t, err) + testBundleVerify(t, &bundle) + + // check that the Secret has been created + checkSecret, err := source.K8sClient.CoreV1().Secrets(source.Namespace).Get(certSecretName, metav1.GetOptions{}) + require.NoError(t, err) + secretBundle := Bundle{ + CACert: checkSecret.Data["ca"], + Cert: checkSecret.Data["cert"], + Key: checkSecret.Data["key"], + } + require.True(t, bundle.Equal(&secretBundle)) +} + +func TestGenSource_follower(t *testing.T) { + + if !hasOpenSSL { + t.Skip("openssl not found") + return + } + + // Generate the bundle + source := testGenSource() + + // Setup a leader elector service that returns a different hostname, so it + // thinks we're the follower + ts := testLeaderServer(t, testGetHostname(t)+"not it") + defer ts.Close() + source.LeaderElector = leader.NewWithURL(ts.URL) + + // Setup the k8s client with a Secret for a follower to pick up + source.Namespace = "default" + secretBundle := testBundle(t) + source.K8sClient = fake.NewSimpleClientset(&v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: certSecretName, + Namespace: source.Namespace, + }, + Data: map[string][]byte{ + "ca": secretBundle.CACert, + "cert": secretBundle.Cert, + "key": secretBundle.Key, + }, + }) + + // setup a Secret informer cache with the fake clientset for followers to use + factory := informers.NewSharedInformerFactoryWithOptions(source.K8sClient, 0, informers.WithNamespace(source.Namespace)) + secrets := factory.Core().V1().Secrets() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go secrets.Informer().Run(ctx.Done()) + source.SecretsCache = secrets + + bundle, err := source.Certificate(ctx, nil) + require.NoError(t, err) + testBundleVerify(t, &bundle) + require.True(t, bundle.Equal(secretBundle), + "bundle returned from source.Certificate() should have matched what the Secret was created with", + ) +} + +func testLeaderServer(t *testing.T, hostname string) *httptest.Server { + t.Helper() + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lResp := leader.LeaderResponse{ + Name: hostname, + } + body, err := json.Marshal(lResp) + if err != nil { + t.Fatalf("failed to marshal leader response: %s", err) + } + w.WriteHeader(200) + w.Write(body) + })) + return ts +} + +func testGetHostname(t *testing.T) string { + t.Helper() + + host, err := os.Hostname() + if err != nil { + t.Fatalf("failed to get hostname for test leader service: %s", err) + } + return host +} diff --git a/leader/leader.go b/leader/leader.go index fc8d8fc8..c0471da5 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -7,13 +7,33 @@ import ( "os" ) -type leaderResponse struct { +const defaultURL = "http://localhost:4040/" + +type LeaderElector struct { + URL string +} + +type LeaderResponse struct { Name string `json:"name"` } +// New returns a LeaderElector with the default service endpoint +func New() *LeaderElector { + return &LeaderElector{ + URL: defaultURL, + } +} + +// NewWithURL returns a LeaderElector with a custom service endpoint URL +func NewWithURL(URL string) *LeaderElector { + return &LeaderElector{ + URL: URL, + } +} + // IsLeader returns whether this host is the leader -func IsLeader() (bool, error) { - resp, err := http.Get("http://localhost:4040/") +func (le *LeaderElector) IsLeader() (bool, error) { + resp, err := http.Get(le.URL) if err != nil { return false, err } @@ -22,7 +42,7 @@ func IsLeader() (bool, error) { if err != nil { return false, err } - leaderResp := &leaderResponse{} + leaderResp := &LeaderResponse{} err = json.Unmarshal(body, leaderResp) if err != nil { return false, err @@ -32,9 +52,8 @@ func IsLeader() (bool, error) { return false, err } if leaderResp.Name == hostname { - // log.Printf("[DEBUG] I'm the leader! %s", hostname) return true, nil } - // log.Printf("[DEBUG] I'm not the leader: %s, %s", leaderResp.Name, hostname) + return false, nil } diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index 1a0f3001..ae243ca4 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -92,6 +92,7 @@ func (c *Command) Run(args []string) int { namespace := getNamespace() var secrets informerv1.SecretInformer + var leaderElector *leader.LeaderElector if c.flagUseLeaderElector { c.UI.Info("using leader elector logic") factory := informers.NewSharedInformerFactoryWithOptions(clientset, 0, informers.WithNamespace(namespace)) @@ -101,15 +102,16 @@ func (c *Command) Run(args []string) int { c.UI.Error("timeout syncing Secrets informer") return 1 } + leaderElector = leader.New() } // Determine where to source the certificates from var certSource cert.Source = &cert.GenSource{ - Name: "Agent Inject", - Hosts: strings.Split(c.flagAutoHosts, ","), - K8sClient: clientset, - Namespace: namespace, - SecretsCache: secrets, - LeaderElectorEnabled: c.flagUseLeaderElector, + Name: "Agent Inject", + Hosts: strings.Split(c.flagAutoHosts, ","), + K8sClient: clientset, + Namespace: namespace, + SecretsCache: secrets, + LeaderElector: leaderElector, } if c.flagCertFile != "" { certSource = &cert.DiskSource{ @@ -250,11 +252,16 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client continue } - // Only the leader should do the caBundle patching in k8s API - isLeader, err := leader.IsLeader() - if err != nil { - c.UI.Error(fmt.Sprintf("error checking leader: %s", err)) - continue + isLeader := true + if c.flagUseLeaderElector { + // Only the leader should do the caBundle patching in k8s API + var err error + le := leader.New() + isLeader, err = le.IsLeader() + if err != nil { + c.UI.Error(fmt.Sprintf("error checking leader: %s", err)) + continue + } } // If there is a MWC name set, then update the CA bundle. From 05dbd9d120c4c6555d41c7a50346745414390142 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Mon, 14 Dec 2020 22:43:00 -0900 Subject: [PATCH 07/14] cleanup --- deploy/injector-deployment.yaml | 2 +- deploy/kustomization.yaml | 2 +- helper/cert/notify.go | 1 - subcommand/injector/command.go | 5 +++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/deploy/injector-deployment.yaml b/deploy/injector-deployment.yaml index f5c9532a..a7345da6 100644 --- a/deploy/injector-deployment.yaml +++ b/deploy/injector-deployment.yaml @@ -53,7 +53,7 @@ spec: successThreshold: 1 timeoutSeconds: 5 - name: sidecar-injector - image: "hashicorp/vault-k8s:dev" + image: "hashicorp/vault-k8s:0.6.0" imagePullPolicy: IfNotPresent env: - name: NAMESPACE diff --git a/deploy/kustomization.yaml b/deploy/kustomization.yaml index f8c8ba19..e5a11aae 100644 --- a/deploy/kustomization.yaml +++ b/deploy/kustomization.yaml @@ -5,4 +5,4 @@ resources: - injector-mutating-webhook.yaml - injector-rbac.yaml - injector-service.yaml -- injector-extras.yaml +- injector-leader-extras.yaml diff --git a/helper/cert/notify.go b/helper/cert/notify.go index 45beafc1..214eb90c 100644 --- a/helper/cert/notify.go +++ b/helper/cert/notify.go @@ -59,7 +59,6 @@ func (n *Notify) Run() { if last.Equal(&next) { continue } - log.Println("[DEBUG] notify.run: sending new cert") last = &next // Send the certificate out, but in case it hangs, because // the certificates aren't being pulled off the channel quickly diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index ae243ca4..3337d129 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -94,7 +94,7 @@ func (c *Command) Run(args []string) int { var secrets informerv1.SecretInformer var leaderElector *leader.LeaderElector if c.flagUseLeaderElector { - c.UI.Info("using leader elector logic") + c.UI.Info("Using leader elector logic") factory := informers.NewSharedInformerFactoryWithOptions(clientset, 0, informers.WithNamespace(namespace)) secrets = factory.Core().V1().Secrets() go secrets.Informer().Run(ctx.Done()) @@ -104,6 +104,7 @@ func (c *Command) Run(args []string) int { } leaderElector = leader.New() } + // Determine where to source the certificates from var certSource cert.Source = &cert.GenSource{ Name: "Agent Inject", @@ -283,7 +284,7 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client err)) continue } - c.UI.Output("[DEBUG] sent new caBundle to mutating webhook config") + c.UI.Output("Sent new caBundle to mutating webhook config") updateReceived = false } From 449aaaad0565cc835d4a45d976757b5bbe43dd71 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Tue, 15 Dec 2020 11:49:54 -0800 Subject: [PATCH 08/14] Update subcommand/injector/flags.go Co-authored-by: Tom Proctor --- subcommand/injector/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subcommand/injector/flags.go b/subcommand/injector/flags.go index 6dbdfd04..f6eb22f2 100644 --- a/subcommand/injector/flags.go +++ b/subcommand/injector/flags.go @@ -69,7 +69,7 @@ type Specification struct { // TelemetryPath is the AGENT_INJECT_TELEMETRY_PATH environment variable. TelemetryPath string `split_words:"true"` - // UseLeaderElector is the AGENT_INJECT_USE_LEADER_ELECTOR + // UseLeaderElector is the AGENT_INJECT_USE_LEADER_ELECTOR environment variable. UseLeaderElector string `split_words:"true"` } From 501069e385c5f4173e1f8a1c535db8c1cc92071f Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Tue, 15 Dec 2020 11:12:05 -0900 Subject: [PATCH 09/14] change namespace length check --- subcommand/injector/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index 3337d129..9c06f28b 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -201,7 +201,7 @@ func (c *Command) Run(args []string) int { func getNamespace() string { namespace := os.Getenv("NAMESPACE") - if len(namespace) > 0 { + if namespace != "" { return namespace } From 5b7f9fb1dbb42eb5faf1035ec07e14e05788051f Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Tue, 15 Dec 2020 20:01:47 -0900 Subject: [PATCH 10/14] updating deploy yaml Using the community gcr registry (k8s.gcr.io), election arg to match the endpoint deploy yaml, and updating the secret name to match the code. --- deploy/injector-deployment.yaml | 4 ++-- deploy/injector-leader-extras.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deploy/injector-deployment.yaml b/deploy/injector-deployment.yaml index a7345da6..c0b50719 100644 --- a/deploy/injector-deployment.yaml +++ b/deploy/injector-deployment.yaml @@ -21,9 +21,9 @@ spec: serviceAccountName: "vault-injector" containers: - name: leader-elector - image: gcr.io/google_containers/leader-elector:0.4 + image: k8s.gcr.io/leader-elector:0.4 args: - - --election=injector-leader + - --election=vault-agent-injector-leader - --election-namespace=$(NAMESPACE) - --http=0.0.0.0:4040 - --ttl=60s diff --git a/deploy/injector-leader-extras.yaml b/deploy/injector-leader-extras.yaml index af286beb..a8e7d158 100644 --- a/deploy/injector-leader-extras.yaml +++ b/deploy/injector-leader-extras.yaml @@ -10,4 +10,4 @@ metadata: apiVersion: v1 kind: Secret metadata: - name: vault-injector-leader + name: vault-injector-certs From 58a5b5f66a92bd34d17669eccd3ca9c27eaebe79 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Tue, 15 Dec 2020 22:01:42 -0900 Subject: [PATCH 11/14] keep the same mutating webhook update frequency To ensure the behavior is unchanged for users not using the leader elector logic. --- subcommand/injector/command.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index 9c06f28b..58cf7666 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -226,14 +226,10 @@ func (c *Command) getCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, clientset *kubernetes.Clientset) { var bundle cert.Bundle - var updateReceived bool for { select { case bundle = <-ch: c.UI.Output("Updated certificate bundle received. Updating certs...") - // this keeps certWatcher from updating the k8s API every second, - // even when there's been no bundle update on the channel - updateReceived = true // Bundle is updated, set it up case <-time.After(1 * time.Second): @@ -266,7 +262,7 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client } // If there is a MWC name set, then update the CA bundle. - if isLeader && updateReceived && c.flagAutoName != "" && len(bundle.CACert) > 0 { + if isLeader && c.flagAutoName != "" && len(bundle.CACert) > 0 { // The CA Bundle value must be base64 encoded value := base64.StdEncoding.EncodeToString(bundle.CACert) @@ -284,9 +280,6 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client err)) continue } - c.UI.Output("Sent new caBundle to mutating webhook config") - - updateReceived = false } // Update the certificate From 9d8292e07b9cca78c338321098d204e3ef49d568 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Tue, 15 Dec 2020 22:04:01 -0900 Subject: [PATCH 12/14] Always recreate CA and Cert on leader change Passing the CA around in the Secret seemed to prevent followers from recreating a CA if they're promoted to leader. Since the followers don't need the CA, removed it from the Secret. --- helper/cert/source_gen.go | 4 ++-- helper/cert/source_gen_test.go | 22 ++++++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index 4a3ea66d..dc5b9ac5 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -85,6 +85,8 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro if err := s.generateCA(); err != nil { return result, err } + // If we had no CA, also ensure the cert is regenerated + last = nil } // Set the CA cert @@ -138,7 +140,6 @@ func (s *GenSource) updateSecret(bundle Bundle) error { Name: certSecretName, }, Data: map[string][]byte{ - "ca": bundle.CACert, "cert": bundle.Cert, "key": bundle.Key, }, @@ -162,7 +163,6 @@ func (s *GenSource) getBundleFromSecret() (Bundle, error) { if err != nil { return bundle, fmt.Errorf("failed to get secret: %s", err) } - bundle.CACert = secret.Data["ca"] bundle.Cert = secret.Data["cert"] bundle.Key = secret.Data["key"] diff --git a/helper/cert/source_gen_test.go b/helper/cert/source_gen_test.go index 15837189..aba0dfa7 100644 --- a/helper/cert/source_gen_test.go +++ b/helper/cert/source_gen_test.go @@ -149,12 +149,12 @@ func TestGenSource_leader(t *testing.T) { // check that the Secret has been created checkSecret, err := source.K8sClient.CoreV1().Secrets(source.Namespace).Get(certSecretName, metav1.GetOptions{}) require.NoError(t, err) - secretBundle := Bundle{ - CACert: checkSecret.Data["ca"], - Cert: checkSecret.Data["cert"], - Key: checkSecret.Data["key"], - } - require.True(t, bundle.Equal(&secretBundle)) + require.Equal(t, checkSecret.Data["cert"], bundle.Cert, + "cert in the Secret should've matched what was returned from source.Certificate()", + ) + require.Equal(t, checkSecret.Data["key"], bundle.Key, + "key in the Secret should've matched what was returned from source.Certificate()", + ) } func TestGenSource_follower(t *testing.T) { @@ -182,7 +182,6 @@ func TestGenSource_follower(t *testing.T) { Namespace: source.Namespace, }, Data: map[string][]byte{ - "ca": secretBundle.CACert, "cert": secretBundle.Cert, "key": secretBundle.Key, }, @@ -198,9 +197,12 @@ func TestGenSource_follower(t *testing.T) { bundle, err := source.Certificate(ctx, nil) require.NoError(t, err) - testBundleVerify(t, &bundle) - require.True(t, bundle.Equal(secretBundle), - "bundle returned from source.Certificate() should have matched what the Secret was created with", + + require.Equal(t, secretBundle.Cert, bundle.Cert, + "cert returned from source.Certificate() should have matched what the Secret was created with", + ) + require.Equal(t, secretBundle.Key, bundle.Key, + "key returned from source.Certificate() should have matched what the Secret was created with", ) } From ac4b4f40036b27b57971bf85dd7e630155bd77e7 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Wed, 16 Dec 2020 09:00:39 -0900 Subject: [PATCH 13/14] Added more logging Added a named logger to GenSource (auto-tls), mostly debug-level for telling which is the leader and which is the follower. --- helper/cert/notify_test.go | 3 +++ helper/cert/source_gen.go | 7 +++++++ helper/cert/source_gen_test.go | 2 ++ subcommand/injector/command.go | 33 +++++++++++++++++---------------- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/helper/cert/notify_test.go b/helper/cert/notify_test.go index 2b220043..d5394b33 100644 --- a/helper/cert/notify_test.go +++ b/helper/cert/notify_test.go @@ -5,6 +5,8 @@ import ( "fmt" "testing" "time" + + "github.com/hashicorp/go-hclog" ) func TestNotify(t *testing.T) { @@ -64,6 +66,7 @@ func TestNotifyRace(t *testing.T) { var certSource Source = &GenSource{ Name: "Agent Inject", Hosts: []string{"some", "hosts"}, + Log: hclog.Default(), } n := NewNotify(ctx, certCh, certSource) diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index dc5b9ac5..fb9a6e73 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -18,6 +18,7 @@ import ( "sync" "time" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault-k8s/leader" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -59,6 +60,8 @@ type GenSource struct { Namespace string SecretsCache informerv1.SecretInformer LeaderElector *leader.LeaderElector + + Log hclog.Logger } // Certificate implements source @@ -76,8 +79,10 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro // and returns that in the result. That will flow through the existing // notify channel structure, testing if it's the same cert as last, etc. if !leaderCheck { + s.Log.Debug("Currently a follower") return s.getBundleFromSecret() } + s.Log.Debug("Currently the leader") } // If we have no CA, generate it for the first time. @@ -87,6 +92,8 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro } // If we had no CA, also ensure the cert is regenerated last = nil + + s.Log.Info("Generated CA") } // Set the CA cert diff --git a/helper/cert/source_gen_test.go b/helper/cert/source_gen_test.go index aba0dfa7..dce8f1b4 100644 --- a/helper/cert/source_gen_test.go +++ b/helper/cert/source_gen_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault-k8s/leader" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" @@ -77,6 +78,7 @@ func testGenSource() *GenSource { return &GenSource{ Name: "Test", Hosts: []string{"127.0.0.1", "localhost"}, + Log: hclog.Default(), } } diff --git a/subcommand/injector/command.go b/subcommand/injector/command.go index 58cf7666..94a81c69 100644 --- a/subcommand/injector/command.go +++ b/subcommand/injector/command.go @@ -105,6 +105,17 @@ func (c *Command) Run(args []string) int { leaderElector = leader.New() } + level, err := c.logLevel() + if err != nil { + c.UI.Error(fmt.Sprintf("Error setting log level: %s", err)) + return 1 + } + + logger := hclog.New(&hclog.LoggerOptions{ + Name: "handler", + Level: level, + JSONFormat: (c.flagLogFormat == "json")}) + // Determine where to source the certificates from var certSource cert.Source = &cert.GenSource{ Name: "Agent Inject", @@ -113,6 +124,7 @@ func (c *Command) Run(args []string) int { Namespace: namespace, SecretsCache: secrets, LeaderElector: leaderElector, + Log: logger.Named("auto-tls"), } if c.flagCertFile != "" { certSource = &cert.DiskSource{ @@ -126,18 +138,7 @@ func (c *Command) Run(args []string) int { certCh := make(chan cert.Bundle) certNotify := cert.NewNotify(ctx, certCh, certSource) go certNotify.Run() - go c.certWatcher(ctx, certCh, clientset) - - level, err := c.logLevel() - if err != nil { - c.UI.Error(fmt.Sprintf("Error setting log level: %s", err)) - return 1 - } - - logger := hclog.New(&hclog.LoggerOptions{ - Name: "handler", - Level: level, - JSONFormat: (c.flagLogFormat == "json")}) + go c.certWatcher(ctx, certCh, clientset, logger.Named("certwatcher")) // Build the HTTP handler and server injector := agentInject.Handler{ @@ -224,12 +225,12 @@ func (c *Command) getCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) return certRaw.(*tls.Certificate), nil } -func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, clientset *kubernetes.Clientset) { +func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, clientset *kubernetes.Clientset, log hclog.Logger) { var bundle cert.Bundle for { select { case bundle = <-ch: - c.UI.Output("Updated certificate bundle received. Updating certs...") + log.Info("Updated certificate bundle received. Updating certs...") // Bundle is updated, set it up case <-time.After(1 * time.Second): @@ -245,7 +246,7 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client crt, err := tls.X509KeyPair(bundle.Cert, bundle.Key) if err != nil { - c.UI.Error(fmt.Sprintf("Error loading TLS keypair: %s", err)) + log.Error(fmt.Sprintf("Error loading TLS keypair: %s", err)) continue } @@ -256,7 +257,7 @@ func (c *Command) certWatcher(ctx context.Context, ch <-chan cert.Bundle, client le := leader.New() isLeader, err = le.IsLeader() if err != nil { - c.UI.Error(fmt.Sprintf("error checking leader: %s", err)) + log.Error(fmt.Sprintf("error checking leader: %s", err)) continue } } From b91470547ba48a6b415bfc2f73f7252a2d5f0bc5 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Wed, 16 Dec 2020 12:07:15 -0900 Subject: [PATCH 14/14] added goroutine to check for leadership change While the leader is waiting for the current certificate to expire, a leadership change could occur, and then the former leader's certificate would be out of sync with the new leader. Added a goroutine that signals a channel on a leadership change. --- helper/cert/source_gen.go | 43 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index fb9a6e73..7e344253 100644 --- a/helper/cert/source_gen.go +++ b/helper/cert/source_gen.go @@ -69,6 +69,7 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro s.mu.Lock() defer s.mu.Unlock() var result Bundle + leaderCh := make(chan bool) if s.LeaderElector != nil { leaderCheck, err := s.LeaderElector.IsLeader() @@ -82,7 +83,13 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro s.Log.Debug("Currently a follower") return s.getBundleFromSecret() } - s.Log.Debug("Currently the leader") + s.Log.Info("Currently the leader") + + // Start a goroutine that checks for a leadership change, otherwise this + // would wait until the current certificate expires before moving on. + changeContext, cancel := context.WithCancel(ctx) + defer cancel() + go s.checkLeader(changeContext, leaderCh) } // If we have no CA, generate it for the first time. @@ -117,6 +124,10 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro defer timer.Stop() select { + case <-leaderCh: + s.Log.Debug("got a leadership change, returning") + return result, fmt.Errorf("lost leadership") + case <-timer.C: // Fall through, generate cert @@ -141,6 +152,36 @@ func (s *GenSource) Certificate(ctx context.Context, last *Bundle) (Bundle, erro return result, err } +func (s *GenSource) checkLeader(ctx context.Context, changed chan<- bool) { + for { + select { + case <-time.After(1 * time.Second): + // Check once a second for a leadership change + s.Log.Named("checkLeader").Trace("checking for leadership change") + + case <-ctx.Done(): + // Quit + return + } + + check, err := s.LeaderElector.IsLeader() + if err != nil { + s.Log.Warn("failed to check for leadership change: %s", err) + } + if !check { + s.Log.Named("checkLeader").Debug("lost the leadership, sending notification") + select { + case changed <- true: + s.Log.Named("checkLeader").Trace("sent changed <- true") + return + case <-ctx.Done(): + s.Log.Named("checkLeader").Trace("got done") + return + } + } + } +} + func (s *GenSource) updateSecret(bundle Bundle) error { secret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{