diff --git a/cmd/debug-make-impersonation-token/main.go b/cmd/debug-make-impersonation-token/main.go index 07dddffe..972ab27e 100644 --- a/cmd/debug-make-impersonation-token/main.go +++ b/cmd/debug-make-impersonation-token/main.go @@ -37,5 +37,5 @@ func main() { if err != nil { panic(err) } - fmt.Println(base64.RawURLEncoding.EncodeToString(reqJSON)) + fmt.Println(base64.StdEncoding.EncodeToString(reqJSON)) } diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index 8f75b20c..508d17a4 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -11,6 +11,7 @@ import ( "fmt" "net" "net/http" + "sync" "time" v1 "k8s.io/api/core/v1" @@ -38,13 +39,17 @@ type impersonatorConfigController struct { k8sClient kubernetes.Interface configMapsInformer corev1informers.ConfigMapInformer servicesInformer corev1informers.ServiceInformer + secretsInformer corev1informers.SecretInformer generatedLoadBalancerServiceName string + tlsSecretName string labels map[string]string startTLSListenerFunc StartTLSListenerFunc httpHandlerFactory func() (http.Handler, error) server *http.Server hasControlPlaneNodes *bool + tlsCert *tls.Certificate + tlsCertMutex sync.RWMutex } type StartTLSListenerFunc func(network, listenAddress string, config *tls.Config) (net.Listener, error) @@ -55,9 +60,11 @@ func NewImpersonatorConfigController( k8sClient kubernetes.Interface, configMapsInformer corev1informers.ConfigMapInformer, servicesInformer corev1informers.ServiceInformer, + secretsInformer corev1informers.SecretInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, withInitialEvent pinnipedcontroller.WithInitialEventOptionFunc, generatedLoadBalancerServiceName string, + tlsSecretName string, labels map[string]string, startTLSListenerFunc StartTLSListenerFunc, httpHandlerFactory func() (http.Handler, error), @@ -71,7 +78,9 @@ func NewImpersonatorConfigController( k8sClient: k8sClient, configMapsInformer: configMapsInformer, servicesInformer: servicesInformer, + secretsInformer: secretsInformer, generatedLoadBalancerServiceName: generatedLoadBalancerServiceName, + tlsSecretName: tlsSecretName, labels: labels, startTLSListenerFunc: startTLSListenerFunc, httpHandlerFactory: httpHandlerFactory, @@ -87,6 +96,11 @@ func NewImpersonatorConfigController( pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(generatedLoadBalancerServiceName, namespace), controllerlib.InformerOption{}, ), + withInformer( + secretsInformer, + pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(tlsSecretName, namespace), + controllerlib.InformerOption{}, + ), // Be sure to run once even if the ConfigMap that the informer is watching doesn't exist. withInitialEvent(controllerlib.Key{ Namespace: namespace, @@ -155,21 +169,31 @@ func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error { } } + if c.shouldHaveTLSSecret(config) { + if err = c.ensureTLSSecretIsCreated(ctx.Context, config); err != nil { + // return err // TODO + } + } else { + if err = c.ensureTLSSecretIsRemoved(ctx.Context); err != nil { + return err + } + } + plog.Debug("Successfully finished impersonatorConfigController Sync") return nil } +func (c *impersonatorConfigController) shouldHaveTLSSecret(config *impersonator.Config) bool { + return c.shouldHaveImpersonator(config) +} + func (c *impersonatorConfigController) shouldHaveImpersonator(config *impersonator.Config) bool { return (config.Mode == impersonator.ModeAuto && !*c.hasControlPlaneNodes) || config.Mode == impersonator.ModeEnabled } func (c *impersonatorConfigController) shouldHaveLoadBalancer(config *impersonator.Config) bool { - // start the load balancer only if: - // - the impersonator is running - // - the cluster is cloud hosted - // - there is no endpoint specified in the config - return c.server != nil && !*c.hasControlPlaneNodes && config.Endpoint == "" + return c.shouldHaveImpersonator(config) && config.Endpoint == "" } func (c *impersonatorConfigController) ensureImpersonatorIsStopped() error { @@ -189,15 +213,6 @@ func (c *impersonatorConfigController) ensureImpersonatorIsStarted() error { return nil } - impersonationCA, err := certauthority.New(pkix.Name{CommonName: "test CA"}, 24*time.Hour) - if err != nil { - return fmt.Errorf("could not create impersonation CA: %w", err) - } - impersonationCert, err := impersonationCA.Issue(pkix.Name{}, []string{"impersonation-proxy"}, nil, 24*time.Hour) - if err != nil { - return fmt.Errorf("could not create impersonation cert: %w", err) - } - handler, err := c.httpHandlerFactory() if err != nil { return err @@ -206,7 +221,7 @@ func (c *impersonatorConfigController) ensureImpersonatorIsStarted() error { 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 impersonationCert, nil + return c.getTLSCert(), nil }, }) if err != nil { @@ -239,6 +254,18 @@ func (c *impersonatorConfigController) isLoadBalancerRunning() (bool, error) { return true, nil } +func (c *impersonatorConfigController) tlsSecretExists() (bool, *v1.Secret, error) { + secret, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(c.tlsSecretName) + notFound := k8serrors.IsNotFound(err) + if notFound { + return false, nil, nil + } + if err != nil { + return false, nil, err + } + return true, secret, nil +} + func (c *impersonatorConfigController) ensureLoadBalancerIsStopped(ctx context.Context) error { running, err := c.isLoadBalancerRunning() if err != nil { @@ -295,3 +322,99 @@ func (c *impersonatorConfigController) ensureLoadBalancerIsStarted(ctx context.C } return nil } + +func (c *impersonatorConfigController) ensureTLSSecretIsCreated(ctx context.Context, config *impersonator.Config) error { + tlsSecretExists, tlsSecret, err := c.tlsSecretExists() + if err != nil { + return err + } + if tlsSecretExists { + certPEM := tlsSecret.Data[v1.TLSCertKey] + keyPEM := tlsSecret.Data[v1.TLSPrivateKeyKey] + tlsCert, _ := tls.X509KeyPair(certPEM, keyPEM) + // TODO handle err, like when the Secret did not contain the fields that we expected + c.setTLSCert(&tlsCert) + return nil + } + impersonationCA, err := certauthority.New(pkix.Name{CommonName: "test CA"}, 24*time.Hour) // TODO change the length of this + if err != nil { + return fmt.Errorf("could not create impersonation CA: %w", err) + } + var ips []net.IP + if config.Endpoint == "" { // TODO are there other cases where we need to do this? + lb, err := c.servicesInformer.Lister().Services(c.namespace).Get(c.generatedLoadBalancerServiceName) + notFound := k8serrors.IsNotFound(err) + if notFound { + return nil + } + if err != nil { + return err + } + ingresses := lb.Status.LoadBalancer.Ingress + if len(ingresses) == 0 { + return nil + } + ip := ingresses[0].IP // TODO multiple ips + ips = []net.IP{net.ParseIP(ip)} + // check with informer to get the ip address of the load balancer if its available + // if not, return + } else { + ips = []net.IP{net.ParseIP(config.Endpoint)} + } + impersonationCert, err := impersonationCA.Issue(pkix.Name{}, nil, ips, 24*time.Hour) // TODO change the length of this + if err != nil { + return fmt.Errorf("could not create impersonation cert: %w", err) + } + + c.setTLSCert(impersonationCert) + + certPEM, keyPEM, err := certauthority.ToPEM(impersonationCert) + // TODO error handling? + + // TODO handle error on create + c.k8sClient.CoreV1().Secrets(c.namespace).Create(ctx, &v1.Secret{ + Type: v1.SecretTypeTLS, + ObjectMeta: metav1.ObjectMeta{ + Name: c.tlsSecretName, + Namespace: c.namespace, + Labels: c.labels, + }, + Data: map[string][]byte{ + "ca.crt": impersonationCA.Bundle(), + v1.TLSPrivateKeyKey: keyPEM, + v1.TLSCertKey: certPEM, + }, + }, metav1.CreateOptions{}) + return nil +} + +func (c *impersonatorConfigController) ensureTLSSecretIsRemoved(ctx context.Context) error { + tlsSecretExists, _, err := c.tlsSecretExists() + if err != nil { + return err + } + if !tlsSecretExists { + return nil + } + plog.Info("Deleting TLS certificates for impersonation proxy", + "service", c.tlsSecretName, + "namespace", c.namespace) + err = c.k8sClient.CoreV1().Secrets(c.namespace).Delete(ctx, c.tlsSecretName, metav1.DeleteOptions{}) + if err != nil { + return err + } + + return nil +} + +func (c *impersonatorConfigController) setTLSCert(cert *tls.Certificate) { + c.tlsCertMutex.Lock() + defer c.tlsCertMutex.Unlock() + c.tlsCert = cert +} + +func (c *impersonatorConfigController) getTLSCert() *tls.Certificate { + c.tlsCertMutex.RLock() + defer c.tlsCertMutex.RUnlock() + return c.tlsCert +} diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index d441eabb..b6fee18e 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -6,6 +6,8 @@ package impersonatorconfig import ( "context" "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" "errors" "fmt" "io/ioutil" @@ -29,6 +31,7 @@ import ( coretesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" + "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/testutil" @@ -61,12 +64,14 @@ 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" var r *require.Assertions var observableWithInformerOption *testutil.ObservableWithInformerOption var observableWithInitialEventOption *testutil.ObservableWithInitialEventOption var configMapsInformerFilter controllerlib.Filter var servicesInformerFilter controllerlib.Filter + var secretsInformerFilter controllerlib.Filter it.Before(func() { r = require.New(t) @@ -75,6 +80,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { sharedInformerFactory := kubeinformers.NewSharedInformerFactory(nil, 0) configMapsInformer := sharedInformerFactory.Core().V1().ConfigMaps() servicesInformer := sharedInformerFactory.Core().V1().Services() + secretsInformer := sharedInformerFactory.Core().V1().Secrets() _ = NewImpersonatorConfigController( installedInNamespace, @@ -82,15 +88,18 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { nil, configMapsInformer, servicesInformer, + secretsInformer, observableWithInformerOption.WithInformer, observableWithInitialEventOption.WithInitialEvent, generatedLoadBalancerServiceName, + tlsSecretName, nil, nil, nil, ) configMapsInformerFilter = observableWithInformerOption.GetFilterForInformer(configMapsInformer) servicesInformerFilter = observableWithInformerOption.GetFilterForInformer(servicesInformer) + secretsInformerFilter = observableWithInformerOption.GetFilterForInformer(secretsInformer) }) when("watching ConfigMap objects", func() { @@ -189,6 +198,54 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { }) }) + when("watching Secret objects", func() { + var subject controllerlib.Filter + var target, wrongNamespace, 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"}} + 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() { + 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)) + }) + }) + + 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)) + }) + }) + + when("a Secret with a different name changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(wrongName)) + r.False(subject.Update(wrongName, unrelated)) + r.False(subject.Update(unrelated, wrongName)) + r.False(subject.Delete(wrongName)) + }) + }) + + when("a Secret with a different name and a different namespace changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(unrelated)) + r.False(subject.Update(unrelated, unrelated)) + r.False(subject.Delete(unrelated)) + }) + }) + }) + when("starting up", func() { it("asks for an initial event because the ConfigMap may not exist yet and it needs to run anyway", func() { r.Equal(&controllerlib.Key{ @@ -205,6 +262,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { const installedInNamespace = "some-namespace" const configMapResourceName = "some-configmap-resource-name" const generatedLoadBalancerServiceName = "some-service-resource-name" + const tlsSecretName = "some-secret-name" var labels = map[string]string{"app": "app-name", "other-key": "other-value"} var r *require.Assertions @@ -232,7 +290,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } var err error //nolint: gosec // Intentionally binding to all network interfaces. - startedTLSListener, err = tls.Listen(network, ":0", config) // automatically choose the port for unit tests + startedTLSListener, err = tls.Listen(network, "127.0.0.1:0", config) // automatically choose the port for unit tests r.NoError(err) return &tlsListenerWrapper{listener: startedTLSListener, closeError: startTLSListenerUponCloseError}, nil } @@ -248,11 +306,21 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } } - var requireTLSServerIsRunning = func() { + var requireTLSServerIsRunning = func(caCrt []byte) { r.Greater(startTLSListenerFuncWasCalled, 0) - tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // TODO once we're using certs, do not skip verify + var tr *http.Transport + if caCrt == nil { + tr = &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec + } + } else { + rootCAs := x509.NewCertPool() + rootCAs.AppendCertsFromPEM(caCrt) + + tr = &http.Transport{ + TLSClientConfig: &tls.Config{RootCAs: rootCAs}, + } } client := &http.Client{Transport: tr} url := "https://" + startedTLSListener.Addr().String() @@ -268,6 +336,22 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Equal("hello world", string(body)) } + var requireTLSServerIsRunningWithoutCerts = func() { + r.Greater(startTLSListenerFuncWasCalled, 0) + + tr := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec + } + + client := &http.Client{Transport: tr} + url := "https://" + startedTLSListener.Addr().String() + req, err := http.NewRequestWithContext(context.Background(), "GET", url, nil) + r.NoError(err) + _, err = client.Do(req) + r.Error(err) + r.Regexp("Get .*: remote error: tls: unrecognized name", err.Error()) + } + var requireTLSServerIsNoLongerRunning = func() { r.Greater(startTLSListenerFuncWasCalled, 0) _, err := tls.Dial( @@ -276,7 +360,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // TODO once we're using certs, do not skip verify ) r.Error(err) - r.Regexp(`dial tcp \[::\]:[0-9]+: connect: connection refused`, err.Error()) + r.Regexp(`dial tcp .*: connect: connection refused`, err.Error()) } var requireTLSServerWasNeverStarted = func() { @@ -306,9 +390,11 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { kubeAPIClient, kubeInformers.Core().V1().ConfigMaps(), kubeInformers.Core().V1().Services(), + kubeInformers.Core().V1().Secrets(), controllerlib.WithInformer, controllerlib.WithInitialEvent, generatedLoadBalancerServiceName, + tlsSecretName, labels, startTLSListenerFunc, func() (http.Handler, error) { @@ -371,6 +457,44 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { )) } + var createStubTLSSecret = func(resourceName string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: installedInNamespace, + // Note that this seems to be ignored by the informer during initial creation, so actually + // the informer will see this as resource version "". Leaving it here to express the intent + // that the initial version is version 0. + ResourceVersion: "0", + }, + Data: map[string][]byte{}, + } + } + + var createActualTLSSecret = func(resourceName string) *corev1.Secret { + impersonationCA, err := certauthority.New(pkix.Name{CommonName: "test CA"}, 24*time.Hour) + r.NoError(err) + impersonationCert, err := impersonationCA.Issue(pkix.Name{}, nil, []net.IP{net.ParseIP("127.0.0.1")}, 24*time.Hour) + r.NoError(err) + certPEM, keyPEM, err := certauthority.ToPEM(impersonationCert) + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: installedInNamespace, + // Note that this seems to be ignored by the informer during initial creation, so actually + // the informer will see this as resource version "". Leaving it here to express the intent + // that the initial version is version 0. + ResourceVersion: "0", + }, + Data: map[string][]byte{ + "ca.crt": impersonationCA.Bundle(), + corev1.TLSPrivateKeyKey: keyPEM, + corev1.TLSCertKey: certPEM, + }, + } + } + var addLoadBalancerServiceToTracker = func(resourceName string, client *kubernetesfake.Clientset) { loadBalancerService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -388,6 +512,30 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(client.Tracker().Add(loadBalancerService)) } + var addLoadBalancerServiceWithIPToTracker = func(resourceName string, client *kubernetesfake.Clientset) { + loadBalancerService := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: installedInNamespace, + // Note that this seems to be ignored by the informer during initial creation, so actually + // the informer will see this as resource version "". Leaving it here to express the intent + // that the initial version is version 0. + ResourceVersion: "0", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + {IP: "127.0.0.1"}, + }, + }, + }, + } + r.NoError(client.Tracker().Add(loadBalancerService)) + } + var deleteLoadBalancerServiceFromTracker = func(resourceName string, client *kubernetesfake.Clientset) { r.NoError(client.Tracker().Delete( schema.GroupVersionResource{Version: "v1", Resource: "services"}, @@ -419,9 +567,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } var requireLoadBalancerWasCreated = func(action coretesting.Action) { - createLoadBalancerAction := action.(coretesting.CreateAction) - r.Equal("create", createLoadBalancerAction.GetVerb()) - createdLoadBalancerService := createLoadBalancerAction.GetObject().(*corev1.Service) + createAction := action.(coretesting.CreateAction) + r.Equal("create", createAction.GetVerb()) + createdLoadBalancerService := createAction.GetObject().(*corev1.Service) r.Equal(generatedLoadBalancerServiceName, createdLoadBalancerService.Name) r.Equal(installedInNamespace, createdLoadBalancerService.Namespace) r.Equal(corev1.ServiceTypeLoadBalancer, createdLoadBalancerService.Spec.Type) @@ -430,9 +578,32 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } var requireLoadBalancerDeleted = func(action coretesting.Action) { - deleteLoadBalancerAction := action.(coretesting.DeleteAction) - r.Equal("delete", deleteLoadBalancerAction.GetVerb()) - r.Equal(generatedLoadBalancerServiceName, deleteLoadBalancerAction.GetName()) + deleteAction := action.(coretesting.DeleteAction) + r.Equal("delete", deleteAction.GetVerb()) + r.Equal(generatedLoadBalancerServiceName, deleteAction.GetName()) + r.Equal("services", deleteAction.GetResource().Resource) + } + + var requireTLSSecretDeleted = func(action coretesting.Action) { + deleteAction := action.(coretesting.DeleteAction) + 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) + 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, 3) + r.NotNil(createdSecret.Data["ca.crt"]) + r.NotNil(createdSecret.Data[corev1.TLSPrivateKeyKey]) + r.NotNil(createdSecret.Data[corev1.TLSCertKey]) + return createdSecret.Data["ca.crt"] } it.Before(func() { @@ -471,20 +642,24 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) }) - when("there are visible control plane nodes and a loadbalancer", func() { + when("there are visible control plane nodes and a loadbalancer and a tls Secret", func() { it.Before(func() { addNodeWithRoleToTracker("control-plane") addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient) addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient) + tlsSecret := createStubTLSSecret(tlsSecretName) + r.NoError(kubeAPIClient.Tracker().Add(tlsSecret)) + r.NoError(kubeInformerClient.Tracker().Add(tlsSecret)) }) - it("does not start the impersonator, deletes the loadbalancer", func() { + it("does not start the impersonator, deletes the loadbalancer, deletes the Secret", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) requireTLSServerWasNeverStarted() - r.Len(kubeAPIClient.Actions(), 2) + r.Len(kubeAPIClient.Actions(), 3) requireNodesListed(kubeAPIClient.Actions()[0]) requireLoadBalancerDeleted(kubeAPIClient.Actions()[1]) + requireTLSSecretDeleted(kubeAPIClient.Actions()[2]) }) }) @@ -495,8 +670,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(controllerlib.TestSync(t, subject, *syncContext)) }) - it("automatically starts the impersonator", func() { - requireTLSServerIsRunning() + it("starts the impersonator without tls certs", func() { + requireTLSServerIsRunningWithoutCerts() }) it("starts the load balancer automatically", func() { @@ -506,7 +681,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) }) - when("there are not visible control plane nodes and a load balancer already exists", func() { + when("there are not visible control plane nodes and a load balancer already exists without an IP", func() { it.Before(func() { addNodeWithRoleToTracker("worker") addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient) @@ -515,8 +690,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(controllerlib.TestSync(t, subject, *syncContext)) }) - it("automatically starts the impersonator", func() { - requireTLSServerIsRunning() + it("starts the impersonator without tls certs", func() { + requireTLSServerIsRunningWithoutCerts() }) it("does not start the load balancer automatically", func() { @@ -537,6 +712,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + requireTLSServerIsRunningWithoutCerts() // update manually because the kubeAPIClient isn't connected to the informer in the tests addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient) @@ -544,9 +720,39 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(controllerlib.TestSync(t, subject, *syncContext)) r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time - requireTLSServerIsRunning() // still running + 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(controllerlib.TestSync(t, subject, *syncContext)) + r.Len(kubeAPIClient.Actions(), 2) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) + requireTLSServerIsRunningWithoutCerts() + + // update manually because the kubeAPIClient isn't connected to the informer in the tests + addLoadBalancerServiceWithIPToTracker(generatedLoadBalancerServiceName, kubeInformerClient) + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0") + + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time + r.Len(kubeAPIClient.Actions(), 3) + ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[2]) + requireTLSServerIsRunning(ca) // running with certs now + + // update manually because the kubeAPIClient isn't connected to the informer in the tests + createdSecret := kubeAPIClient.Actions()[2].(coretesting.CreateAction).GetObject().(*corev1.Secret) + createdSecret.ResourceVersion = "1" + r.NoError(kubeInformerClient.Tracker().Add(createdSecret)) + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1") + + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a third time + r.Len(kubeAPIClient.Actions(), 3) // no more actions + requireTLSServerIsRunning(ca) // still running + }) }) when("getting the control plane nodes returns an error, e.g. when there are no nodes", func() { @@ -587,8 +793,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it.Before(func() { addImpersonatorConfigMapToTracker(configMapResourceName, here.Doc(` mode: auto - endpoint: https://proxy.example.com:8443/ - `), + endpoint: 127.0.0.1 + `), // TODO what to do about ports + // TODO IP address and hostname should work ) }) @@ -614,9 +821,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the impersonator according to the settings in the ConfigMap", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() + r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) - r.Len(kubeAPIClient.Actions(), 1) + ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSServerIsRunning(ca) }) }) }) @@ -637,7 +845,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) when("the configuration is enabled mode", func() { - when("there are control plane nodes", func() { + when("no load balancer", func() { it.Before(func() { addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled") addNodeWithRoleToTracker("control-plane") @@ -646,90 +854,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("starts the impersonator", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() - }) - - it("returns an error when the tls listener fails to start", func() { - startTLSListenerFuncError = errors.New("tls error") - startInformersAndController() - r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error") - }) - - it("does not start the load balancer", func() { - startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - r.Len(kubeAPIClient.Actions(), 1) - requireNodesListed(kubeAPIClient.Actions()[0]) - }) - }) - - when("there are no control plane nodes but a loadbalancer already exists", func() { - it.Before(func() { - addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled") - addNodeWithRoleToTracker("worker") - addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient) - addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient) - }) - - it("starts the impersonator", func() { - startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() - }) - - it("returns an error when the tls listener fails to start", func() { - startTLSListenerFuncError = errors.New("tls error") - startInformersAndController() - r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error") - }) - - it("does not start the load balancer", func() { - startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - r.Len(kubeAPIClient.Actions(), 1) - requireNodesListed(kubeAPIClient.Actions()[0]) - }) - }) - - when("there are control plane nodes and a loadbalancer already exists", func() { - it.Before(func() { - addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled") - addNodeWithRoleToTracker("control-plane") - addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient) - addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient) - }) - - it("starts the impersonator", func() { - startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() - }) - - it("returns an error when the tls listener fails to start", func() { - startTLSListenerFuncError = errors.New("tls error") - startInformersAndController() - r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error") - }) - - it("stops the load balancer", func() { - startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - r.Len(kubeAPIClient.Actions(), 2) - requireNodesListed(kubeAPIClient.Actions()[0]) - requireLoadBalancerDeleted(kubeAPIClient.Actions()[1]) - }) - }) - - when("there are no control plane nodes and there is no load balancer", func() { - it.Before(func() { - addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled") - addNodeWithRoleToTracker("worker") - }) - - it("starts the impersonator", func() { - startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() + requireTLSServerIsRunningWithoutCerts() }) it("returns an error when the tls listener fails to start", func() { @@ -746,6 +871,55 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) }) }) + + when("a loadbalancer already exists", func() { + it.Before(func() { + addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled") + addNodeWithRoleToTracker("worker") + addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient) + addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient) + }) + + it("starts the impersonator", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + requireTLSServerIsRunningWithoutCerts() + }) + + it("returns an error when the tls listener fails to start", func() { + startTLSListenerFuncError = errors.New("tls error") + startInformersAndController() + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error") + }) + + it("does not start the load balancer", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + r.Len(kubeAPIClient.Actions(), 1) + requireNodesListed(kubeAPIClient.Actions()[0]) + }) + }) + + when("a load balancer and a secret already exist", func() { + var tlsSecret *corev1.Secret + it.Before(func() { + addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled") + addNodeWithRoleToTracker("worker") + tlsSecret = createActualTLSSecret(tlsSecretName) + r.NoError(kubeAPIClient.Tracker().Add(tlsSecret)) + r.NoError(kubeInformerClient.Tracker().Add(tlsSecret)) + addLoadBalancerServiceWithIPToTracker(generatedLoadBalancerServiceName, kubeInformerClient) + addLoadBalancerServiceWithIPToTracker(generatedLoadBalancerServiceName, kubeAPIClient) + }) + + it("starts the impersonator with the existing tls certs, does not start loadbalancer or make tls secret", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + r.Len(kubeAPIClient.Actions(), 1) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireTLSServerIsRunning(tlsSecret.Data["ca.crt"]) + }) + }) }) when("the configuration switches from enabled to disabled mode", func() { @@ -758,7 +932,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() + requireTLSServerIsRunningWithoutCerts() r.Len(kubeAPIClient.Actions(), 2) requireNodesListed(kubeAPIClient.Actions()[0]) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) @@ -782,7 +956,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2") r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() + requireTLSServerIsRunningWithoutCerts() r.Len(kubeAPIClient.Actions(), 4) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[3]) }) @@ -795,7 +969,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("returns the error from the sync function", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - requireTLSServerIsRunning() + requireTLSServerIsRunningWithoutCerts() updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: disabled", "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1") @@ -860,5 +1034,29 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "could not create load balancer: error on create") }) }) + + when("there is an error deleting the tls secret", func() { + it.Before(func() { + addNodeWithRoleToTracker("control-plane") + addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient) + addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient) + tlsSecret := createStubTLSSecret(tlsSecretName) + r.NoError(kubeAPIClient.Tracker().Add(tlsSecret)) + r.NoError(kubeInformerClient.Tracker().Add(tlsSecret)) + startInformersAndController() + kubeAPIClient.PrependReactor("delete", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, fmt.Errorf("error on delete") + }) + }) + + it("does not start the impersonator, deletes the loadbalancer, returns an error", func() { + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "error on delete") + requireTLSServerWasNeverStarted() + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireLoadBalancerDeleted(kubeAPIClient.Actions()[1]) + requireTLSSecretDeleted(kubeAPIClient.Actions()[2]) + }) + }) }, spec.Parallel(), spec.Report(report.Terminal{})) } diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 73c61b9c..b76deec4 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -296,9 +296,11 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { client.Kubernetes, informers.installationNamespaceK8s.Core().V1().ConfigMaps(), informers.installationNamespaceK8s.Core().V1().Services(), + informers.installationNamespaceK8s.Core().V1().Secrets(), controllerlib.WithInformer, controllerlib.WithInitialEvent, - "pinniped-concierge-impersonation-proxy-load-balancer", // TODO this string should come from `c.NamesConfig` + "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` c.Labels, tls.Listen, func() (http.Handler, error) {