NE-2664: add opt-in config to deploy haproxy as sidecar

pkg/manifests/assets/router/deployment.yaml

@@ -10,6 +10,8 @@ spec:
       annotations:
         target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}'
     spec:
+      # manually configure the service account, so we can configure it only in the router container.
+      automountServiceAccountToken: false
       serviceAccountName: router
       # nodeSelector is set at runtime.
       priorityClassName: system-cluster-critical
@@ -69,6 +71,9 @@ spec:
           - mountPath: /var/run/configmaps/service-ca
             name: service-ca-bundle
             readOnly: true
+          - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
+            name: kube-api-access
+            readOnly: true
       tolerations:
       # Ensure the pod isn't deleted during serial NoExecuteTaintManager tests.
       # Remember that NoExecute uses Delete, not Evict, because removing the pod
@@ -90,3 +95,20 @@ spec:
             path: service-ca.crt
           name: service-ca-bundle
           optional: false
+      - name: kube-api-access
+        projected:
+          defaultMode: 0400
+          sources:
+          - serviceAccountToken:
+              expirationSeconds: 3600
+              path: token
+          - configMap:
+              name: kube-root-ca.crt
+              items:
+              - key: ca.crt
+                path: ca.crt
+          - downwardAPI:
+              items:
+              - path: namespace
+                fieldRef:
+                  fieldPath: metadata.namespace

pkg/operator/controller/ingress/deployment.go

@@ -11,6 +11,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -158,6 +159,12 @@ const (
 	StatsPortName = "metrics"
 
 	haproxyMaxTimeoutMilliseconds = 2147483647 * time.Millisecond
+
+	routerInitContainerName        = "init-router"
+	routerHAProxyContainerName     = "haproxy"
+	routerHAProxyConfigVolume      = "haproxy-config"
+	routerHAProxyAdminSocket       = "/var/lib/haproxy/run/admin.sock"
+	routerServiceAccountVolumeName = "kube-api-access"
 )
 
 // ensureRouterDeployment ensures the router deployment exists for a given
@@ -1310,6 +1317,70 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
 		})
 	}
 
+	// TODO move to a proper API - part of https://github.com/openshift/enhancements/pull/1965
+	haproxySidecar := ci.Annotations != nil && ci.Annotations["haproxy-sidecar"] != ""
+
+	if haproxySidecar {
+		// initImage just needs bash, reusing router's image is fine since we already need to download it anyway.
+		initImage := deployment.Spec.Template.Spec.Containers[0].Image
+
+		// haproxyImage uses router's image for now, but it needs its own image.
+		// HAProxy images: https://redhat.atlassian.net/browse/NE-2218
+		// API field: https://redhat.atlassian.net/browse/NE-2217
+		haproxyImage := deployment.Spec.Template.Spec.Containers[0].Image
+
+		// Add the haproxy config volume, this volume is shared between router controller and haproxy sidecar
+		deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, corev1.Volume{
+			Name: routerHAProxyConfigVolume,
+			VolumeSource: corev1.VolumeSource{
+				EmptyDir: &corev1.EmptyDirVolumeSource{},
+			},
+		})
+
+		// The initContainer is responsible for copying static config data from router's container to the shared volume
+		deployment.Spec.Template.Spec.InitContainers = append(deployment.Spec.Template.Spec.InitContainers, corev1.Container{
+			Name:            routerInitContainerName,
+			Image:           initImage,
+			ImagePullPolicy: deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy,
+			Command:         []string{"/bin/bash", "-c", "cp -R -p /var/lib/haproxy/* /mnt/config/"},
+			VolumeMounts: []corev1.VolumeMount{{
+				Name:      routerHAProxyConfigVolume,
+				MountPath: "/mnt/config",
+			}},
+		})
+
+		// Adding the shared haproxy config volume to the router's container. It is mounted in the same expected /var/lib/haproxy directory,
+		// overriding the original content, which was already copied to the shared volume via the init container
+		deployment.Spec.Template.Spec.Containers[0].VolumeMounts = append(deployment.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{
+			Name:      routerHAProxyConfigVolume,
+			MountPath: "/var/lib/haproxy",
+		})
+
+		// HAProxy sidecar mount all the router volumes, except the service account token
+		haproxyVolumeMounts := slices.Clone(deployment.Spec.Template.Spec.Containers[0].VolumeMounts)
+		haproxyVolumeMounts = slices.DeleteFunc(haproxyVolumeMounts, func(mount corev1.VolumeMount) bool {
+			return mount.Name == routerServiceAccountVolumeName
+		})
+
+		// Finally, adding haproxy as a sidecar container
+		deployment.Spec.Template.Spec.Containers = append(deployment.Spec.Template.Spec.Containers, corev1.Container{
+			Name:            routerHAProxyContainerName,
+			Image:           haproxyImage,
+			ImagePullPolicy: deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy,
+			Command:         []string{"/var/lib/haproxy/start-haproxy"},
+			Args:            []string{"-W", "-db", "-S", routerHAProxyAdminSocket + ",mode,600", "-f", "/var/lib/haproxy/conf/haproxy.config"},
+			Resources:       deployment.Spec.Template.Spec.Containers[0].Resources,
+			VolumeMounts:    haproxyVolumeMounts,
+		})
+
+		// ROUTER_HAPROXY_ADMIN_UNIX_SOCKET envvar, if configured, instructs the router that
+		// haproxy is deployed as a sidecar container, otherwise router uses the embedded haproxy.
+		env = append(env, corev1.EnvVar{
+			Name:  "ROUTER_HAPROXY_ADMIN_UNIX_SOCKET",
+			Value: routerHAProxyAdminSocket,
+		})
+	}
+
 	// TODO: The only connections from the router that may need the cluster-wide proxy are those for downloading CRLs,
 	// which, as of writing this, will always be http. If https becomes necessary, the router will need to mount the
 	// trusted CA bundle that cluster-network-operator generates. The process for adding that is described here:
