Add loadbalancer for impersonation proxy when needed

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
+	// start the load balancer only if:
-	// 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
+	// - the impersonator is running
-	// TODO when auto mode decides that the proxy should be disabled, then it also does not create the LoadBalancer (or it deletes it)
+	// - the cluster is cloud hosted
-
+	// - there is no endpoint specified in the config
-	// client, err := kubeclient.New()
+	if c.server != nil && !*c.hasControlPlaneNodes && config.Endpoint == "" {
-	// if err != nil {
+		if err = c.startLoadBalancer(ctx.Context); err != nil {
-	// 	plog.WarningErr("could not create client", err)
+			return err
-	// } else {
+		}
-	// 	appNameLabel := cfg.Labels["app"]
+	} else {
-	// 	loadBalancer := v1.Service{
+		if err = c.stopLoadBalancer(ctx.Context); err != nil {
-	// 		Spec: v1.ServiceSpec{
+			return err
-	// 			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)
-	// 	}
-	// }
 
 	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
+}

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(
-						coretesting.NewListAction(
+						schema.GroupVersionResource{Version: "v1", Resource: "nodes"},
-							schema.GroupVersionResource{Version: "v1", Resource: "nodes"},
+						schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"},
-							schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"},
+						"",
-							"",
+						metav1.ListOptions{}),
-							metav1.ListOptions{}),
+					kubeAPIClient.Actions()[0],
-					},
-					kubeAPIClient.Actions(),
 				)
 
 				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{}))
 }

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