From a2ecd052400c42731097459413aeb2d92f08d527 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 1 Mar 2021 17:02:08 -0800 Subject: [PATCH] Impersonator config controller writes CA cert & key to different Secret - The CA cert will end up in the end user's kubeconfig on their client machine, so if it changes they would need to fetch the new one and update their kubeconfig. Therefore, we should avoid changing it as much as possible. - Now the controller writes the CA to a different Secret. It writes both the cert and the key so it can reuse them to create more TLS certificates in the future. - For now, it only needs to make more TLS certificates if the old TLS cert Secret gets deleted or updated to be invalid. This allows for manual rotation of the TLS certs by simply deleting the Secret. In the future, we may want to implement some kind of auto rotation. - For now, rotation of both the CA and TLS certs will also happen if you manually delete the CA Secret. However, this would cause the end users to immediately need to get the new CA into their kubeconfig, so this is not as elegant as a normal rotation flow where you would have a window of time where you have more than one CA. --- internal/certauthority/certauthority.go | 27 +- internal/certauthority/certauthority_test.go | 46 +- .../impersonatorconfig/impersonator_config.go | 420 +++++---- .../impersonator_config_test.go | 874 ++++++++++++------ .../controllermanager/prepare_controllers.go | 1 + .../concierge_impersonation_proxy_test.go | 28 +- 6 files changed, 892 insertions(+), 504 deletions(-) diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index 87bdd784..64fa2ecb 100644 --- a/internal/certauthority/certauthority.go +++ b/internal/certauthority/certauthority.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package certauthority implements a simple x509 certificate authority suitable for use in an aggregated API service. @@ -44,12 +44,17 @@ type env struct { // CA holds the state for a simple x509 certificate authority suitable for use in an aggregated API service. type CA struct { - // caCert is the DER-encoded certificate for the current CA. + // caCertBytes is the DER-encoded certificate for the current CA. caCertBytes []byte // signer is the private key for the current CA. signer crypto.Signer + // privateKey is the same private key represented by signer, but in a format which allows export. + // It is only set by New, not by Load, since Load can handle various types of PrivateKey but New + // only needs to create keys of type ecdsa.PrivateKey. + privateKey *ecdsa.PrivateKey + // env is our reference to the outside world (clocks and random number generation). env env } @@ -99,11 +104,11 @@ func newInternal(subject pkix.Name, ttl time.Duration, env env) (*CA, error) { } // Generate a new P256 keypair. - privateKey, err := ecdsa.GenerateKey(elliptic.P256(), env.keygenRNG) + ca.privateKey, err = ecdsa.GenerateKey(elliptic.P256(), env.keygenRNG) if err != nil { return nil, fmt.Errorf("could not generate CA private key: %w", err) } - ca.signer = privateKey + ca.signer = ca.privateKey // Make a CA certificate valid for some ttl and backdated by some amount. now := env.clock() @@ -123,7 +128,7 @@ func newInternal(subject pkix.Name, ttl time.Duration, env env) (*CA, error) { } // Self-sign the CA to get the DER certificate. - caCertBytes, err := x509.CreateCertificate(env.signingRNG, &caTemplate, &caTemplate, &privateKey.PublicKey, privateKey) + caCertBytes, err := x509.CreateCertificate(env.signingRNG, &caTemplate, &caTemplate, &ca.privateKey.PublicKey, ca.privateKey) if err != nil { return nil, fmt.Errorf("could not issue CA certificate: %w", err) } @@ -136,6 +141,18 @@ func (c *CA) Bundle() []byte { return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: c.caCertBytes}) } +// PrivateKeyToPEM returns the current CA private key in PEM format, if this CA was constructed by New. +func (c *CA) PrivateKeyToPEM() ([]byte, error) { + if c.privateKey == nil { + return nil, fmt.Errorf("no private key data (did you try to use this after Load?)") + } + derKey, err := x509.MarshalECPrivateKey(c.privateKey) + if err != nil { + return nil, err + } + return pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: derKey}), nil +} + // Pool returns the current CA signing bundle as a *x509.CertPool. func (c *CA) Pool() *x509.CertPool { pool := x509.NewCertPool() diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index 4c1fdf8e..4193990b 100644 --- a/internal/certauthority/certauthority_test.go +++ b/internal/certauthority/certauthority_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package certauthority @@ -80,22 +80,25 @@ func TestLoad(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, ca.caCertBytes) require.NotNil(t, ca.signer) + require.Nil(t, ca.privateKey) // this struct field is only used for CA's created by New() }) } } func TestNew(t *testing.T) { now := time.Now() - got, err := New(pkix.Name{CommonName: "Test CA"}, time.Minute) + ca, err := New(pkix.Name{CommonName: "Test CA"}, time.Minute) require.NoError(t, err) - require.NotNil(t, got) + require.NotNil(t, ca) // Make sure the CA certificate looks roughly like what we expect. - caCert, err := x509.ParseCertificate(got.caCertBytes) + caCert, err := x509.ParseCertificate(ca.caCertBytes) require.NoError(t, err) require.Equal(t, "Test CA", caCert.Subject.CommonName) require.WithinDuration(t, now.Add(-10*time.Second), caCert.NotBefore, 10*time.Second) require.WithinDuration(t, now.Add(time.Minute), caCert.NotAfter, 10*time.Second) + + require.NotNil(t, ca.privateKey) } func TestNewInternal(t *testing.T) { @@ -175,21 +178,34 @@ func TestNewInternal(t *testing.T) { } func TestBundle(t *testing.T) { - t.Run("success", func(t *testing.T) { - ca := CA{caCertBytes: []byte{1, 2, 3, 4, 5, 6, 7, 8}} - got := ca.Bundle() - require.Equal(t, "-----BEGIN CERTIFICATE-----\nAQIDBAUGBwg=\n-----END CERTIFICATE-----\n", string(got)) - }) + ca := CA{caCertBytes: []byte{1, 2, 3, 4, 5, 6, 7, 8}} + certPEM := ca.Bundle() + require.Equal(t, "-----BEGIN CERTIFICATE-----\nAQIDBAUGBwg=\n-----END CERTIFICATE-----\n", string(certPEM)) +} + +func TestPrivateKeyToPEM(t *testing.T) { + ca, err := New(pkix.Name{CommonName: "Test CA"}, time.Hour) + require.NoError(t, err) + keyPEM, err := ca.PrivateKeyToPEM() + require.NoError(t, err) + require.Regexp(t, "(?s)-----BEGIN EC "+"PRIVATE KEY-----\n.*\n-----END EC PRIVATE KEY-----", string(keyPEM)) + certPEM := ca.Bundle() + // Check that the public and private keys work together. + _, err = tls.X509KeyPair(certPEM, keyPEM) + require.NoError(t, err) + + reloaded, err := Load(string(certPEM), string(keyPEM)) + require.NoError(t, err) + _, err = reloaded.PrivateKeyToPEM() + require.EqualError(t, err, "no private key data (did you try to use this after Load?)") } func TestPool(t *testing.T) { - t.Run("success", func(t *testing.T) { - ca, err := New(pkix.Name{CommonName: "test"}, 1*time.Hour) - require.NoError(t, err) + ca, err := New(pkix.Name{CommonName: "test"}, 1*time.Hour) + require.NoError(t, err) - got := ca.Pool() - require.Len(t, got.Subjects(), 1) - }) + pool := ca.Pool() + require.Len(t, pool.Subjects(), 1) } type errSigner struct { diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index 4e8468d1..661fd086 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -33,7 +33,13 @@ import ( ) const ( - impersonationProxyPort = ":8444" + impersonationProxyPort = "8444" + defaultHTTPSPort = 443 + oneYear = 100 * 365 * 24 * time.Hour + caCommonName = "Pinniped Impersonation Proxy CA" + caCrtKey = "ca.crt" + caKeyKey = "ca.key" + appLabelKey = "app" ) type impersonatorConfigController struct { @@ -45,13 +51,14 @@ type impersonatorConfigController struct { secretsInformer corev1informers.SecretInformer generatedLoadBalancerServiceName string tlsSecretName string + caSecretName string labels map[string]string startTLSListenerFunc StartTLSListenerFunc httpHandlerFactory func() (http.Handler, error) server *http.Server hasControlPlaneNodes *bool - tlsCert *tls.Certificate + tlsCert *tls.Certificate // always read/write using tlsCertMutex tlsCertMutex sync.RWMutex } @@ -68,6 +75,7 @@ func NewImpersonatorConfigController( withInitialEvent pinnipedcontroller.WithInitialEventOptionFunc, generatedLoadBalancerServiceName string, tlsSecretName string, + caSecretName string, labels map[string]string, startTLSListenerFunc StartTLSListenerFunc, httpHandlerFactory func() (http.Handler, error), @@ -84,6 +92,7 @@ func NewImpersonatorConfigController( secretsInformer: secretsInformer, generatedLoadBalancerServiceName: generatedLoadBalancerServiceName, tlsSecretName: tlsSecretName, + caSecretName: caSecretName, labels: labels, startTLSListenerFunc: startTLSListenerFunc, httpHandlerFactory: httpHandlerFactory, @@ -101,10 +110,13 @@ func NewImpersonatorConfigController( ), withInformer( secretsInformer, - pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(tlsSecretName, namespace), + pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { + return (obj.GetName() == tlsSecretName || obj.GetName() == caSecretName) && obj.GetNamespace() == namespace + }, nil), controllerlib.InformerOption{}, ), - // Be sure to run once even if the ConfigMap that the informer is watching doesn't exist. + // Be sure to run once even if the ConfigMap that the informer is watching doesn't exist so we can implement + // the default configuration behavior. withInitialEvent(controllerlib.Key{ Namespace: namespace, Name: configMapResourceName, @@ -112,31 +124,13 @@ func NewImpersonatorConfigController( ) } -func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error { +func (c *impersonatorConfigController) Sync(syncCtx controllerlib.Context) error { plog.Debug("Starting impersonatorConfigController Sync") + ctx := syncCtx.Context - configMap, err := c.configMapsInformer.Lister().ConfigMaps(c.namespace).Get(c.configMapResourceName) - notFound := k8serrors.IsNotFound(err) - if err != nil && !notFound { - return fmt.Errorf("failed to get %s/%s configmap: %w", c.namespace, c.configMapResourceName, err) - } - - var config *impersonator.Config - if notFound { - plog.Info("Did not find impersonation proxy config: using default config values", - "configmap", c.configMapResourceName, - "namespace", c.namespace, - ) - config = impersonator.NewConfig() // use default configuration options - } else { - config, err = impersonator.ConfigFromConfigMap(configMap) - if err != nil { - return fmt.Errorf("invalid impersonator configuration: %v", err) - } - plog.Info("Read impersonation proxy config", - "configmap", c.configMapResourceName, - "namespace", c.namespace, - ) + config, err := c.loadImpersonationProxyConfiguration() + if err != nil { + return err } // Make a live API call to avoid the cost of having an informer watch all node changes on the cluster, @@ -144,7 +138,7 @@ func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error { // Once we have concluded that there is or is not a visible control plane, then cache that decision // to avoid listing nodes very often. if c.hasControlPlaneNodes == nil { - hasControlPlaneNodes, err := clusterhost.New(c.k8sClient).HasControlPlaneNodes(ctx.Context) + hasControlPlaneNodes, err := clusterhost.New(c.k8sClient).HasControlPlaneNodes(ctx) if err != nil { return err } @@ -163,25 +157,25 @@ func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error { } if c.shouldHaveLoadBalancer(config) { - if err = c.ensureLoadBalancerIsStarted(ctx.Context); err != nil { + if err = c.ensureLoadBalancerIsStarted(ctx); err != nil { return err } } else { - if err = c.ensureLoadBalancerIsStopped(ctx.Context); err != nil { + if err = c.ensureLoadBalancerIsStopped(ctx); err != nil { return err } } if c.shouldHaveTLSSecret(config) { - err = c.ensureTLSSecret(ctx, config) - if err != nil { + var impersonationCA *certauthority.CA + if impersonationCA, err = c.ensureCASecretIsCreated(ctx); err != nil { return err } - } else { - err = c.ensureTLSSecretIsRemoved(ctx.Context) - if err != nil { + if err = c.ensureTLSSecret(ctx, config, impersonationCA); err != nil { return err } + } else if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { + return err } plog.Debug("Successfully finished impersonatorConfigController Sync") @@ -189,22 +183,32 @@ func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error { return nil } -func (c *impersonatorConfigController) ensureTLSSecret(ctx controllerlib.Context, config *impersonator.Config) error { - secret, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(c.tlsSecretName) +func (c *impersonatorConfigController) loadImpersonationProxyConfiguration() (*impersonator.Config, error) { + configMap, err := c.configMapsInformer.Lister().ConfigMaps(c.namespace).Get(c.configMapResourceName) notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return nil, fmt.Errorf("failed to get %s/%s configmap: %w", c.namespace, c.configMapResourceName, err) + } + + var config *impersonator.Config if notFound { - secret = nil + plog.Info("Did not find impersonation proxy config: using default config values", + "configmap", c.configMapResourceName, + "namespace", c.namespace, + ) + config = impersonator.NewConfig() // use default configuration options + } else { + config, err = impersonator.ConfigFromConfigMap(configMap) + if err != nil { + return nil, fmt.Errorf("invalid impersonator configuration: %v", err) + } + plog.Info("Read impersonation proxy config", + "configmap", c.configMapResourceName, + "namespace", c.namespace, + ) } - if !notFound && err != nil { - return err - } - if secret, err = c.deleteWhenTLSCertificateDoesNotMatchDesiredState(ctx.Context, config, secret); err != nil { - return err - } - if err = c.ensureTLSSecretIsCreatedAndLoaded(ctx.Context, config, secret); err != nil { - return err - } - return nil + + return config, nil } func (c *impersonatorConfigController) shouldHaveImpersonator(config *impersonator.Config) bool { @@ -219,63 +223,7 @@ func (c *impersonatorConfigController) shouldHaveTLSSecret(config *impersonator. return c.shouldHaveImpersonator(config) } -func certHostnameAndIPMatchDesiredState(desiredIP net.IP, actualIPs []net.IP, desiredHostname string, actualHostnames []string) bool { - if desiredIP != nil && len(actualIPs) == 1 && desiredIP.Equal(actualIPs[0]) && len(actualHostnames) == 0 { - return true - } - if desiredHostname != "" && len(actualHostnames) == 1 && desiredHostname == actualHostnames[0] && len(actualIPs) == 0 { - return true - } - return false -} - -func (c *impersonatorConfigController) ensureImpersonatorIsStopped() error { - if c.server != nil { - plog.Info("Stopping impersonation proxy", "port", impersonationProxyPort) - err := c.server.Close() - c.server = nil - if err != nil { - return err - } - } - return nil -} - -func (c *impersonatorConfigController) ensureImpersonatorIsStarted() error { - if c.server != nil { - return nil - } - - handler, err := c.httpHandlerFactory() - if err != nil { - return err - } - - listener, err := c.startTLSListenerFunc("tcp", impersonationProxyPort, &tls.Config{ - MinVersion: tls.VersionTLS12, // Allow v1.2 because clients like the default `curl` on MacOS don't support 1.3 yet. - GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - return c.getTLSCert(), nil - }, - }) - if err != nil { - return err - } - - c.server = &http.Server{Handler: handler} - - go func() { - plog.Info("Starting impersonation proxy", "port", impersonationProxyPort) - err = c.server.Serve(listener) - if errors.Is(err, http.ErrServerClosed) { - plog.Info("The impersonation proxy server has shut down") - } else { - plog.Error("Unexpected shutdown of the impersonation proxy server", err) - } - }() - return nil -} - -func (c *impersonatorConfigController) isLoadBalancerRunning() (bool, error) { +func (c *impersonatorConfigController) loadBalancerExists() (bool, error) { _, err := c.servicesInformer.Lister().Services(c.namespace).Get(c.generatedLoadBalancerServiceName) notFound := k8serrors.IsNotFound(err) if notFound { @@ -299,26 +247,72 @@ func (c *impersonatorConfigController) tlsSecretExists() (bool, *v1.Secret, erro return true, secret, nil } +func (c *impersonatorConfigController) ensureImpersonatorIsStarted() error { + if c.server != nil { + return nil + } + + handler, err := c.httpHandlerFactory() + if err != nil { + return err + } + + listener, err := c.startTLSListenerFunc("tcp", ":"+impersonationProxyPort, &tls.Config{ + MinVersion: tls.VersionTLS12, // Allow v1.2 because clients like the default `curl` on MacOS don't support 1.3 yet. + GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { + return c.getTLSCert(), nil + }, + }) + if err != nil { + return err + } + + c.server = &http.Server{Handler: handler} + + go func() { + plog.Info("Starting impersonation proxy", "port", impersonationProxyPort) + err = c.server.Serve(listener) + if errors.Is(err, http.ErrServerClosed) { + plog.Info("The impersonation proxy server has shut down") + } else { + plog.Error("Unexpected shutdown of the impersonation proxy server", err) + } + }() + return nil +} + +func (c *impersonatorConfigController) ensureImpersonatorIsStopped() error { + if c.server != nil { + plog.Info("Stopping impersonation proxy", "port", impersonationProxyPort) + err := c.server.Close() + c.server = nil + if err != nil { + return err + } + } + return nil +} + func (c *impersonatorConfigController) ensureLoadBalancerIsStarted(ctx context.Context) error { - running, err := c.isLoadBalancerRunning() + running, err := c.loadBalancerExists() if err != nil { return err } if running { return nil } - appNameLabel := c.labels["app"] + appNameLabel := c.labels[appLabelKey] loadBalancer := v1.Service{ Spec: v1.ServiceSpec{ - Type: "LoadBalancer", + Type: v1.ServiceTypeLoadBalancer, Ports: []v1.ServicePort{ { - TargetPort: intstr.FromInt(8444), - Port: 443, + TargetPort: intstr.FromString(impersonationProxyPort), + Port: defaultHTTPSPort, Protocol: v1.ProtocolTCP, }, }, - Selector: map[string]string{"app": appNameLabel}, + Selector: map[string]string{appLabelKey: appNameLabel}, }, ObjectMeta: metav1.ObjectMeta{ Name: c.generatedLoadBalancerServiceName, @@ -330,14 +324,11 @@ func (c *impersonatorConfigController) ensureLoadBalancerIsStarted(ctx context.C "service", c.generatedLoadBalancerServiceName, "namespace", c.namespace) _, err = c.k8sClient.CoreV1().Services(c.namespace).Create(ctx, &loadBalancer, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("could not create load balancer: %w", err) - } - return nil + return err } func (c *impersonatorConfigController) ensureLoadBalancerIsStopped(ctx context.Context) error { - running, err := c.isLoadBalancerRunning() + running, err := c.loadBalancerExists() if err != nil { return err } @@ -348,45 +339,56 @@ func (c *impersonatorConfigController) ensureLoadBalancerIsStopped(ctx context.C plog.Info("Deleting load balancer for impersonation proxy", "service", c.generatedLoadBalancerServiceName, "namespace", c.namespace) - err = c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedLoadBalancerServiceName, metav1.DeleteOptions{}) - if err != nil { + return c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedLoadBalancerServiceName, metav1.DeleteOptions{}) +} + +func (c *impersonatorConfigController) ensureTLSSecret(ctx context.Context, config *impersonator.Config, ca *certauthority.CA) error { + secretFromInformer, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(c.tlsSecretName) + notFound := k8serrors.IsNotFound(err) + if !notFound && err != nil { return err } - return nil -} - -func (c *impersonatorConfigController) deleteWhenTLSCertificateDoesNotMatchDesiredState(ctx context.Context, config *impersonator.Config, secret *v1.Secret) (*v1.Secret, error) { - if secret == nil { - // There is no Secret, so there is nothing to delete. - return secret, nil + if !notFound { + secretWasDeleted, err := c.deleteTLSSecretWhenCertificateDoesNotMatchDesiredState(ctx, config, ca, secretFromInformer) + if err != nil { + return err + } + // If it was deleted by the above call, then set it to nil. This allows us to avoid waiting + // for the informer cache to update before deciding to proceed to create the new Secret below. + if secretWasDeleted { + secretFromInformer = nil + } } + return c.ensureTLSSecretIsCreatedAndLoaded(ctx, config, secretFromInformer, ca) +} + +func (c *impersonatorConfigController) deleteTLSSecretWhenCertificateDoesNotMatchDesiredState(ctx context.Context, config *impersonator.Config, ca *certauthority.CA, secret *v1.Secret) (bool, error) { certPEM := secret.Data[v1.TLSCertKey] block, _ := pem.Decode(certPEM) if block == nil { plog.Warning("Found missing or not PEM-encoded data in TLS Secret", - "invalidCertPEM", certPEM, + "invalidCertPEM", string(certPEM), "secret", c.tlsSecretName, "namespace", c.namespace) deleteErr := c.ensureTLSSecretIsRemoved(ctx) if deleteErr != nil { - return nil, fmt.Errorf("found missing or not PEM-encoded data in TLS Secret, but got error while deleting it: %w", deleteErr) + return false, fmt.Errorf("found missing or not PEM-encoded data in TLS Secret, but got error while deleting it: %w", deleteErr) } - return nil, nil + return true, nil } actualCertFromSecret, err := x509.ParseCertificate(block.Bytes) if err != nil { plog.Error("Found invalid PEM data in TLS Secret", err, - "invalidCertPEM", certPEM, + "invalidCertPEM", string(certPEM), "secret", c.tlsSecretName, "namespace", c.namespace) - deleteErr := c.ensureTLSSecretIsRemoved(ctx) - if deleteErr != nil { - return nil, fmt.Errorf("PEM data represented an invalid cert, but got error while deleting it: %w", deleteErr) + if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { + return false, fmt.Errorf("PEM data represented an invalid cert, but got error while deleting it: %w", err) } - return nil, nil + return true, nil } keyPEM := secret.Data[v1.TLSPrivateKeyKey] @@ -395,25 +397,33 @@ func (c *impersonatorConfigController) deleteWhenTLSCertificateDoesNotMatchDesir plog.Error("Found invalid private key PEM data in TLS Secret", err, "secret", c.tlsSecretName, "namespace", c.namespace) - deleteErr := c.ensureTLSSecretIsRemoved(ctx) - if deleteErr != nil { - return nil, fmt.Errorf("cert had an invalid private key, but got error while deleting it: %w", deleteErr) + if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { + return false, fmt.Errorf("cert had an invalid private key, but got error while deleting it: %w", err) } - return nil, nil + return true, nil + } + + opts := x509.VerifyOptions{Roots: ca.Pool()} + if _, err = actualCertFromSecret.Verify(opts); err != nil { + // The TLS cert was not signed by the current CA. Since they are mismatched, delete the TLS cert + // so we can recreate it using the current CA. + if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { + return false, err + } + return true, nil } desiredIP, desiredHostname, nameIsReady, err := c.findDesiredTLSCertificateName(config) if err != nil { - return secret, err + return false, err } if !nameIsReady { // We currently have a secret but we are waiting for a load balancer to be assigned an ingress, so // our current secret must be old/unwanted. - err = c.ensureTLSSecretIsRemoved(ctx) - if err != nil { - return secret, err + if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { + return false, err } - return nil, nil + return true, nil } actualIPs := actualCertFromSecret.IPAddresses @@ -428,17 +438,26 @@ func (c *impersonatorConfigController) deleteWhenTLSCertificateDoesNotMatchDesir if certHostnameAndIPMatchDesiredState(desiredIP, actualIPs, desiredHostname, actualHostnames) { // The cert already matches the desired state, so there is no need to delete/recreate it. - return secret, nil + return false, nil } - err = c.ensureTLSSecretIsRemoved(ctx) - if err != nil { - return secret, err + if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { + return false, err } - return nil, nil + return true, nil } -func (c *impersonatorConfigController) ensureTLSSecretIsCreatedAndLoaded(ctx context.Context, config *impersonator.Config, secret *v1.Secret) error { +func certHostnameAndIPMatchDesiredState(desiredIP net.IP, actualIPs []net.IP, desiredHostname string, actualHostnames []string) bool { + if desiredIP != nil && len(actualIPs) == 1 && desiredIP.Equal(actualIPs[0]) && len(actualHostnames) == 0 { + return true + } + if desiredHostname != "" && len(actualHostnames) == 1 && desiredHostname == actualHostnames[0] && len(actualIPs) == 0 { + return true + } + return false +} + +func (c *impersonatorConfigController) ensureTLSSecretIsCreatedAndLoaded(ctx context.Context, config *impersonator.Config, secret *v1.Secret, ca *certauthority.CA) error { if secret != nil { err := c.loadTLSCertFromSecret(secret) if err != nil { @@ -447,13 +466,6 @@ func (c *impersonatorConfigController) ensureTLSSecretIsCreatedAndLoaded(ctx con return nil } - // TODO create/save/watch the CA separately so we can reuse it to mint tls certs as the settings are dynamically changed, - // so that clients don't need to be updated to use a different CA just because the server-side settings were changed. - impersonationCA, err := certauthority.New(pkix.Name{CommonName: "Pinniped Impersonation Proxy CA"}, 100*365*24*time.Hour) - if err != nil { - return fmt.Errorf("could not create impersonation CA: %w", err) - } - ip, hostname, nameIsReady, err := c.findDesiredTLSCertificateName(config) if err != nil { return err @@ -463,15 +475,7 @@ func (c *impersonatorConfigController) ensureTLSSecretIsCreatedAndLoaded(ctx con return nil } - var hostnames []string - var ips []net.IP - if hostname != "" { - hostnames = []string{hostname} - } - if ip != nil { - ips = []net.IP{ip} - } - newTLSSecret, err := c.createNewTLSSecret(ctx, impersonationCA, ips, hostnames) + newTLSSecret, err := c.createNewTLSSecret(ctx, ca, ip, hostname) if err != nil { return err } @@ -484,6 +488,61 @@ func (c *impersonatorConfigController) ensureTLSSecretIsCreatedAndLoaded(ctx con return nil } +func (c *impersonatorConfigController) ensureCASecretIsCreated(ctx context.Context) (*certauthority.CA, error) { + caSecret, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(c.caSecretName) + if err != nil && !k8serrors.IsNotFound(err) { + return nil, err + } + + var impersonationCA *certauthority.CA + if k8serrors.IsNotFound(err) { + impersonationCA, err = c.createCASecret(ctx) + } else { + crtBytes := caSecret.Data[caCrtKey] + keyBytes := caSecret.Data[caKeyKey] + impersonationCA, err = certauthority.Load(string(crtBytes), string(keyBytes)) + } + if err != nil { + return nil, err + } + + return impersonationCA, nil +} + +func (c *impersonatorConfigController) createCASecret(ctx context.Context) (*certauthority.CA, error) { + impersonationCA, err := certauthority.New(pkix.Name{CommonName: caCommonName}, oneYear) + if err != nil { + return nil, fmt.Errorf("could not create impersonation CA: %w", err) + } + + caPrivateKeyPEM, err := impersonationCA.PrivateKeyToPEM() + if err != nil { + return nil, err + } + + secret := v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.caSecretName, + Namespace: c.namespace, + Labels: c.labels, + }, + Data: map[string][]byte{ + caCrtKey: impersonationCA.Bundle(), + caKeyKey: caPrivateKeyPEM, + }, + Type: v1.SecretTypeOpaque, + } + + plog.Info("Creating CA certificates for impersonation proxy", + "secret", c.caSecretName, + "namespace", c.namespace) + if _, err = c.k8sClient.CoreV1().Secrets(c.namespace).Create(ctx, &secret, metav1.CreateOptions{}); err != nil { + return nil, err + } + + return impersonationCA, nil +} + func (c *impersonatorConfigController) findDesiredTLSCertificateName(config *impersonator.Config) (net.IP, string, bool, error) { if config.Endpoint != "" { return c.findTLSCertificateNameFromEndpointConfig(config) @@ -504,7 +563,8 @@ func (c *impersonatorConfigController) findTLSCertificateNameFromLoadBalancer() lb, err := c.servicesInformer.Lister().Services(c.namespace).Get(c.generatedLoadBalancerServiceName) notFound := k8serrors.IsNotFound(err) if notFound { - // Maybe the loadbalancer hasn't been cached in the informer yet. We aren't ready and will try again later. + // Although we created the load balancer, maybe it hasn't been cached in the informer yet. + // We aren't ready and will try again later in this case. return nil, "", false, nil } if err != nil { @@ -534,8 +594,17 @@ func (c *impersonatorConfigController) findTLSCertificateNameFromLoadBalancer() return nil, "", false, fmt.Errorf("could not find valid IP addresses or hostnames from load balancer %s/%s", c.namespace, lb.Name) } -func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, ca *certauthority.CA, ips []net.IP, hostnames []string) (*v1.Secret, error) { - impersonationCert, err := ca.Issue(pkix.Name{}, hostnames, ips, 100*365*24*time.Hour) +func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, ca *certauthority.CA, ip net.IP, hostname string) (*v1.Secret, error) { + var hostnames []string + var ips []net.IP + if hostname != "" { + hostnames = []string{hostname} + } + if ip != nil { + ips = []net.IP{ip} + } + + impersonationCert, err := ca.Issue(pkix.Name{}, hostnames, ips, oneYear) if err != nil { return nil, fmt.Errorf("could not create impersonation cert: %w", err) } @@ -546,17 +615,16 @@ func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, c } newTLSSecret := &v1.Secret{ - Type: v1.SecretTypeTLS, ObjectMeta: metav1.ObjectMeta{ Name: c.tlsSecretName, Namespace: c.namespace, Labels: c.labels, }, Data: map[string][]byte{ - "ca.crt": ca.Bundle(), v1.TLSPrivateKeyKey: keyPEM, v1.TLSCertKey: certPEM, }, + Type: v1.SecretTypeTLS, } plog.Info("Creating TLS certificates for impersonation proxy", @@ -564,12 +632,7 @@ func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, c "hostnames", hostnames, "secret", c.tlsSecretName, "namespace", c.namespace) - _, err = c.k8sClient.CoreV1().Secrets(c.namespace).Create(ctx, newTLSSecret, metav1.CreateOptions{}) - if err != nil { - return nil, err - } - - return newTLSSecret, nil + return c.k8sClient.CoreV1().Secrets(c.namespace).Create(ctx, newTLSSecret, metav1.CreateOptions{}) } func (c *impersonatorConfigController) loadTLSCertFromSecret(tlsSecret *v1.Secret) error { @@ -577,16 +640,11 @@ func (c *impersonatorConfigController) loadTLSCertFromSecret(tlsSecret *v1.Secre keyPEM := tlsSecret.Data[v1.TLSPrivateKeyKey] tlsCert, err := tls.X509KeyPair(certPEM, keyPEM) if err != nil { - plog.Error("Could not parse TLS cert PEM data from Secret", - err, - "secret", c.tlsSecretName, - "namespace", c.namespace, - ) c.setTLSCert(nil) return fmt.Errorf("could not parse TLS cert PEM data from Secret: %w", err) } plog.Info("Loading TLS certificates for impersonation proxy", - "certPEM", certPEM, + "certPEM", string(certPEM), "secret", c.tlsSecretName, "namespace", c.namespace) c.setTLSCert(&tlsCert) diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 7570e2f5..3b779179 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -64,7 +64,8 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { const installedInNamespace = "some-namespace" const configMapResourceName = "some-configmap-resource-name" const generatedLoadBalancerServiceName = "some-service-resource-name" - const tlsSecretName = "some-secret-name" + const tlsSecretName = "some-tls-secret-name" //nolint:gosec // this is not a credential + const caSecretName = "some-ca-secret-name" var r *require.Assertions var observableWithInformerOption *testutil.ObservableWithInformerOption @@ -93,6 +94,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { observableWithInitialEventOption.WithInitialEvent, generatedLoadBalancerServiceName, tlsSecretName, + caSecretName, nil, nil, nil, @@ -200,31 +202,41 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { when("watching Secret objects", func() { var subject controllerlib.Filter - var target, wrongNamespace, wrongName, unrelated *corev1.Secret + var target1, target2, wrongNamespace1, wrongNamespace2, wrongName, unrelated *corev1.Secret it.Before(func() { subject = secretsInformerFilter - target = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: tlsSecretName, Namespace: installedInNamespace}} - wrongNamespace = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: tlsSecretName, Namespace: "wrong-namespace"}} + target1 = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: tlsSecretName, Namespace: installedInNamespace}} + target2 = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: caSecretName, Namespace: installedInNamespace}} + wrongNamespace1 = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: tlsSecretName, Namespace: "wrong-namespace"}} + wrongNamespace2 = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: caSecretName, Namespace: "wrong-namespace"}} wrongName = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: installedInNamespace}} unrelated = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "wrong-namespace"}} }) - when("the target Secret changes", func() { + when("one of the target Secrets changes", func() { it("returns true to trigger the sync method", func() { - r.True(subject.Add(target)) - r.True(subject.Update(target, unrelated)) - r.True(subject.Update(unrelated, target)) - r.True(subject.Delete(target)) + r.True(subject.Add(target1)) + r.True(subject.Update(target1, unrelated)) + r.True(subject.Update(unrelated, target1)) + r.True(subject.Delete(target1)) + r.True(subject.Add(target2)) + r.True(subject.Update(target2, unrelated)) + r.True(subject.Update(unrelated, target2)) + r.True(subject.Delete(target2)) }) }) when("a Secret from another namespace changes", func() { it("returns false to avoid triggering the sync method", func() { - r.False(subject.Add(wrongNamespace)) - r.False(subject.Update(wrongNamespace, unrelated)) - r.False(subject.Update(unrelated, wrongNamespace)) - r.False(subject.Delete(wrongNamespace)) + r.False(subject.Add(wrongNamespace1)) + r.False(subject.Update(wrongNamespace1, unrelated)) + r.False(subject.Update(unrelated, wrongNamespace1)) + r.False(subject.Delete(wrongNamespace1)) + r.False(subject.Add(wrongNamespace2)) + r.False(subject.Update(wrongNamespace2, unrelated)) + r.False(subject.Update(unrelated, wrongNamespace2)) + r.False(subject.Delete(wrongNamespace2)) }) }) @@ -262,7 +274,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { const installedInNamespace = "some-namespace" const configMapResourceName = "some-configmap-resource-name" const loadBalancerServiceName = "some-service-resource-name" - const tlsSecretName = "some-secret-name" + const tlsSecretName = "some-tls-secret-name" //nolint:gosec // this is not a credential + const caSecretName = "some-ca-secret-name" const localhostIP = "127.0.0.1" const httpsPort = ":443" var labels = map[string]string{"app": "app-name", "other-key": "other-value"} @@ -279,7 +292,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var startTLSListenerFuncWasCalled int var startTLSListenerFuncError error var startTLSListenerUponCloseError error - var httpHanderFactoryFuncError error + var httpHandlerFactoryFuncError error var startedTLSListener net.Listener var startTLSListenerFunc = func(network, listenAddress string, config *tls.Config) (net.Listener, error) { @@ -335,7 +348,6 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } else { rootCAs := x509.NewCertPool() rootCAs.AppendCertsFromPEM(caCrt) - tr = &http.Transport{ TLSClientConfig: &tls.Config{RootCAs: rootCAs}, DialContext: overrideDialContext, @@ -390,14 +402,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }, 10*time.Second, time.Millisecond) } - var waitForLoadBalancerToBeDeleted = func(informer corev1informers.ServiceInformer, name string) { + var waitForServiceToBeDeleted = func(informer corev1informers.ServiceInformer, name string) { r.Eventually(func() bool { _, err := informer.Lister().Services(installedInNamespace).Get(name) return k8serrors.IsNotFound(err) }, 10*time.Second, time.Millisecond) } - var waitForTLSCertSecretToBeDeleted = func(informer corev1informers.SecretInformer, name string) { + var waitForSecretToBeDeleted = func(informer corev1informers.SecretInformer, name string) { r.Eventually(func() bool { _, err := informer.Lister().Secrets(installedInNamespace).Get(name) return k8serrors.IsNotFound(err) @@ -419,13 +431,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { controllerlib.WithInitialEvent, loadBalancerServiceName, tlsSecretName, + caSecretName, labels, startTLSListenerFunc, func() (http.Handler, error) { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { _, err := fmt.Fprintf(w, "hello world") r.NoError(err) - }), httpHanderFactoryFuncError + }), httpHandlerFactoryFuncError }, ) @@ -494,34 +507,51 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } } - var newStubTLSSecret = func(resourceName string) *corev1.Secret { + var newEmptySecret = func(resourceName string) *corev1.Secret { return newSecretWithData(resourceName, map[string][]byte{}) } - var createCertSecretData = func(dnsNames []string, ip string) map[string][]byte { - impersonationCA, err := certauthority.New(pkix.Name{CommonName: "test CA"}, 24*time.Hour) + var newCA = func() *certauthority.CA { + ca, err := certauthority.New(pkix.Name{CommonName: "test CA"}, 24*time.Hour) r.NoError(err) - impersonationCert, err := impersonationCA.Issue(pkix.Name{}, dnsNames, []net.IP{net.ParseIP(ip)}, 24*time.Hour) + return ca + } + + var newCACertSecretData = func(ca *certauthority.CA) map[string][]byte { + keyPEM, err := ca.PrivateKeyToPEM() + r.NoError(err) + return map[string][]byte{ + "ca.crt": ca.Bundle(), + "ca.key": keyPEM, + } + } + + var newTLSCertSecretData = func(ca *certauthority.CA, dnsNames []string, ip string) map[string][]byte { + impersonationCert, err := ca.Issue(pkix.Name{}, dnsNames, []net.IP{net.ParseIP(ip)}, 24*time.Hour) r.NoError(err) certPEM, keyPEM, err := certauthority.ToPEM(impersonationCert) r.NoError(err) return map[string][]byte{ - "ca.crt": impersonationCA.Bundle(), corev1.TLSPrivateKeyKey: keyPEM, corev1.TLSCertKey: certPEM, } } - var newActualTLSSecret = func(resourceName string, ip string) *corev1.Secret { - return newSecretWithData(resourceName, createCertSecretData(nil, ip)) + var newActualCASecret = func(ca *certauthority.CA, resourceName string) *corev1.Secret { + return newSecretWithData(resourceName, newCACertSecretData(ca)) } - var newActualTLSSecretWithMultipleHostnames = func(resourceName string, ip string) *corev1.Secret { - return newSecretWithData(resourceName, createCertSecretData([]string{"foo", "bar"}, ip)) + var newActualTLSSecret = func(ca *certauthority.CA, resourceName string, ip string) *corev1.Secret { + return newSecretWithData(resourceName, newTLSCertSecretData(ca, nil, ip)) + } + + var newActualTLSSecretWithMultipleHostnames = func(ca *certauthority.CA, resourceName string, ip string) *corev1.Secret { + return newSecretWithData(resourceName, newTLSCertSecretData(ca, []string{"foo", "bar"}, ip)) } var addSecretFromCreateActionToTracker = func(action coretesting.Action, client *kubernetesfake.Clientset, resourceVersion string) { - createdSecret := action.(coretesting.CreateAction).GetObject().(*corev1.Secret) + createdSecret, ok := action.(coretesting.CreateAction).GetObject().(*corev1.Secret) + r.True(ok, "should have been able to cast this action to CreateAction: %v", action) createdSecret.ResourceVersion = resourceVersion r.NoError(client.Tracker().Add(createdSecret)) } @@ -561,8 +591,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(client.Tracker().Add(loadBalancerService)) } - var addSecretToTracker = func(secret *corev1.Secret, client *kubernetesfake.Clientset) { - r.NoError(client.Tracker().Add(secret)) + var addSecretToTrackers = func(secret *corev1.Secret, clients ...*kubernetesfake.Clientset) { + for _, client := range clients { + r.NoError(client.Tracker().Add(secret)) + } } var updateLoadBalancerServiceInTracker = func(resourceName string, ingresses []corev1.LoadBalancerIngress, client *kubernetesfake.Clientset, newResourceVersion string) { @@ -582,7 +614,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { )) } - var deleteLoadBalancerServiceFromTracker = func(resourceName string, client *kubernetesfake.Clientset) { + var deleteServiceFromTracker = func(resourceName string, client *kubernetesfake.Clientset) { r.NoError(client.Tracker().Delete( schema.GroupVersionResource{Version: "v1", Resource: "services"}, installedInNamespace, @@ -590,7 +622,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { )) } - var deleteTLSCertSecretFromTracker = func(resourceName string, client *kubernetesfake.Clientset) { + var deleteSecretFromTracker = func(resourceName string, client *kubernetesfake.Clientset) { r.NoError(client.Tracker().Delete( schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, installedInNamespace, @@ -621,7 +653,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } var requireLoadBalancerWasCreated = func(action coretesting.Action) { - createAction := action.(coretesting.CreateAction) + createAction, ok := action.(coretesting.CreateAction) + r.True(ok, "should have been able to cast this action to CreateAction: %v", action) r.Equal("create", createAction.GetVerb()) createdLoadBalancerService := createAction.GetObject().(*corev1.Service) r.Equal(loadBalancerServiceName, createdLoadBalancerService.Name) @@ -631,44 +664,66 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Equal(labels, createdLoadBalancerService.Labels) } - var requireLoadBalancerDeleted = func(action coretesting.Action) { - deleteAction := action.(coretesting.DeleteAction) + var requireLoadBalancerWasDeleted = func(action coretesting.Action) { + deleteAction, ok := action.(coretesting.DeleteAction) + r.True(ok, "should have been able to cast this action to DeleteAction: %v", action) r.Equal("delete", deleteAction.GetVerb()) r.Equal(loadBalancerServiceName, deleteAction.GetName()) r.Equal("services", deleteAction.GetResource().Resource) } - var requireTLSSecretDeleted = func(action coretesting.Action) { - deleteAction := action.(coretesting.DeleteAction) + var requireTLSSecretWasDeleted = func(action coretesting.Action) { + deleteAction, ok := action.(coretesting.DeleteAction) + r.True(ok, "should have been able to cast this action to DeleteAction: %v", action) r.Equal("delete", deleteAction.GetVerb()) r.Equal(tlsSecretName, deleteAction.GetName()) r.Equal("secrets", deleteAction.GetResource().Resource) } - var requireTLSSecretWasCreated = func(action coretesting.Action) []byte { - createAction := action.(coretesting.CreateAction) + var requireCASecretWasCreated = func(action coretesting.Action) []byte { + createAction, ok := action.(coretesting.CreateAction) + r.True(ok, "should have been able to cast this action to CreateAction: %v", action) r.Equal("create", createAction.GetVerb()) createdSecret := createAction.GetObject().(*corev1.Secret) - r.Equal(tlsSecretName, createdSecret.Name) + r.Equal(caSecretName, createdSecret.Name) r.Equal(installedInNamespace, createdSecret.Namespace) - r.Equal(corev1.SecretTypeTLS, createdSecret.Type) + r.Equal(corev1.SecretTypeOpaque, createdSecret.Type) r.Equal(labels, createdSecret.Labels) - r.Len(createdSecret.Data, 3) - r.NotNil(createdSecret.Data["ca.crt"]) - r.NotNil(createdSecret.Data[corev1.TLSPrivateKeyKey]) - r.NotNil(createdSecret.Data[corev1.TLSCertKey]) - validCert := testutil.ValidateCertificate(t, string(createdSecret.Data["ca.crt"]), string(createdSecret.Data[corev1.TLSCertKey])) - validCert.RequireMatchesPrivateKey(string(createdSecret.Data[corev1.TLSPrivateKeyKey])) - validCert.RequireLifetime(time.Now().Add(-10*time.Second), time.Now().Add(100*time.Hour*24*365), 10*time.Second) - // Make sure the CA certificate looks roughly like what we expect. - block, _ := pem.Decode(createdSecret.Data["ca.crt"]) + r.Len(createdSecret.Data, 2) + createdCertPEM := createdSecret.Data["ca.crt"] + createdKeyPEM := createdSecret.Data["ca.key"] + r.NotNil(createdCertPEM) + r.NotNil(createdKeyPEM) + _, err := tls.X509KeyPair(createdCertPEM, createdKeyPEM) + r.NoError(err, "key does not match cert") + // Decode and parse the cert to check some of its fields. + block, _ := pem.Decode(createdCertPEM) require.NotNil(t, block) caCert, err := x509.ParseCertificate(block.Bytes) require.NoError(t, err) require.Equal(t, "Pinniped Impersonation Proxy CA", caCert.Subject.CommonName) require.WithinDuration(t, time.Now().Add(-10*time.Second), caCert.NotBefore, 10*time.Second) require.WithinDuration(t, time.Now().Add(100*time.Hour*24*365), caCert.NotAfter, 10*time.Second) - return createdSecret.Data["ca.crt"] + return createdCertPEM + } + + var requireTLSSecretWasCreated = func(action coretesting.Action, caCert []byte) { + createAction, ok := action.(coretesting.CreateAction) + r.True(ok, "should have been able to cast this action to CreateAction: %v", action) + r.Equal("create", createAction.GetVerb()) + createdSecret := createAction.GetObject().(*corev1.Secret) + r.Equal(tlsSecretName, createdSecret.Name) + r.Equal(installedInNamespace, createdSecret.Namespace) + r.Equal(corev1.SecretTypeTLS, createdSecret.Type) + r.Equal(labels, createdSecret.Labels) + r.Len(createdSecret.Data, 2) + createdCertPEM := createdSecret.Data[corev1.TLSCertKey] + createdKeyPEM := createdSecret.Data[corev1.TLSPrivateKeyKey] + r.NotNil(createdKeyPEM) + r.NotNil(createdCertPEM) + validCert := testutil.ValidateCertificate(t, string(caCert), string(createdCertPEM)) + validCert.RequireMatchesPrivateKey(string(createdKeyPEM)) + validCert.RequireLifetime(time.Now().Add(-10*time.Second), time.Now().Add(100*time.Hour*24*365), 10*time.Second) } var runControllerSync = func() error { @@ -714,9 +769,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { addNodeWithRoleToTracker("control-plane", kubeAPIClient) addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeInformerClient) addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeAPIClient) - tlsSecret := newStubTLSSecret(tlsSecretName) - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + addSecretToTrackers(newEmptySecret(tlsSecretName), kubeAPIClient, kubeInformerClient) }) it("does not start the impersonator, deletes the loadbalancer, deletes the Secret", func() { @@ -725,8 +778,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireTLSServerWasNeverStarted() r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - requireLoadBalancerDeleted(kubeAPIClient.Actions()[1]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[2]) + requireLoadBalancerWasDeleted(kubeAPIClient.Actions()[1]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[2]) }) }) @@ -739,9 +792,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the load balancer automatically", func() { requireTLSServerIsRunningWithoutCerts() - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + requireCASecretWasCreated(kubeAPIClient.Actions()[2]) }) }) @@ -756,8 +810,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("does not start the load balancer automatically", func() { requireTLSServerIsRunningWithoutCerts() - r.Len(kubeAPIClient.Actions(), 1) + r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) + requireCASecretWasCreated(kubeAPIClient.Actions()[1]) }) }) @@ -772,8 +827,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("does not start the load balancer automatically", func() { requireTLSServerIsRunningWithoutCerts() - r.Len(kubeAPIClient.Actions(), 1) + r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) + requireCASecretWasCreated(kubeAPIClient.Actions()[1]) }) }) @@ -788,8 +844,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("does not start the load balancer automatically", func() { requireTLSServerIsRunningWithoutCerts() - r.Len(kubeAPIClient.Actions(), 1) + r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) + requireCASecretWasCreated(kubeAPIClient.Actions()[1]) }) }) @@ -803,17 +860,22 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) it("starts the impersonator with certs that match the first IP address", func() { - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) requireTLSServerIsRunning(ca, "127.0.0.123", map[string]string{"127.0.0.123:443": testServerAddr()}) }) it("keeps the secret around after resync", func() { + // Simulate the informer cache's background update from its watch. addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "0") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "0") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) // nothing changed + r.Len(kubeAPIClient.Actions(), 3) // nothing changed }) }) @@ -828,17 +890,22 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) it("starts the impersonator with certs that match the first hostname", func() { - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) requireTLSServerIsRunning(ca, firstHostname, map[string]string{firstHostname + httpsPort: testServerAddr()}) }) it("keeps the secret around after resync", func() { + // Simulate the informer cache's background update from its watch. addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "0") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "0") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) // nothing changed + r.Len(kubeAPIClient.Actions(), 3) // nothing changed }) }) @@ -853,28 +920,36 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) it("starts the impersonator with certs that match the first hostname", func() { - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) requireTLSServerIsRunning(ca, firstHostname, map[string]string{firstHostname + httpsPort: testServerAddr()}) }) it("keeps the secret around after resync", func() { + // Simulate the informer cache's background update from its watch. addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "0") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "0") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) // nothing changed + r.Len(kubeAPIClient.Actions(), 3) // nothing changed }) }) - when("there are not visible control plane nodes, a secret exists with multiple hostnames and an IP", func() { + when("there are not visible control plane nodes, a TLS secret exists with multiple hostnames and an IP", func() { + var caCrt []byte it.Before(func() { addNodeWithRoleToTracker("worker", kubeAPIClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeAPIClient) - tlsSecret := newActualTLSSecretWithMultipleHostnames(tlsSecretName, localhostIP) - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + ca := newCA() + caSecret := newActualCASecret(ca, caSecretName) + caCrt = caSecret.Data["ca.crt"] + addSecretToTrackers(caSecret, kubeAPIClient, kubeInformerClient) + addSecretToTrackers(newActualTLSSecretWithMultipleHostnames(ca, tlsSecretName, localhostIP), kubeAPIClient, kubeInformerClient) startInformersAndController() r.NoError(runControllerSync()) }) @@ -882,9 +957,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("deletes and recreates the secret to match the IP in the load balancer without the extra hostnames", func() { r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[2]) - requireTLSServerIsRunning(ca, testServerAddr(), nil) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], caCrt) + requireTLSServerIsRunning(caCrt, testServerAddr(), nil) }) }) @@ -893,9 +968,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { addNodeWithRoleToTracker("worker", kubeAPIClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: "127.0.0.42"}}, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: "127.0.0.42"}}, kubeAPIClient) - tlsSecret := newActualTLSSecret(tlsSecretName, localhostIP) - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + ca := newCA() + addSecretToTrackers(newActualCASecret(ca, caSecretName), kubeAPIClient, kubeInformerClient) + addSecretToTrackers(newActualTLSSecretWithMultipleHostnames(ca, tlsSecretName, localhostIP), kubeAPIClient, kubeInformerClient) kubeAPIClient.PrependReactor("delete", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, fmt.Errorf("error on delete") }) @@ -906,21 +981,23 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Error(runControllerSync(), "error on delete") r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1]) requireTLSServerIsRunningWithoutCerts() }) }) when("the cert's name might need to change but there is an error while determining the new name", func() { - var ca []byte + var caCrt []byte it.Before(func() { addNodeWithRoleToTracker("worker", kubeAPIClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeAPIClient) - tlsSecret := newActualTLSSecret(tlsSecretName, localhostIP) - ca = tlsSecret.Data["ca.crt"] - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + ca := newCA() + caSecret := newActualCASecret(ca, caSecretName) + caCrt = caSecret.Data["ca.crt"] + addSecretToTrackers(caSecret, kubeAPIClient, kubeInformerClient) + tlsSecret := newActualTLSSecret(ca, tlsSecretName, localhostIP) + addSecretToTrackers(tlsSecret, kubeAPIClient, kubeInformerClient) }) it("returns an error and keeps running the proxy with the old cert", func() { @@ -928,7 +1005,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(runControllerSync()) r.Len(kubeAPIClient.Actions(), 1) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSServerIsRunning(ca, testServerAddr(), nil) + requireTLSServerIsRunning(caCrt, testServerAddr(), nil) updateLoadBalancerServiceInTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: "not-an-ip"}}, kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") @@ -936,131 +1013,11 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.EqualError(runControllerSync(), "could not find valid IP addresses or hostnames from load balancer some-namespace/some-service-resource-name") r.Len(kubeAPIClient.Actions(), 1) // no new actions - requireTLSServerIsRunning(ca, testServerAddr(), nil) + requireTLSServerIsRunning(caCrt, testServerAddr(), nil) }) }) }) - when("sync is called more than once", func() { - it.Before(func() { - addNodeWithRoleToTracker("worker", kubeAPIClient) - }) - - it("only starts the impersonator once and only lists the cluster's nodes once", func() { - startInformersAndController() - r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) - requireNodesListed(kubeAPIClient.Actions()[0]) - requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) - requireTLSServerIsRunningWithoutCerts() - - // Simulate the informer cache's background update from its watch. - addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "1") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") - - r.NoError(runControllerSync()) - r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time - requireTLSServerIsRunningWithoutCerts() // still running - r.Len(kubeAPIClient.Actions(), 2) // no new API calls - }) - - it("creates certs from the ip address listed on the load balancer", func() { - startInformersAndController() - r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) - requireNodesListed(kubeAPIClient.Actions()[0]) - requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) - requireTLSServerIsRunningWithoutCerts() - - // Simulate the informer cache's background update from its watch. - addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "0") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0") - - updateLoadBalancerServiceInTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient, "1") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") - - r.NoError(runControllerSync()) - r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time - r.Len(kubeAPIClient.Actions(), 3) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[2]) - requireTLSServerIsRunning(ca, testServerAddr(), nil) // running with certs now - - // Simulate the informer cache's background update from its watch. - addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") - - r.NoError(runControllerSync()) - r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a third time - r.Len(kubeAPIClient.Actions(), 3) // no more actions - requireTLSServerIsRunning(ca, testServerAddr(), nil) // still running - }) - - it("creates certs from the hostname listed on the load balancer", func() { - hostname := "fake.example.com" - startInformersAndController() - r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) - requireNodesListed(kubeAPIClient.Actions()[0]) - requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) - requireTLSServerIsRunningWithoutCerts() - - // Simulate the informer cache's background update from its watch. - addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "0") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0") - - updateLoadBalancerServiceInTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP, Hostname: hostname}}, kubeInformerClient, "1") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") - - r.NoError(runControllerSync()) - r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time - r.Len(kubeAPIClient.Actions(), 3) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[2]) - requireTLSServerIsRunning(ca, hostname, map[string]string{hostname + httpsPort: testServerAddr()}) // running with certs now - - // Simulate the informer cache's background update from its watch. - addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") - - r.NoError(runControllerSync()) - r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a third time - r.Len(kubeAPIClient.Actions(), 3) // no more actions - requireTLSServerIsRunning(ca, hostname, map[string]string{hostname + httpsPort: testServerAddr()}) // still running - }) - }) - - when("getting the control plane nodes returns an error, e.g. when there are no nodes", func() { - it("returns an error", func() { - startInformersAndController() - r.EqualError(runControllerSync(), "no nodes found") - requireTLSServerWasNeverStarted() - }) - }) - - when("the http handler factory function returns an error", func() { - it.Before(func() { - addNodeWithRoleToTracker("worker", kubeAPIClient) - httpHanderFactoryFuncError = errors.New("some factory error") - }) - - it("returns an error", func() { - startInformersAndController() - r.EqualError(runControllerSync(), "some factory error") - requireTLSServerWasNeverStarted() - }) - }) - - when("the configmap is invalid", func() { - it.Before(func() { - addImpersonatorConfigMapToTracker(configMapResourceName, "not yaml", kubeInformerClient) - }) - - it("returns an error", func() { - startInformersAndController() - r.EqualError(runControllerSync(), "invalid impersonator configuration: decode yaml: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type impersonator.Config") - requireTLSServerWasNeverStarted() - }) - }) - when("the ConfigMap is already in the installation namespace", func() { when("the configuration is auto mode with an endpoint", func() { it.Before(func() { @@ -1090,9 +1047,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the impersonator according to the settings in the ConfigMap", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) requireTLSServerIsRunning(ca, testServerAddr(), nil) }) }) @@ -1135,9 +1093,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the load balancer", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + requireCASecretWasCreated(kubeAPIClient.Actions()[2]) }) }) @@ -1164,20 +1123,23 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("does not start the load balancer", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 1) + r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) + requireCASecretWasCreated(kubeAPIClient.Actions()[1]) }) }) when("a load balancer and a secret already exists", func() { - var ca []byte + var caCrt []byte it.Before(func() { addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled", kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) - tlsSecret := newActualTLSSecret(tlsSecretName, localhostIP) - ca = tlsSecret.Data["ca.crt"] - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + ca := newCA() + caSecret := newActualCASecret(ca, caSecretName) + caCrt = caSecret.Data["ca.crt"] + addSecretToTrackers(caSecret, kubeAPIClient, kubeInformerClient) + tlsSecret := newActualTLSSecret(ca, tlsSecretName, localhostIP) + addSecretToTrackers(tlsSecret, kubeAPIClient, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeAPIClient) }) @@ -1187,7 +1149,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(runControllerSync()) r.Len(kubeAPIClient.Actions(), 1) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSServerIsRunning(ca, testServerAddr(), nil) + requireTLSServerIsRunning(caCrt, testServerAddr(), nil) }) }) @@ -1202,18 +1164,19 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the impersonator, generates a valid cert for the hostname", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) // Check that the server is running and that TLS certs that are being served are are for fakeHostname. requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) }) }) when("endpoint is IP address with a port", func() { - const fakeIpWithPort = "127.0.0.1:3000" + const fakeIPWithPort = "127.0.0.1:3000" it.Before(func() { - configMapYAML := fmt.Sprintf("{mode: enabled, endpoint: %s}", fakeIpWithPort) + configMapYAML := fmt.Sprintf("{mode: enabled, endpoint: %s}", fakeIPWithPort) addImpersonatorConfigMapToTracker(configMapResourceName, configMapYAML, kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) }) @@ -1221,11 +1184,12 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the impersonator, generates a valid cert for the hostname", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) - // Check that the server is running and that TLS certs that are being served are are for fakeIpWithPort. - requireTLSServerIsRunning(ca, fakeIpWithPort, map[string]string{fakeIpWithPort: testServerAddr()}) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) + // Check that the server is running and that TLS certs that are being served are are for fakeIPWithPort. + requireTLSServerIsRunning(ca, fakeIPWithPort, map[string]string{fakeIPWithPort: testServerAddr()}) }) }) @@ -1240,9 +1204,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the impersonator, generates a valid cert for the hostname", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) // Check that the server is running and that TLS certs that are being served are are for fakeHostnameWithPort. requireTLSServerIsRunning(ca, fakeHostnameWithPort, map[string]string{fakeHostnameWithPort: testServerAddr()}) }) @@ -1261,44 +1226,180 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("regenerates the cert for the hostname, then regenerates it for the IP again", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) // Check that the server is running and that TLS certs that are being served are are for fakeIP. requireTLSServerIsRunning(ca, fakeIP, map[string]string{fakeIP + httpsPort: testServerAddr()}) // Simulate the informer cache's background update from its watch. addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "2") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "2") // Switch the endpoint config to a hostname. updateImpersonatorConfigMapInTracker(configMapResourceName, hostnameYAML, kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1") r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 4) - requireTLSSecretDeleted(kubeAPIClient.Actions()[2]) - ca = requireTLSSecretWasCreated(kubeAPIClient.Actions()[3]) + r.Len(kubeAPIClient.Actions(), 5) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[3]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[4], ca) // reuses the old CA // Check that the server is running and that TLS certs that are being served are are for fakeHostname. requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) // Simulate the informer cache's background update from its watch. - deleteTLSCertSecretFromTracker(tlsSecretName, kubeInformerClient) - addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[3], kubeInformerClient, "2") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "2") + deleteSecretFromTracker(tlsSecretName, kubeInformerClient) + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[4], kubeInformerClient, "3") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "3") // Switch the endpoint config back to an IP. updateImpersonatorConfigMapInTracker(configMapResourceName, ipAddressYAML, kubeInformerClient, "2") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2") r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 6) - requireTLSSecretDeleted(kubeAPIClient.Actions()[4]) - ca = requireTLSSecretWasCreated(kubeAPIClient.Actions()[5]) + r.Len(kubeAPIClient.Actions(), 7) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[5]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[6], ca) // reuses the old CA again // Check that the server is running and that TLS certs that are being served are are for fakeIP. requireTLSServerIsRunning(ca, fakeIP, map[string]string{fakeIP + httpsPort: testServerAddr()}) }) }) + + when("the TLS cert goes missing and needs to be recreated, e.g. when a user manually deleted it", func() { + const fakeHostname = "fake.example.com" + it.Before(func() { + configMapYAML := fmt.Sprintf("{mode: enabled, endpoint: %s}", fakeHostname) + addImpersonatorConfigMapToTracker(configMapResourceName, configMapYAML, kubeInformerClient) + addNodeWithRoleToTracker("worker", kubeAPIClient) + startInformersAndController() + }) + + it("uses the existing CA cert the make a new TLS cert", func() { + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) + // Check that the server is running and that TLS certs that are being served are are for fakeHostname. + requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + + // Simulate the informer cache's background update from its watch for the CA Secret. + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + + // Delete the TLS Secret that was just created from the Kube API server. Note that we never + // simulated it getting added to the informer cache, so we don't need to remove it from there. + deleteSecretFromTracker(tlsSecretName, kubeAPIClient) + + // Run again. It should create a new TLS cert using the old CA cert. + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 4) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[3], ca) + // Check that the server is running and that TLS certs that are being served are are for fakeHostname. + requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + }) + }) + + when("the CA cert goes missing and needs to be recreated, e.g. when a user manually deleted it", func() { + const fakeHostname = "fake.example.com" + it.Before(func() { + configMapYAML := fmt.Sprintf("{mode: enabled, endpoint: %s}", fakeHostname) + addImpersonatorConfigMapToTracker(configMapResourceName, configMapYAML, kubeInformerClient) + addNodeWithRoleToTracker("worker", kubeAPIClient) + startInformersAndController() + }) + + it("makes a new CA cert, deletes the old TLS cert, and makes a new TLS cert using the new CA", func() { + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) + // Check that the server is running and that TLS certs that are being served are are for fakeHostname. + requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + + // Simulate the informer cache's background update from its watch for the CA Secret. + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + + // Delete the CA Secret that was just created from the Kube API server. Note that we never + // simulated it getting added to the informer cache, so we don't need to remove it from there. + deleteSecretFromTracker(caSecretName, kubeAPIClient) + + // Run again. It should create both a new CA cert and a new TLS cert using the new CA cert. + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 6) + ca = requireCASecretWasCreated(kubeAPIClient.Actions()[3]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[4]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[5], ca) // created using the new CA + // Check that the server is running and that TLS certs that are being served are are for fakeHostname. + requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + }) + }) + + when("the CA cert is overwritten by another valid CA cert", func() { + const fakeHostname = "fake.example.com" + var caCrt []byte + it.Before(func() { + configMapYAML := fmt.Sprintf("{mode: enabled, endpoint: %s}", fakeHostname) + addImpersonatorConfigMapToTracker(configMapResourceName, configMapYAML, kubeInformerClient) + addNodeWithRoleToTracker("worker", kubeAPIClient) + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) + // Check that the server is running and that TLS certs that are being served are are for fakeHostname. + requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + + // Simulate the informer cache's background update from its watch for the CA Secret. + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + + // Simulate someone updating the CA Secret out of band, e.g. when a human edits it with kubectl. + // Delete the CA Secret that was just created from the Kube API server. Note that we never + // simulated it getting added to the informer cache, so we don't need to remove it from there. + // Then add a new one. Delete + new = update, since only the final state is observed. + deleteSecretFromTracker(caSecretName, kubeAPIClient) + anotherCA := newCA() + newCASecret := newActualCASecret(anotherCA, caSecretName) + caCrt = newCASecret.Data["ca.crt"] + newCASecret.ResourceVersion = "2" + addSecretToTrackers(newCASecret, kubeInformerClient, kubeAPIClient) + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "2") + }) + + it("deletes the old TLS cert and makes a new TLS cert using the new CA", func() { + // Run again. It should use the updated CA cert to create a new TLS cert. + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 5) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[3]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[4], caCrt) // created using the updated CA + // Check that the server is running and that TLS certs that are being served are are for fakeHostname. + requireTLSServerIsRunning(caCrt, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + }) + + when("deleting the TLS cert due to mismatched CA results in an error", func() { + it.Before(func() { + kubeAPIClient.PrependReactor("delete", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { + if action.(coretesting.DeleteAction).GetName() == tlsSecretName { + return true, nil, fmt.Errorf("error on tls secret delete") + } + return false, nil, nil + }) + }) + + it("returns an error", func() { + r.Error(runControllerSync(), "error on tls secret delete") + r.Len(kubeAPIClient.Actions(), 4) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[3]) // tried to delete cert but failed + }) + }) + }) }) when("the configuration switches from enabled to disabled mode", func() { @@ -1312,32 +1413,35 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(runControllerSync()) requireTLSServerIsRunningWithoutCerts() - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + requireCASecretWasCreated(kubeAPIClient.Actions()[2]) // Simulate the informer cache's background update from its watch. addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: disabled", kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1") r.NoError(runControllerSync()) requireTLSServerIsNoLongerRunning() - r.Len(kubeAPIClient.Actions(), 3) - requireLoadBalancerDeleted(kubeAPIClient.Actions()[2]) + r.Len(kubeAPIClient.Actions(), 4) + requireLoadBalancerWasDeleted(kubeAPIClient.Actions()[3]) - deleteLoadBalancerServiceFromTracker(loadBalancerServiceName, kubeInformerClient) - waitForLoadBalancerToBeDeleted(kubeInformers.Core().V1().Services(), loadBalancerServiceName) + deleteServiceFromTracker(loadBalancerServiceName, kubeInformerClient) + waitForServiceToBeDeleted(kubeInformers.Core().V1().Services(), loadBalancerServiceName) updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: enabled", kubeInformerClient, "2") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2") r.NoError(runControllerSync()) requireTLSServerIsRunningWithoutCerts() - r.Len(kubeAPIClient.Actions(), 4) - requireLoadBalancerWasCreated(kubeAPIClient.Actions()[3]) + r.Len(kubeAPIClient.Actions(), 5) + requireLoadBalancerWasCreated(kubeAPIClient.Actions()[4]) }) when("there is an error while shutting down the server", func() { @@ -1371,34 +1475,37 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { // Should have started in "enabled" mode with an "endpoint", so no load balancer is needed. r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) // created immediately because "endpoint" was specified + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) // created immediately because "endpoint" was specified + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) requireTLSServerIsRunning(ca, testServerAddr(), nil) // Simulate the informer cache's background update from its watch. addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "2") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "2") // Switch to "enabled" mode without an "endpoint", so a load balancer is needed now. updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: enabled", kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1") r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 4) - requireLoadBalancerWasCreated(kubeAPIClient.Actions()[2]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[3]) // the Secret was deleted because it contained a cert with the wrong IP + r.Len(kubeAPIClient.Actions(), 5) + requireLoadBalancerWasCreated(kubeAPIClient.Actions()[3]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[4]) // the Secret was deleted because it contained a cert with the wrong IP requireTLSServerIsRunningWithoutCerts() // Simulate the informer cache's background update from its watch. - addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[3], kubeInformerClient, "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") - deleteTLSCertSecretFromTracker(tlsSecretName, kubeInformerClient) - waitForTLSCertSecretToBeDeleted(kubeInformers.Core().V1().Secrets(), tlsSecretName) + deleteSecretFromTracker(tlsSecretName, kubeInformerClient) + waitForSecretToBeDeleted(kubeInformers.Core().V1().Secrets(), tlsSecretName) // The controller should be waiting for the load balancer's ingress to become available. r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 4) // no new actions while it is waiting for the load balancer's ingress + r.Len(kubeAPIClient.Actions(), 5) // no new actions while it is waiting for the load balancer's ingress requireTLSServerIsRunningWithoutCerts() // Update the ingress of the LB in the informer's client and run Sync again. @@ -1406,14 +1513,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { updateLoadBalancerServiceInTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: fakeIP}}, kubeInformerClient, "2") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "2") r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 5) - ca = requireTLSSecretWasCreated(kubeAPIClient.Actions()[4]) // created because the LB ingress became available + r.Len(kubeAPIClient.Actions(), 6) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[5], ca) // reuses the existing CA // Check that the server is running and that TLS certs that are being served are are for fakeIP. requireTLSServerIsRunning(ca, fakeIP, map[string]string{fakeIP + httpsPort: testServerAddr()}) // Simulate the informer cache's background update from its watch. - addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[4], kubeInformerClient, "2") - waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "2") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[5], kubeInformerClient, "3") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "3") // Now switch back to having the "endpoint" specified, so the load balancer is not needed anymore. configMapYAML := fmt.Sprintf("{mode: enabled, endpoint: %s}", localhostIP) @@ -1421,14 +1528,143 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2") r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 8) - requireLoadBalancerDeleted(kubeAPIClient.Actions()[5]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[6]) - requireTLSSecretWasCreated(kubeAPIClient.Actions()[7]) // recreated because the endpoint was updated + r.Len(kubeAPIClient.Actions(), 9) + requireLoadBalancerWasDeleted(kubeAPIClient.Actions()[6]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[7]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[8], ca) // recreated because the endpoint was updated, reused the old CA }) }) }) + when("sync is called more than once", func() { + it.Before(func() { + addNodeWithRoleToTracker("worker", kubeAPIClient) + }) + + it("only starts the impersonator once and only lists the cluster's nodes once", func() { + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + requireCASecretWasCreated(kubeAPIClient.Actions()[2]) + requireTLSServerIsRunningWithoutCerts() + + // Simulate the informer cache's background update from its watch. + addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + + r.NoError(runControllerSync()) + r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time + requireTLSServerIsRunningWithoutCerts() // still running + r.Len(kubeAPIClient.Actions(), 3) // no new API calls + }) + + it("creates certs from the ip address listed on the load balancer", func() { + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[2]) + requireTLSServerIsRunningWithoutCerts() + + // Simulate the informer cache's background update from its watch. + addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "0") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + + updateLoadBalancerServiceInTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") + + r.NoError(runControllerSync()) + r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time + r.Len(kubeAPIClient.Actions(), 4) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[3], ca) // uses the ca from last time + requireTLSServerIsRunning(ca, testServerAddr(), nil) // running with certs now + + // Simulate the informer cache's background update from its watch. + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[3], kubeInformerClient, "2") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "2") + + r.NoError(runControllerSync()) + r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started again + r.Len(kubeAPIClient.Actions(), 4) // no more actions + requireTLSServerIsRunning(ca, testServerAddr(), nil) // still running + }) + + it("creates certs from the hostname listed on the load balancer", func() { + hostname := "fake.example.com" + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[2]) + requireTLSServerIsRunningWithoutCerts() + + // Simulate the informer cache's background update from its watch. + addServiceFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "0") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0") + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[2], kubeInformerClient, "0") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "0") + + updateLoadBalancerServiceInTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP, Hostname: hostname}}, kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "1") + + r.NoError(runControllerSync()) + r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time + r.Len(kubeAPIClient.Actions(), 4) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[3], ca) // uses the ca from last time + requireTLSServerIsRunning(ca, hostname, map[string]string{hostname + httpsPort: testServerAddr()}) // running with certs now + + // Simulate the informer cache's background update from its watch. + addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[3], kubeInformerClient, "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + + r.NoError(runControllerSync()) + r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a third time + r.Len(kubeAPIClient.Actions(), 4) // no more actions + requireTLSServerIsRunning(ca, hostname, map[string]string{hostname + httpsPort: testServerAddr()}) // still running + }) + }) + + when("getting the control plane nodes returns an error, e.g. when there are no nodes", func() { + it("returns an error", func() { + startInformersAndController() + r.EqualError(runControllerSync(), "no nodes found") + requireTLSServerWasNeverStarted() + }) + }) + + when("the http handler factory function returns an error", func() { + it.Before(func() { + addNodeWithRoleToTracker("worker", kubeAPIClient) + httpHandlerFactoryFuncError = errors.New("some factory error") + }) + + it("returns an error", func() { + startInformersAndController() + r.EqualError(runControllerSync(), "some factory error") + requireTLSServerWasNeverStarted() + }) + }) + + when("the configmap is invalid", func() { + it.Before(func() { + addImpersonatorConfigMapToTracker(configMapResourceName, "not yaml", kubeInformerClient) + }) + + it("returns an error", func() { + startInformersAndController() + r.EqualError(runControllerSync(), "invalid impersonator configuration: decode yaml: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type impersonator.Config") + requireTLSServerWasNeverStarted() + }) + }) + when("there is an error creating the load balancer", func() { it.Before(func() { addNodeWithRoleToTracker("worker", kubeAPIClient) @@ -1439,7 +1675,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) it("exits with an error", func() { - r.EqualError(runControllerSync(), "could not create load balancer: error on create") + r.EqualError(runControllerSync(), "error on create") }) }) @@ -1448,17 +1684,61 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { addImpersonatorConfigMapToTracker(configMapResourceName, "{mode: enabled, endpoint: example.com}", kubeInformerClient) addNodeWithRoleToTracker("control-plane", kubeAPIClient) kubeAPIClient.PrependReactor("create", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, fmt.Errorf("error on create") + createdSecret := action.(coretesting.CreateAction).GetObject().(*corev1.Secret) + if createdSecret.Name == tlsSecretName { + return true, nil, fmt.Errorf("error on tls secret create") + } + return false, nil, nil }) }) it("starts the impersonator without certs and returns an error", func() { startInformersAndController() - r.EqualError(runControllerSync(), "error on create") + r.EqualError(runControllerSync(), "error on tls secret create") + requireTLSServerIsRunningWithoutCerts() + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) + }) + }) + + when("there is an error creating the CA secret", func() { + it.Before(func() { + addImpersonatorConfigMapToTracker(configMapResourceName, "{mode: enabled, endpoint: example.com}", kubeInformerClient) + addNodeWithRoleToTracker("control-plane", kubeAPIClient) + kubeAPIClient.PrependReactor("create", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { + createdSecret := action.(coretesting.CreateAction).GetObject().(*corev1.Secret) + if createdSecret.Name == caSecretName { + return true, nil, fmt.Errorf("error on ca secret create") + } + return false, nil, nil + }) + }) + + it("starts the impersonator without certs and returns an error", func() { + startInformersAndController() + r.EqualError(runControllerSync(), "error on ca secret create") requireTLSServerIsRunningWithoutCerts() r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + }) + }) + + when("the CA secret exists but is invalid while the TLS secret needs to be created", func() { + it.Before(func() { + addNodeWithRoleToTracker("control-plane", kubeAPIClient) + addImpersonatorConfigMapToTracker(configMapResourceName, "{mode: enabled, endpoint: example.com}", kubeInformerClient) + addSecretToTrackers(newEmptySecret(caSecretName), kubeAPIClient, kubeInformerClient) + }) + + it("starts the impersonator without certs and returns an error", func() { + startInformersAndController() + r.EqualError(runControllerSync(), "could not load CA: tls: failed to find any PEM data in certificate input") + requireTLSServerIsRunningWithoutCerts() + r.Len(kubeAPIClient.Actions(), 1) + requireNodesListed(kubeAPIClient.Actions()[0]) }) }) @@ -1467,9 +1747,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { addNodeWithRoleToTracker("control-plane", kubeAPIClient) addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeInformerClient) addLoadBalancerServiceToTracker(loadBalancerServiceName, kubeAPIClient) - tlsSecret := newStubTLSSecret(tlsSecretName) - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + addSecretToTrackers(newEmptySecret(tlsSecretName), kubeAPIClient, kubeInformerClient) startInformersAndController() kubeAPIClient.PrependReactor("delete", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, fmt.Errorf("error on delete") @@ -1481,12 +1759,12 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireTLSServerWasNeverStarted() r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - requireLoadBalancerDeleted(kubeAPIClient.Actions()[1]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[2]) + requireLoadBalancerWasDeleted(kubeAPIClient.Actions()[1]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[2]) }) }) - when("the PEM formatted data in the Secret is not a valid cert", func() { + when("the PEM formatted data in the TLS Secret is not a valid cert", func() { it.Before(func() { configMapYAML := fmt.Sprintf("{mode: enabled, endpoint: %s}", localhostIP) addImpersonatorConfigMapToTracker(configMapResourceName, configMapYAML, kubeInformerClient) @@ -1501,17 +1779,17 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { corev1.TLSCertKey: []byte("-----BEGIN CERTIFICATE-----\naGVsbG8gd29ybGQK\n-----END CERTIFICATE-----\n"), }, } - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + addSecretToTrackers(tlsSecret, kubeAPIClient, kubeInformerClient) }) it("deletes the invalid certs, creates new certs, and starts the impersonator", func() { startInformersAndController() r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 3) + r.Len(kubeAPIClient.Actions(), 4) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) // deleted the bad cert - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[2]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[2]) // deleted the bad cert + requireTLSSecretWasCreated(kubeAPIClient.Actions()[3], ca) requireTLSServerIsRunning(ca, testServerAddr(), nil) }) @@ -1526,21 +1804,25 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { startInformersAndController() r.EqualError(runControllerSync(), "PEM data represented an invalid cert, but got error while deleting it: error on delete") requireTLSServerIsRunningWithoutCerts() - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) // tried deleted the bad cert, which failed + requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[2]) // tried deleted the bad cert, which failed requireTLSServerIsRunningWithoutCerts() }) }) }) when("a tls secret already exists but it is not valid", func() { + var caCrt []byte it.Before(func() { addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled", kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) - tlsSecret := newStubTLSSecret(tlsSecretName) // secret exists but lacks certs - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + ca := newCA() + caSecret := newActualCASecret(ca, caSecretName) + caCrt = caSecret.Data["ca.crt"] + addSecretToTrackers(caSecret, kubeAPIClient, kubeInformerClient) + addSecretToTrackers(newEmptySecret(tlsSecretName), kubeAPIClient, kubeInformerClient) // secret exists but lacks certs addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeAPIClient) }) @@ -1550,9 +1832,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(runControllerSync()) r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) // deleted the bad cert - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[2]) - requireTLSServerIsRunning(ca, testServerAddr(), nil) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1]) // deleted the bad cert + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], caCrt) + requireTLSServerIsRunning(caCrt, testServerAddr(), nil) }) when("there is an error while the invalid cert is being deleted", func() { @@ -1568,20 +1850,24 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireTLSServerIsRunningWithoutCerts() r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) // tried deleted the bad cert, which failed + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1]) // tried deleted the bad cert, which failed requireTLSServerIsRunningWithoutCerts() }) }) }) when("a tls secret already exists but the private key is not valid", func() { + var caCrt []byte it.Before(func() { addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled", kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) - tlsSecret := newActualTLSSecret(tlsSecretName, localhostIP) + ca := newCA() + caSecret := newActualCASecret(ca, caSecretName) + caCrt = caSecret.Data["ca.crt"] + addSecretToTrackers(caSecret, kubeAPIClient, kubeInformerClient) + tlsSecret := newActualTLSSecret(ca, tlsSecretName, localhostIP) tlsSecret.Data["tls.key"] = nil - addSecretToTracker(tlsSecret, kubeAPIClient) - addSecretToTracker(tlsSecret, kubeInformerClient) + addSecretToTrackers(tlsSecret, kubeAPIClient, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeInformerClient) addLoadBalancerServiceWithIngressToTracker(loadBalancerServiceName, []corev1.LoadBalancerIngress{{IP: localhostIP}}, kubeAPIClient) }) @@ -1591,9 +1877,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(runControllerSync()) r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) // deleted the bad cert - ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[2]) - requireTLSServerIsRunning(ca, testServerAddr(), nil) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1]) // deleted the bad cert + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], caCrt) + requireTLSServerIsRunning(caCrt, testServerAddr(), nil) }) when("there is an error while the invalid cert is being deleted", func() { @@ -1609,7 +1895,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireTLSServerIsRunningWithoutCerts() r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) - requireTLSSecretDeleted(kubeAPIClient.Actions()[1]) // tried deleted the bad cert, which failed + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1]) // tried deleted the bad cert, which failed requireTLSServerIsRunningWithoutCerts() }) }) diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index b76deec4..ce63fc91 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -301,6 +301,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { controllerlib.WithInitialEvent, "pinniped-concierge-impersonation-proxy-load-balancer", // TODO this string should come from `c.NamesConfig` "pinniped-concierge-impersonation-proxy-tls-serving-certificate", // TODO this string should come from `c.NamesConfig` + "pinniped-concierge-impersonation-proxy-ca-certificate", // TODO this string should come from `c.NamesConfig` c.Labels, tls.Listen, func() (http.Handler, error) { diff --git a/test/integration/concierge_impersonation_proxy_test.go b/test/integration/concierge_impersonation_proxy_test.go index aec7bcdd..de113224 100644 --- a/test/integration/concierge_impersonation_proxy_test.go +++ b/test/integration/concierge_impersonation_proxy_test.go @@ -33,6 +33,7 @@ const ( // TODO don't hard code "pinniped-concierge-" in these strings. It should be constructed from the env app name. impersonationProxyConfigMapName = "pinniped-concierge-impersonation-proxy-config" impersonationProxyTLSSecretName = "pinniped-concierge-impersonation-proxy-tls-serving-certificate" //nolint:gosec // this is not a credential + impersonationProxyCASecretName = "pinniped-concierge-impersonation-proxy-ca-certificate" impersonationProxyLoadBalancerName = "pinniped-concierge-impersonation-proxy-load-balancer" ) @@ -160,21 +161,30 @@ func TestImpersonationProxy(t *testing.T) { }) } - // Wait for ca data to be available at the secret location. + // Check that the controller generated a CA. Get the CA data so we can use it as a client. + // TODO We should be getting the CA data from the CredentialIssuer's status instead, once that is implemented. var caSecret *corev1.Secret - require.Eventually(t, - func() bool { - caSecret, err = adminClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName, metav1.GetOptions{}) - return caSecret != nil && caSecret.Data["ca.crt"] != nil - }, 5*time.Minute, 250*time.Millisecond) + require.Eventually(t, func() bool { + caSecret, err = adminClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyCASecretName, metav1.GetOptions{}) + return err == nil && caSecret != nil && caSecret.Data["ca.crt"] != nil + }, 10*time.Second, 250*time.Millisecond) + caCertPEM := caSecret.Data["ca.crt"] + + // Check that the generated TLS cert Secret was created by the controller. + // This could take a while if we are waiting for the load balancer to get an IP or hostname assigned to it, and it + // should be fast when we are not waiting for a load balancer (e.g. on kind). + require.Eventually(t, func() bool { + _, err = adminClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName, metav1.GetOptions{}) + return err == nil + }, 5*time.Minute, 250*time.Millisecond) // Create an impersonation proxy client with that CA data to use for the rest of this test. // This client performs TLS checks, so it also provides test coverage that the impersonation proxy server is generating TLS certs correctly. var impersonationProxyClient *kubernetes.Clientset if env.HasCapability(library.HasExternalLoadBalancerProvider) { - impersonationProxyClient = impersonationProxyViaLoadBalancerClient(impersonationProxyLoadBalancerIngress, caSecret.Data["ca.crt"]) + impersonationProxyClient = impersonationProxyViaLoadBalancerClient(impersonationProxyLoadBalancerIngress, caCertPEM) } else { - impersonationProxyClient = impersonationProxyViaSquidClient(caSecret.Data["ca.crt"]) + impersonationProxyClient = impersonationProxyViaSquidClient(caCertPEM) } // Test that the user can perform basic actions through the client with their username and group membership @@ -381,7 +391,7 @@ func TestImpersonationProxy(t *testing.T) { // Check that the generated TLS cert Secret was deleted by the controller. require.Eventually(t, func() bool { - caSecret, err = adminClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName, metav1.GetOptions{}) + _, err = adminClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName, metav1.GetOptions{}) return k8serrors.IsNotFound(err) }, 10*time.Second, 250*time.Millisecond) }