@@ -1591,6 +1662,7 @@ func hashableDeployment(deployment *appsv1.Deployment, onlyTemplate bool) *appsv
 		return containers[i].Name < containers[j].Name
 	})
 	hashableDeployment.Spec.Template.Spec.Containers = containers
+	hashableDeployment.Spec.Template.Spec.InitContainers = deployment.Spec.Template.Spec.InitContainers
 	hashableDeployment.Spec.Template.Spec.DNSPolicy = deployment.Spec.Template.Spec.DNSPolicy
 	hashableDeployment.Spec.Template.Spec.HostNetwork = deployment.Spec.Template.Spec.HostNetwork
 	volumes := make([]corev1.Volume, len(deployment.Spec.Template.Spec.Volumes))
@@ -1773,6 +1845,7 @@ func deploymentConfigChanged(current, expected *appsv1.Deployment) (bool, *appsv
 		containers[i+1] = *container.DeepCopy()
 	}
 	updated.Spec.Template.Spec.Containers = containers
+	updated.Spec.Template.Spec.InitContainers = expected.Spec.Template.Spec.InitContainers
 	updated.Spec.Template.Spec.DNSPolicy = expected.Spec.Template.Spec.DNSPolicy
 	updated.Spec.Template.Labels = expected.Spec.Template.Labels
 

pkg/operator/controller/ingress/deployment_test.go

@@ -4,13 +4,15 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
@@ -59,10 +61,10 @@ func checkDeploymentHasEnvSorted(t *testing.T, deployment *appsv1.Deployment) {
 	}
 }
 
