diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index cfa4f5ca..655856ef 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -4,6 +4,7 @@ package impersonatorconfig import ( + "context" "crypto/tls" "crypto/x509/pkix" "errors" @@ -12,7 +13,10 @@ import ( "net/http" "time" + v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" @@ -34,10 +38,12 @@ type impersonatorConfigController struct { k8sClient kubernetes.Interface configMapsInformer corev1informers.ConfigMapInformer generatedLoadBalancerServiceName string + labels map[string]string startTLSListenerFunc StartTLSListenerFunc httpHandlerFactory func() (http.Handler, error) server *http.Server + loadBalancer *v1.Service hasControlPlaneNodes *bool } @@ -51,6 +57,7 @@ func NewImpersonatorConfigController( withInformer pinnipedcontroller.WithInformerOptionFunc, withInitialEvent pinnipedcontroller.WithInitialEventOptionFunc, generatedLoadBalancerServiceName string, + labels map[string]string, startTLSListenerFunc StartTLSListenerFunc, httpHandlerFactory func() (http.Handler, error), ) controllerlib.Controller { @@ -63,6 +70,7 @@ func NewImpersonatorConfigController( k8sClient: k8sClient, configMapsInformer: configMapsInformer, generatedLoadBalancerServiceName: generatedLoadBalancerServiceName, + labels: labels, startTLSListenerFunc: startTLSListenerFunc, httpHandlerFactory: httpHandlerFactory, }, @@ -130,38 +138,19 @@ func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error { } } - // TODO when the proxy is going to run, and the endpoint goes from being not specified to being specified, then the LoadBalancer is deleted - // TODO when the proxy is going to run, and when the endpoint goes from being specified to being not specified, then the LoadBalancer is created - // TODO when auto mode decides that the proxy should be disabled, then it also does not create the LoadBalancer (or it deletes it) - - // client, err := kubeclient.New() - // if err != nil { - // plog.WarningErr("could not create client", err) - // } else { - // appNameLabel := cfg.Labels["app"] - // loadBalancer := v1.Service{ - // Spec: v1.ServiceSpec{ - // Type: "LoadBalancer", - // Ports: []v1.ServicePort{ - // { - // TargetPort: intstr.FromInt(8444), - // Port: 443, - // Protocol: v1.ProtocolTCP, - // }, - // }, - // Selector: map[string]string{"app": appNameLabel}, - // }, - // ObjectMeta: metav1.ObjectMeta{ - // Name: "impersonation-proxy-load-balancer", - // Namespace: podInfo.Namespace, - // Labels: cfg.Labels, - // }, - // } - // _, err = client.Kubernetes.CoreV1().Services(podInfo.Namespace).Create(ctx, &loadBalancer, metav1.CreateOptions{}) - // if err != nil { - // plog.WarningErr("could not create load balancer", err) - // } - // } + // start the load balancer only if: + // - the impersonator is running + // - the cluster is cloud hosted + // - there is no endpoint specified in the config + if c.server != nil && !*c.hasControlPlaneNodes && config.Endpoint == "" { + if err = c.startLoadBalancer(ctx.Context); err != nil { + return err + } + } else { + if err = c.stopLoadBalancer(ctx.Context); err != nil { + return err + } + } return nil } @@ -220,3 +209,45 @@ func (c *impersonatorConfigController) startImpersonator() error { }() return nil } + +func (c *impersonatorConfigController) stopLoadBalancer(ctx context.Context) error { + if c.loadBalancer != nil { + err := c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedLoadBalancerServiceName, metav1.DeleteOptions{}) + if err != nil { + return err + } + } + return nil +} + +func (c *impersonatorConfigController) startLoadBalancer(ctx context.Context) error { + if c.loadBalancer != nil { + return nil + } + + appNameLabel := c.labels["app"] // TODO what if this doesn't exist + loadBalancer := v1.Service{ + Spec: v1.ServiceSpec{ + Type: "LoadBalancer", + Ports: []v1.ServicePort{ + { + TargetPort: intstr.FromInt(8444), + Port: 443, + Protocol: v1.ProtocolTCP, + }, + }, + Selector: map[string]string{"app": appNameLabel}, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: c.generatedLoadBalancerServiceName, + Namespace: c.namespace, + Labels: c.labels, + }, + } + createdLoadBalancer, 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) + } + c.loadBalancer = createdLoadBalancer + return nil +} diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 65d35546..c4037122 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" @@ -79,6 +80,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { generatedLoadBalancerServiceName, nil, nil, + nil, ) configMapsInformerFilter = observableWithInformerOption.GetFilterForInformer(configMapsInformer) }) @@ -147,6 +149,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { const installedInNamespace = "some-namespace" const configMapResourceName = "some-configmap-resource-name" const generatedLoadBalancerServiceName = "some-service-resource-name" + var labels = map[string]string{"app": "app-name", "other-key": "other-value"} var r *require.Assertions @@ -242,6 +245,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { controllerlib.WithInformer, controllerlib.WithInitialEvent, generatedLoadBalancerServiceName, + labels, startTLSListenerFunc, func() (http.Handler, error) { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { @@ -351,13 +355,27 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("there are not visible control plane nodes", func() { it.Before(func() { addNodeWithRoleToTracker("worker") + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) }) it("automatically starts the impersonator", func() { - startInformersAndController() - r.NoError(controllerlib.TestSync(t, subject, *syncContext)) requireTLSServerIsRunning() }) + + it("starts the load balancer automatically", func() { + // action 0: list nodes + // action 1: create load balancer + // that should be all + createLoadBalancerAction := kubeAPIClient.Actions()[1].(coretesting.CreateAction) + r.Equal("create", createLoadBalancerAction.GetVerb()) + createdLoadBalancerService := createLoadBalancerAction.GetObject().(*corev1.Service) + r.Equal(generatedLoadBalancerServiceName, createdLoadBalancerService.Name) + r.Equal(installedInNamespace, createdLoadBalancerService.Namespace) + r.Equal(corev1.ServiceTypeLoadBalancer, createdLoadBalancerService.Spec.Type) + r.Equal("app-name", createdLoadBalancerService.Spec.Selector["app"]) + r.Equal(labels, createdLoadBalancerService.Labels) + }) }) }) @@ -369,21 +387,20 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("only starts the impersonator once and only lists the cluster's nodes once", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + r.Equal(2, len(kubeAPIClient.Actions())) r.Equal( - []coretesting.Action{ - coretesting.NewListAction( - schema.GroupVersionResource{Version: "v1", Resource: "nodes"}, - schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"}, - "", - metav1.ListOptions{}), - }, - kubeAPIClient.Actions(), + coretesting.NewListAction( + schema.GroupVersionResource{Version: "v1", Resource: "nodes"}, + schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"}, + "", + metav1.ListOptions{}), + kubeAPIClient.Actions()[0], ) r.NoError(controllerlib.TestSync(t, subject, *syncContext)) r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time requireTLSServerIsRunning() // still running - r.Equal(1, len(kubeAPIClient.Actions())) // no new API calls + r.Equal(2, len(kubeAPIClient.Actions())) // no new API calls }) }) @@ -451,6 +468,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) requireTLSServerIsRunning() + r.Equal(1, len(kubeAPIClient.Actions())) }) }) }) @@ -485,25 +503,48 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { startInformersAndController() r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error") }) + + it("does not start the load balancer if there are control plane nodes", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + // action 0: list nodes + // that should be all + r.Equal(1, len(kubeAPIClient.Actions())) + }) }) when("the configuration switches from enabled to disabled mode", func() { it.Before(func() { addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled") - addNodeWithRoleToTracker("control-plane") + addNodeWithRoleToTracker("worker") }) - it("starts the impersonator, then shuts it down, then starts it again", func() { + it("starts the impersonator and loadbalancer, then shuts it down, then starts it again", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) requireTLSServerIsRunning() + // TODO extract this + // action 0: list nodes + // action 1: create load balancer + // that should be all + createLoadBalancerAction := kubeAPIClient.Actions()[1].(coretesting.CreateAction) + r.Equal("create", createLoadBalancerAction.GetVerb()) + createdLoadBalancerService := createLoadBalancerAction.GetObject().(*corev1.Service) + r.Equal(generatedLoadBalancerServiceName, createdLoadBalancerService.Name) + r.Equal(installedInNamespace, createdLoadBalancerService.Namespace) + r.Equal(corev1.ServiceTypeLoadBalancer, createdLoadBalancerService.Spec.Type) + r.Equal("app-name", createdLoadBalancerService.Spec.Selector["app"]) + r.Equal(labels, createdLoadBalancerService.Labels) updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: disabled", "1") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1") r.NoError(controllerlib.TestSync(t, subject, *syncContext)) requireTLSServerIsNoLongerRunning() + deleteLoadBalancerAction := kubeAPIClient.Actions()[2].(coretesting.DeleteAction) + r.Equal("delete", deleteLoadBalancerAction.GetVerb()) + r.Equal(generatedLoadBalancerServiceName, deleteLoadBalancerAction.GetName()) updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: enabled", "2") waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2") @@ -530,6 +571,59 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) }) }) + + when("the endpoint switches from not specified, to specified, to not specified", func() { + it.Before(func() { + addImpersonatorConfigMapToTracker(configMapResourceName, here.Doc(` + mode: enabled + endpoint: https://proxy.example.com:8443/ + `)) + addNodeWithRoleToTracker("worker") + }) + + it("starts, stops, restarts the loadbalancer", func() { + startInformersAndController() + + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + + loadBalancer, err := kubeAPIClient.CoreV1().Services(installedInNamespace).Get(context.Background(), generatedLoadBalancerServiceName, metav1.GetOptions{}) + r.Nil(loadBalancer) + r.EqualError(err, "services \"some-service-resource-name\" not found") + + updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: enabled", "1") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1") + + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + loadBalancer, err = kubeAPIClient.CoreV1().Services(installedInNamespace).Get(context.Background(), generatedLoadBalancerServiceName, metav1.GetOptions{}) + r.NotNil(loadBalancer) + r.NoError(err, "services \"some-service-resource-name\" not found") + + updateImpersonatorConfigMapInTracker(configMapResourceName, here.Doc(` + mode: enabled + endpoint: https://proxy.example.com:8443/ + `), "2") + waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2") + + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + loadBalancer, err = kubeAPIClient.CoreV1().Services(installedInNamespace).Get(context.Background(), generatedLoadBalancerServiceName, metav1.GetOptions{}) + r.Nil(loadBalancer) + r.EqualError(err, "services \"some-service-resource-name\" not found") + }) + }) + }) + + when("there is an error creating the load balancer", func() { + it.Before(func() { + addNodeWithRoleToTracker("worker") + startInformersAndController() + kubeAPIClient.PrependReactor("create", "services", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, fmt.Errorf("error on create") + }) + }) + + it("exits with an error", func() { + r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "could not create load balancer: error on create") + }) }) }, spec.Parallel(), spec.Report(report.Terminal{})) } diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index ed0bfc4a..a342b684 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -292,6 +292,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { controllerlib.WithInformer, controllerlib.WithInitialEvent, "pinniped-concierge-impersonation-proxy-load-balancer", // TODO this string should come from `c.NamesConfig` + c.Labels, tls.Listen, func() (http.Handler, error) { impersonationProxyHandler, err := impersonator.New(c.AuthenticatorCache, c.LoginJSONDecoder, klogr.New().WithName("impersonation-proxy"))