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) }