Add test for captured cookie truncation

test/e2e/all_test.go

@@ -35,6 +35,7 @@ func TestAll(t *testing.T) {
 		t.Run("TestForwardedHeaderPolicyReplace", TestForwardedHeaderPolicyReplace)
 		t.Run("TestHAProxyTimeouts", TestHAProxyTimeouts)
 		t.Run("TestHAProxyTimeoutsRejection", TestHAProxyTimeoutsRejection)
+		t.Run("TestCookieLen", TestCookieLen)
 		t.Run("TestHTTPCookieCapture", TestHTTPCookieCapture)
 		t.Run("TestHTTPHeaderBufferSize", TestHTTPHeaderBufferSize)
 		t.Run("TestHTTPHeaderCapture", TestHTTPHeaderCapture)

test/e2e/client_tls_test.go

@@ -1412,19 +1412,6 @@ func verifyCRLs(t *testing.T, pod *corev1.Pod, expectedCRLs map[string]*x509.Rev
 	return true, nil
 }
 
-func getPods(t *testing.T, cl client.Client, deployment *appsv1.Deployment) (*corev1.PodList, error) {
-	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
-	if err != nil {
-		return nil, fmt.Errorf("deployment %s has invalid spec.selector: %w", deployment.Name, err)
-	}
-	podList := &corev1.PodList{}
-	if err := cl.List(context.TODO(), podList, client.MatchingLabelsSelector{Selector: selector}); err != nil {
-		t.Logf("failed to list pods for deployment %q: %v", deployment.Name, err)
-		return nil, err
-	}
-	return podList, nil
-}
-
 func getActiveCRLs(t *testing.T, clientPod *corev1.Pod) ([]*x509.RevocationList, error) {
 	t.Helper()
 	cmd := []string{

test/e2e/haproxy_timeouts_test.go → test/e2e/tuning_options_test.go

@@ -4,19 +4,28 @@
 package e2e
 
 import (
+	"bufio"
 	"bytes"
 	"context"
+	"fmt"
+	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
+	routev1 "github.com/openshift/api/route/v1"
 	"github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
+	"github.com/openshift/cluster-ingress-operator/pkg/operator/controller/ingress"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apiserver/pkg/storage/names"
+	"k8s.io/client-go/kubernetes"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -320,3 +329,195 @@ func TestHAProxyTimeoutsRejection(t *testing.T) {
 		}
 	}
 }
+
+// TestCookieLen verifies that when logging is enabled and a cookie capture is specified, the cookie is truncated at its max length.
+func TestCookieLen(t *testing.T) {
+	t.Parallel()
+	icName := types.NamespacedName{
+		Namespace: operatorNamespace,
+		Name:      names.SimpleNameGenerator.GenerateName("test-cookielen-"),
+	}
+	domain := icName.Name + "." + dnsConfig.Spec.BaseDomain
+	ic := newPrivateController(icName, domain)
+	cookieName := "X-foo-bar"
+	maxLength := 128
+	ic.Spec.Logging = &operatorv1.IngressControllerLogging{
+		Access: &operatorv1.AccessLogging{
+			Destination: operatorv1.LoggingDestination{
+				Type: "Container",
+			},
+			HTTPCaptureCookies: []operatorv1.IngressControllerCaptureHTTPCookie{{
+				IngressControllerCaptureHTTPCookieUnion: operatorv1.IngressControllerCaptureHTTPCookieUnion{
+					MatchType: operatorv1.CookieMatchTypeExact,
+					Name:      cookieName,
+				},
+				MaxLength: maxLength,
+			}},
+			HttpLogFormat: "cookie: %CC",
+		},
+	}
+	if err := kclient.Create(context.TODO(), ic); err != nil {
+		t.Fatalf("Failed to create ingresscontroller %s: %v", icName, err)
+	}
+	defer assertIngressControllerDeleted(t, kclient, ic)
+
+	if err := waitForIngressControllerCondition(t, kclient, 2*time.Minute, icName, availableConditionsForPrivateIngressController...); err != nil {
+		t.Errorf("Timed out waiting for ingresscontroller %s to become available: %v", icName.Name, err)
+	}
+
+	routerDeployment := &appsv1.Deployment{}
+	routerDeploymentName := controller.RouterDeploymentName(ic)
+	if err := kclient.Get(context.TODO(), routerDeploymentName, routerDeployment); err != nil {
+		t.Fatalf("Failed to get router deployment for ingresscontroller %s: %v", icName.Name, err)
+	}
+
+	// As a sanity check, verify that the router deployment sets ROUTER_CAPTURE_HTTP_COOKIE and ROUTER_COOKIELEN to the
+	// expected values.
+	for _, envVar := range routerDeployment.Spec.Template.Spec.Containers[0].Env {
+		if envVar.Name == ingress.RouterCaptureHTTPCookies {
+			expectedValue := fmt.Sprintf("%s=:%d", cookieName, maxLength)
+			if envVar.Value != expectedValue {
+				t.Fatalf("Environment variable %s mismatch. Expected %q, got %q", ingress.RouterCaptureHTTPCookies, expectedValue, envVar.Value)
+			}
+		}
+	}
+
+	// Verify that the rendered haproxy configuration sets tune.http.cookielen to maxLength+1
+
+	// Find the router pod for our ingress controller. This ingress controller specifies replicas=1, so there will only
+	// be one.
+	routerPodList, err := getPods(t, kclient, routerDeployment)
+	if err != nil {
+		t.Fatalf("failed to get pods in deployment %s: %v", routerDeploymentName, err)
+	} else if len(routerPodList.Items) == 0 {
+		t.Fatalf("no pods found in deployment %s", routerDeploymentName)
+	}
+	routerPod := routerPodList.Items[0]
+
+	stdout := bytes.Buffer{}
+	stderr := bytes.Buffer{}
+	cmd := []string{"grep", "tune\\.http\\.cookielen", "/var/lib/haproxy/conf/haproxy.config"}
+	if err := podExec(t, routerPod, &stdout, &stderr, cmd); err != nil {
+		t.Fatalf("failed to execute command %q: %v\nstdout:\n%s\nstderr:\n%s", cmd, err, stdout.String(), stderr.String())
+	}
+	cookielenRegexp := regexp.MustCompile(`tune\.http\.cookielen (\d+)`)
+	cookielenMatches := cookielenRegexp.FindSubmatch(stdout.Bytes())
+	if cookielenMatches == nil || len(cookielenMatches) != 2 {
+		t.Fatalf("failed to parse tune.http.cookielen from grep output. stdout:\n%s", stdout.String())
+	}
+	foundCookielen, err := strconv.Atoi(string(cookielenMatches[1]))
+	if err != nil {
+		t.Fatalf("failed to parse tune.http.cookielen from regexp match %q: %v", cookielenMatches[1], err)
+	}
+	if foundCookielen != maxLength+1 {
+		t.Fatalf("unexpected value for tune.http.cookielen from haproxy config. expected %d, got %d", maxLength+1, foundCookielen)
+	}
+
+	// Send a request through the router that includes the captured cookie, and verify that it's truncated as expected.
+
+	// Use the router image for the client pod since it includes curl.
+	curlPodImage := routerDeployment.Spec.Template.Spec.Containers[0].Image
+	curlPodName := types.NamespacedName{
+		Name:      names.SimpleNameGenerator.GenerateName("cookielen-curl-"),
+		Namespace: "default",
+	}
+	curlPod := buildExecPod(curlPodName.Name, curlPodName.Namespace, curlPodImage)
+	if err := kclient.Create(context.TODO(), curlPod); err != nil {
+		t.Fatalf("failed to create pod %q: %v", curlPodName.Name, err)
+	}
+	defer assertDeleted(t, kclient, curlPod)
+
+	// Wait for the curl pod to be available
+	err = wait.PollImmediate(2*time.Second, 3*time.Minute, func() (bool, error) {
+		if err := kclient.Get(context.TODO(), curlPodName, curlPod); err != nil {
+			t.Logf("failed to get client pod %q: %v", curlPodName, err)
+			return false, nil
+		}
+		for _, cond := range curlPod.Status.Conditions {
+			if cond.Type == corev1.PodReady {
+				return cond.Status == corev1.ConditionTrue, nil
+			}
+		}
+		return false, nil
+	})
+
+	canaryRoute := &routev1.Route{}
+	canaryRouteName := controller.CanaryRouteName()
+	if err := kclient.Get(context.TODO(), canaryRouteName, canaryRoute); err != nil {
+		t.Fatalf("failed to get route %q: %v", canaryRouteName, err)
+	}
+
+	// Get our ingress controllers internal service
+	icInternalService := &corev1.Service{}
+	icInternalServiceName := controller.InternalIngressControllerServiceName(ic)
+	if err := kclient.Get(context.TODO(), icInternalServiceName, icInternalService); err != nil {
+		t.Fatalf("failed to get service %q: %v", icInternalServiceName, err)
+	}
+
+	// Create a cookie that's one character over the max length of a captured cookie. Pad the cookie with a bunch of
+	// 'a's, and include an x at the end that should get truncated. The name of the cookie and the '=' are included in
+	// the max length, so account for that in the size of the padding.
+	padding := strings.Repeat("a", maxLength-len(cookieName)-1)
+	cookieString := fmt.Sprintf("%s=%sx", cookieName, padding)
+	stdout = bytes.Buffer{}
+	stderr = bytes.Buffer{}
+	cmd = []string{
+		"curl", "-k", "-v",
+		"-b", cookieString,
+		// --resolve used this way makes curl send the request to our ingress controller's internal service address as
+		// if it was directed to it by a load balancer. This ensures the request is handled by our ingress controller
+		// without having to wait extra time for a load balancer to be provisioned.
+		"--resolve", fmt.Sprintf("%s:443:%s", canaryRoute.Spec.Host, icInternalService.Spec.ClusterIP),
+		fmt.Sprintf("https://%s", canaryRoute.Spec.Host),
+	}
+	if err := podExec(t, *curlPod, &stdout, &stderr, cmd); err != nil {
+		t.Fatalf("Failed running command %q: %v\nstdout:\n%s\nstderr:\n%s", cmd, err, stdout.String(), stderr.String())
+	}
+
+	// Search the logs container for the specified cookie, making sure that it exactly matches the expected truncated value.
+	expectedCookieString := fmt.Sprintf("%s=%s", cookieName, padding)
+	cookieNameRegexp := regexp.MustCompile(fmt.Sprintf("cookie: %s=", cookieName))
+	fullCookieRegexp := regexp.MustCompile(fmt.Sprintf("cookie: %s$", expectedCookieString))
+
+	kubeConfig, err := config.GetConfig()
+	if err != nil {
+		t.Fatalf("failed to get kube config: %v", err)
+	}
+
+	client, err := kubernetes.NewForConfig(kubeConfig)
+	if err != nil {
+		t.Fatalf("failed to create kube client: %v", err)
+	}
+
+	logsContainerName := "logs"
+	readCloser, err := client.CoreV1().Pods(routerPod.Namespace).GetLogs(routerPod.Name, &corev1.PodLogOptions{
+		Container: logsContainerName,
+		Follow:    false,
+	}).Stream(context.TODO())
+	if err != nil {
+		t.Fatalf("Failed to read logs from pod %s container %s: %v", routerPod.Name, logsContainerName, err)
+	}
+
+	defer func() {
+		if err := readCloser.Close(); err != nil {
+			t.Logf("Failed to close reader for pod %s container %s: %v", routerPod.Name, logsContainerName, err)
+		}
+	}()
+
+	foundCookie := false
+	scanner := bufio.NewScanner(readCloser)
+	for scanner.Scan() {
+		line := scanner.Text()
+		if cookieNameRegexp.MatchString(line) {
+			if fullCookieRegexp.MatchString(line) {
+				foundCookie = true
+				break
+			} else {
+				t.Errorf("Found cookie %s, but value is not as expected. Found:\n%s", cookieName, line)
+			}
+		}
+	}
+	if !foundCookie {
+		t.Errorf("Cookie %q was not found in router pod %q logs", expectedCookieString, routerPod.Name)
+	}
+}

test/e2e/util_test.go

@@ -320,6 +320,19 @@ func getDeployment(t *testing.T, client client.Client, name types.NamespacedName
 	return &dep, nil
 }
 
+func getPods(t *testing.T, cl client.Client, deployment *appsv1.Deployment) (*corev1.PodList, error) {
+	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
+	if err != nil {
+		return nil, fmt.Errorf("deployment %s has invalid spec.selector: %w", deployment.Name, err)
+	}
+	podList := &corev1.PodList{}
+	if err := cl.List(context.TODO(), podList, client.MatchingLabelsSelector{Selector: selector}); err != nil {
+		t.Logf("failed to list pods for deployment %q: %v", deployment.Name, err)
+		return nil, err
+	}
+	return podList, nil
+}
+
 func podExec(t *testing.T, pod corev1.Pod, stdout, stderr *bytes.Buffer, cmd []string) error {
 	t.Helper()
 	kubeConfig, err := config.GetConfig()