-func checkDeploymentHasContainer(t *testing.T, deployment *appsv1.Deployment, name string, expect bool) {
+func checkDeploymentHasContainer(t *testing.T, containers []corev1.Container, name string, expect bool) {
 	t.Helper()
 
-	for _, container := range deployment.Spec.Template.Spec.Containers {
+	for _, container := range containers {
 		if container.Name == name {
 			if expect {
 				return
@@ -77,7 +79,7 @@ func checkDeploymentHasContainer(t *testing.T, deployment *appsv1.Deployment, na
 
 func checkRouterContainerSecurityContext(t *testing.T, deployment *appsv1.Deployment) {
 	t.Helper()
-	checkDeploymentHasContainer(t, deployment, routerContainerName, true)
+	checkDeploymentHasContainer(t, deployment.Spec.Template.Spec.Containers, routerContainerName, true)
 
 	for _, container := range deployment.Spec.Template.Spec.Containers {
 		if container.Name == routerContainerName {
@@ -648,6 +650,34 @@ func assertVolumeHasDefaultMode(t *testing.T, expected int32, actual *int32, vol
 	}
 }
 
+func assertVolumeHasServiceAccount(t *testing.T, volume corev1.Volume) {
+	t.Helper()
+
+	if !assert.NotNil(t, volume.Projected) {
+		return
+	}
+
+	assertVolumeHasDefaultMode(t, int32(0400), volume.Projected.DefaultMode, volume.Name)
+
+	for _, source := range volume.Projected.Sources {
+		switch {
+		case source.ServiceAccountToken != nil:
+			assert.Equal(t, ptr.To(int64(3600)), source.ServiceAccountToken.ExpirationSeconds)
+			assert.Equal(t, "token", source.ServiceAccountToken.Path)
+		case source.ConfigMap != nil:
+			assert.Equal(t, "kube-root-ca.crt", source.ConfigMap.Name)
+			assert.Equal(t, []corev1.KeyToPath{{Key: "ca.crt", Path: "ca.crt"}}, source.ConfigMap.Items)
+		case source.DownwardAPI != nil:
+			volumeFile := []corev1.DownwardAPIVolumeFile{{
+				Path: "namespace",
+				FieldRef: &corev1.ObjectFieldSelector{
+					FieldPath: "metadata.namespace"},
+			}}
+			assert.Equal(t, volumeFile, source.DownwardAPI.Items)
+		}
+	}
+}
+
 // checkProbes asserts that the given container specifies liveness, readiness,
 // and startup probes, and that these probes have the expected parameters.  If
 // useLocalhost is true, checkProbes asserts that the probe's HTTP action
@@ -695,18 +725,66 @@ func TestDesiredRouterDeploymentSpecTemplate(t *testing.T) {
 		t.Fatalf("invalid router Deployment: %v", err)
 	}
 
-	expectedVolumeSecretPairs := map[string]string{
-		"default-certificate": fmt.Sprintf("router-certs-%s", ic.Name),
-		"metrics-certs":       fmt.Sprintf("router-metrics-certs-%s", ic.Name),
-		"stats-auth":          fmt.Sprintf("router-stats-%s", ic.Name),
+	expectedVolumes := []string{
+		"default-certificate",
+		"metrics-certs",
+		"stats-auth",
+		"service-ca-bundle",
+		routerServiceAccountVolumeName,
+	}
+	hasDesiredRouterDeploymentSpecTemplate(t, ic, deployment, expectedVolumes)
+}
+
+func TestDesiredRouterDeploymentSpecTemplateSidecar(t *testing.T) {
+	ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)
+
+	if ic.Annotations == nil {
+		ic.Annotations = make(map[string]string)
+	}
+	ic.Annotations["haproxy-sidecar"] = "1"
+	deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, nil, proxyNeeded, false, nil, clusterProxyConfig)
+	if err != nil {
+		t.Fatalf("invalid router Deployment: %v", err)
+	}
+
+	require.Len(t, deployment.Spec.Template.Spec.InitContainers, 1, "len(initContainers)")
+	require.Len(t, deployment.Spec.Template.Spec.Containers, 2, "len(containers)")
+
+	checkDeploymentHasContainer(t, deployment.Spec.Template.Spec.Containers, routerHAProxyContainerName, true)
+	checkDeploymentHasContainer(t, deployment.Spec.Template.Spec.InitContainers, routerInitContainerName, true)
+
+	envvars := []envData{
+		{name: "ROUTER_HAPROXY_ADMIN_UNIX_SOCKET", expectPresent: true, expectedValue: routerHAProxyAdminSocket},
+	}
+	if err := checkDeploymentEnvironment(t, deployment, envvars); err != nil {
+		t.Error(err)
 	}
+
 	expectedVolumes := []string{
 		"default-certificate",
 		"metrics-certs",
 		"stats-auth",
 		"service-ca-bundle",
+		routerServiceAccountVolumeName, // available in the pod but not mounted in the container
+		routerHAProxyConfigVolume,
 	}
+	hasDesiredRouterDeploymentSpecTemplate(t, ic, deployment, expectedVolumes)
+
+	kubeAPIAccessMounted := slices.ContainsFunc(deployment.Spec.Template.Spec.Containers[1].VolumeMounts, func(mount corev1.VolumeMount) bool {
+		return mount.Name == routerServiceAccountVolumeName
+	})
+	require.Falsef(t, kubeAPIAccessMounted, "haproxy sidecar is unexpectedly mounting volume %q", routerServiceAccountVolumeName)
+}
+
+// hasDesiredRouterDeploymentSpecTemplate has common configuration for router deployments, with and without an HAProxy sidecar
+func hasDesiredRouterDeploymentSpecTemplate(t *testing.T, ic *operatorv1.IngressController, deployment *appsv1.Deployment, expectedVolumes []string) {
 	assertHasVolumes(t, deployment.Spec.Template.Spec.Volumes, expectedVolumes)
+
+	expectedVolumeSecretPairs := map[string]string{
+		"default-certificate": fmt.Sprintf("router-certs-%s", ic.Name),
+		"metrics-certs":       fmt.Sprintf("router-metrics-certs-%s", ic.Name),
+		"stats-auth":          fmt.Sprintf("router-stats-%s", ic.Name),
+	}
 	for _, volume := range deployment.Spec.Template.Spec.Volumes {
 		if secretName, ok := expectedVolumeSecretPairs[volume.Name]; ok {
 			if volume.Secret.SecretName != secretName {
@@ -718,6 +796,9 @@ func TestDesiredRouterDeploymentSpecTemplate(t *testing.T) {
 		switch volume.Name {
 		case "service-ca-bundle":
 			assertVolumeHasDefaultMode(t, int32(0644), volume.ConfigMap.DefaultMode, volume.Name)
+		case routerServiceAccountVolumeName:
+			assertVolumeHasServiceAccount(t, volume)
+		case routerHAProxyConfigVolume:
 		default:
 			t.Errorf("router deployment has unexpected volume %s", volume.Name)
 		}
@@ -757,7 +838,7 @@ func TestDesiredRouterDeploymentSpecTemplate(t *testing.T) {
 
 	checkProbes(t, &deployment.Spec.Template.Spec.Containers[0], false)
 
-	checkDeploymentHasContainer(t, deployment, operatorv1.ContainerLoggingSidecarContainerName, false)
+	checkDeploymentHasContainer(t, deployment.Spec.Template.Spec.Containers, operatorv1.ContainerLoggingSidecarContainerName, false)
 
 	checkDeploymentHasEnvSorted(t, deployment)
 }
@@ -856,6 +937,7 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
 		"error-pages",
 		"rsyslog-config",
 		"rsyslog-socket",
+		routerServiceAccountVolumeName,
 	}
 	assertHasVolumes(t, deployment.Spec.Template.Spec.Volumes, expectedVolumes)
 	for _, volume := range deployment.Spec.Template.Spec.Volumes {
@@ -864,13 +946,15 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
 			assertVolumeHasDefaultMode(t, int32(0644), volume.Secret.DefaultMode, volume.Name)
 		case "error-pages", "rsyslog-config", "service-ca-bundle":
 			assertVolumeHasDefaultMode(t, int32(0644), volume.ConfigMap.DefaultMode, volume.Name)
+		case routerServiceAccountVolumeName:
+			assertVolumeHasServiceAccount(t, volume)
 		case "rsyslog-socket":
 		default:
 			t.Errorf("router deployment has unexpected volume %s", volume.Name)
 		}
 	}
 
-	checkDeploymentHasContainer(t, deployment, operatorv1.ContainerLoggingSidecarContainerName, true)
+	checkDeploymentHasContainer(t, deployment.Spec.Template.Spec.Containers, operatorv1.ContainerLoggingSidecarContainerName, true)
 	tests := []envData{
 		{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},
 		{"ROUTER_LOAD_BALANCE_ALGORITHM", true, "leastconn"},
@@ -1084,6 +1168,7 @@ func TestDesiredRouterDeploymentVariety(t *testing.T) {
 		"metrics-certs",
 		"stats-auth",
 		"service-ca-bundle",
+		routerServiceAccountVolumeName,
 	}
 	assertHasVolumes(t, deployment.Spec.Template.Spec.Volumes, expectedVolumes)
 	for _, volume := range deployment.Spec.Template.Spec.Volumes {
@@ -1097,12 +1182,14 @@ func TestDesiredRouterDeploymentVariety(t *testing.T) {
 		switch volume.Name {
 		case "service-ca-bundle":
 			assertVolumeHasDefaultMode(t, int32(0644), volume.ConfigMap.DefaultMode, volume.Name)
+		case routerServiceAccountVolumeName:
+			assertVolumeHasServiceAccount(t, volume)
 		default:
 			t.Errorf("router deployment has unexpected volume %s", volume.Name)
 		}
 	}
 
-	checkDeploymentHasContainer(t, deployment, operatorv1.ContainerLoggingSidecarContainerName, false)
+	checkDeploymentHasContainer(t, deployment.Spec.Template.Spec.Containers, operatorv1.ContainerLoggingSidecarContainerName, false)
 	tests := []envData{
 		{"ROUTER_LOG_FACILITY", true, "local2"},
 		{"ROUTER_LOG_MAX_LENGTH", true, "4096"},
@@ -1300,6 +1387,7 @@ func TestDesiredRouterDeploymentClientTLS(t *testing.T) {
 		"stats-auth",
 		"service-ca-bundle",
 		"client-ca",
+		routerServiceAccountVolumeName,
 	}
 	assertHasVolumes(t, deployment.Spec.Template.Spec.Volumes, expectedVolumes)
 	for _, volume := range deployment.Spec.Template.Spec.Volumes {
@@ -1311,6 +1399,8 @@ func TestDesiredRouterDeploymentClientTLS(t *testing.T) {
 		case "client-ca":
 			assertVolumeHasDefaultMode(t, int32(0644), volume.ConfigMap.DefaultMode, volume.Name)
 			assert.Equal(t, "router-client-ca-default", volume.VolumeSource.ConfigMap.LocalObjectReference.Name)
+		case routerServiceAccountVolumeName:
+			assertVolumeHasServiceAccount(t, volume)
 		default:
 			t.Errorf("router deployment has unexpected volume %s", volume.Name)
 		}