amend tchap suggestions

the change includes tests moving to `WaitForOperatorToPickUpChanges()` when authentication
CR is updated, VerifyOAuthServerDeploymentProxyConfig() being used in now merged
env var tests, ensuring no connect entries for squid proxy in relevant tests,
and CA file reconfigure for squid proxy to verify hot reload behavior.

Author: ehearne-redhat

test/e2e-component-proxy/component_proxy_oidc_login.go

@@ -2,11 +2,12 @@ package component_proxy
 
 import (
 	"context"
+	"crypto/x509"
+	"encoding/pem"
 	"fmt"
 	"io"
 	"net/url"
-	"os"
-	"path"
+	"os/exec"
 	"testing"
 	"time"
 
@@ -23,19 +24,15 @@ import (
 
 	"github.com/openshift/api/features"
 	operatorv1 "github.com/openshift/api/operator/v1"
-	"github.com/openshift/library-go/pkg/crypto"
 	"github.com/openshift/library-go/pkg/oauth/tokenrequest"
 	"github.com/openshift/library-go/pkg/oauth/tokenrequest/challengehandlers"
 
 	test "github.com/openshift/cluster-authentication-operator/test/library"
 )
 
 var _ = g.Describe("[sig-auth] authentication operator", func() {
-	g.It("[Serial][Operator][ComponentProxy] should set only HTTPS_PROXY env var when only httpsProxy is configured", func() {
-		testPartialProxyEnvVars()
-	})
-	g.It("[Serial][Operator][ComponentProxy] should set both HTTP_PROXY and HTTPS_PROXY env vars when fully configured", func() {
-		testFullProxyEnvVars()
+	g.It("[Serial][Operator][ComponentProxy] should set partial and full env vars when configured", func() {
+		testPartialFullProxyEnvVars()
 	})
 	g.It("[Serial][Operator][ComponentProxy] should apply proxy config and perform full OIDC login flow", func() {
 		testProxyOIDCLoginFlow()
@@ -114,7 +111,7 @@ func assertOIDCLogin(t testing.TB, kubeConfig *rest.Config, routeClient test.Tes
 	o.Expect(err).NotTo(o.HaveOccurred(), "OIDC login flow should succeed")
 }
 
-func testPartialProxyEnvVars() {
+func testPartialFullProxyEnvVars() {
 	ctx := context.Background()
 	t := g.GinkgoTB()
 	clients := test.NewTestClients(t)
@@ -126,7 +123,7 @@ func testPartialProxyEnvVars() {
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Deploying Squid forward proxy")
-	proxyURL, _, _, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
+	httpProxyURL, httpsProxyURL, caPEM, _, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
 	g.DeferCleanup(func() {
 		g.GinkgoWriter.Println("cleaning up: removing Squid proxy")
 		proxyCleanup()
@@ -138,66 +135,59 @@ func testPartialProxyEnvVars() {
 
 	g.By("Setting only httpsProxy in component proxy config")
 	operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{
-		HTTPSProxy: proxyURL,
+		HTTPSProxy: httpsProxyURL,
 	}
 	_, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Waiting for operator to reconcile proxy config")
-	err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
+	err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication")
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Verifying oauth-server has HTTPS_PROXY but not HTTP_PROXY")
-	envVars := test.GetOAuthServerProxyEnvVars(t, clients.KubeClient)
-	o.Expect(envVars).To(o.HaveKeyWithValue("HTTPS_PROXY", proxyURL))
-	o.Expect(envVars).NotTo(o.HaveKey("HTTP_PROXY"))
-	// NO_PROXY default values (.cluster.local, .svc, etc.) are verified in proxy_test.go unit tests
-	o.Expect(envVars).To(o.HaveKey("NO_PROXY"))
-}
-
-func testFullProxyEnvVars() {
-	ctx := context.Background()
-	t := g.GinkgoTB()
-	clients := test.NewTestClients(t)
-
-	test.CheckFeatureGateEnabledOrSkip(t, clients.ConfigClient, features.FeatureGateAuthenticationComponentProxy)
+	test.VerifyOAuthServerDeploymentProxyConfig(t, clients.KubeClient, "", httpsProxyURL, "", false)
 
-	g.By("Waiting for authentication operator to be stable before test")
-	err := test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
-	o.Expect(err).NotTo(o.HaveOccurred())
-
-	g.By("Deploying Squid forward proxy")
-	proxyURL, _, _, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
+	configMapName := "e2e-proxy-trusted-ca"
+	caConfigMap := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      configMapName,
+			Namespace: "openshift-config",
+		},
+		Data: map[string]string{
+			"ca-bundle.crt": string(caPEM),
+		},
+	}
+	_, err = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, caConfigMap, metav1.CreateOptions{})
+	o.Expect(err).NotTo(o.HaveOccurred(), "should be able to create trustedCA ConfigMap")
 	g.DeferCleanup(func() {
-		g.GinkgoWriter.Println("cleaning up: removing Squid proxy")
-		proxyCleanup()
+		if err := clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Delete(ctx, configMapName, metav1.DeleteOptions{}); err != nil {
+			g.GinkgoWriter.Printf("failed to clean up ConfigMap %s: %v\n", configMapName, err)
+		}
 	})
 
-	g.By("Saving original proxy config")
-	operatorAuth, proxyConfigCleanup := test.SaveAndRestoreProxyConfig(t, clients.OperatorClient, clients.ConfigClient)
-	g.DeferCleanup(proxyConfigCleanup)
-
 	noProxyHost := "noproxy.example.com"
 
-	g.By("Setting httpProxy, httpsProxy, and noProxy in component proxy config")
+	operatorAuth, err = clients.OperatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{})
+	o.Expect(err).NotTo(o.HaveOccurred())
+
+	g.By("Setting httpProxy, httpsProxy, noProxy, and trustedCA in component proxy config")
 	operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{
-		HTTPProxy:  proxyURL,
-		HTTPSProxy: proxyURL,
+		HTTPProxy:  httpProxyURL,
+		HTTPSProxy: httpsProxyURL,
 		NoProxy:    []string{noProxyHost},
+		TrustedCA: operatorv1.AuthenticationConfigMapReference{
+			Name: configMapName,
+		},
 	}
 	_, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Waiting for operator to reconcile proxy config")
-	err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
+	err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication")
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Verifying oauth-server has HTTP_PROXY, HTTPS_PROXY, and NO_PROXY with custom entry")
-	envVars := test.GetOAuthServerProxyEnvVars(t, clients.KubeClient)
-	o.Expect(envVars).To(o.HaveKeyWithValue("HTTP_PROXY", proxyURL))
-	o.Expect(envVars).To(o.HaveKeyWithValue("HTTPS_PROXY", proxyURL))
-	o.Expect(envVars).To(o.HaveKey("NO_PROXY"))
-	o.Expect(envVars["NO_PROXY"]).To(o.ContainSubstring(noProxyHost))
+	test.VerifyOAuthServerDeploymentProxyConfig(t, clients.KubeClient, httpProxyURL, httpsProxyURL, noProxyHost, true)
 }
 
 func testProxyOIDCLoginFlow() {
@@ -213,7 +203,7 @@ func testProxyOIDCLoginFlow() {
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Deploying Squid forward proxy")
-	proxyURL, _, proxyNamespace, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
+	_, httpsProxyURL, _, proxyNamespace, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
 	g.DeferCleanup(func() {
 		g.GinkgoWriter.Println("cleaning up: removing Squid proxy")
 		proxyCleanup()
@@ -224,7 +214,7 @@ func testProxyOIDCLoginFlow() {
 	g.DeferCleanup(proxyConfigCleanup)
 
 	operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{
-		HTTPSProxy: proxyURL,
+		HTTPSProxy: httpsProxyURL,
 	}
 	_, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
@@ -255,7 +245,7 @@ func testProxyOIDCLoginFlow() {
 	})
 
 	g.By("Waiting for operator to reconcile proxy config")
-	err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
+	err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication")
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Creating Keycloak test user and group")
@@ -287,7 +277,7 @@ func testDirectIdPFallback() {
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Deploying Squid forward proxy")
-	proxyURL, _, _, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
+	_, httpsProxyURL, _, proxyNS, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
 	g.DeferCleanup(func() {
 		g.GinkgoWriter.Println("cleaning up: removing Squid proxy")
 		proxyCleanup()
@@ -299,13 +289,13 @@ func testDirectIdPFallback() {
 
 	g.By("Setting component-scoped proxy config")
 	operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{
-		HTTPSProxy: proxyURL,
+		HTTPSProxy: httpsProxyURL,
 	}
 	_, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Waiting for operator to reconcile proxy config")
-	err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
+	err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication")
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Deploying Keycloak and adding OIDC IdP")
@@ -317,6 +307,9 @@ func testDirectIdPFallback() {
 		}
 	}))
 
+	issuerURL, err := url.Parse(kcClient.IssuerURL())
+	o.Expect(err).NotTo(o.HaveOccurred())
+
 	g.By("Creating Keycloak test user and group")
 	group := "ocp-test-direct-fallback-group"
 	o.Expect(kcClient.CreateGroup(group)).To(o.Succeed())
@@ -333,11 +326,16 @@ func testDirectIdPFallback() {
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Waiting for operator to reconcile proxy removal")
-	err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
+	err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication")
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Performing OIDC login flow via direct IdP connectivity after proxy removal")
 	assertOIDCLogin(t, kubeConfig, *clients, username, password, group)
+
+	logs := test.GetSquidProxyLogs(t, clients.KubeClient, proxyNS)
+
+	keycloakHost := issuerURL.Hostname()
+	o.Expect(logs).NotTo(o.ContainSubstring(keycloakHost), "squid logs should not contain keycloak connect")
 }
 
 func testTrustedCAHotReload() {
@@ -353,7 +351,7 @@ func testTrustedCAHotReload() {
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Deploying Squid forward proxy")
-	proxyURL, _, proxyNamespace, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
+	_, httpsProxyURL, caFile, proxyNamespace, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
 	g.DeferCleanup(func() {
 		g.GinkgoWriter.Println("cleaning up: removing Squid proxy")
 		proxyCleanup()
@@ -387,31 +385,14 @@ func testTrustedCAHotReload() {
 		networkPolicyCleanup()
 	})
 
-	g.By("Generating self-signed CA and creating trustedCA ConfigMap")
-	tempDir, err := os.MkdirTemp("", "testca")
-	o.Expect(err).NotTo(o.HaveOccurred())
-	defer os.RemoveAll(tempDir)
-
-	ca, err := crypto.MakeSelfSignedCA(
-		path.Join(tempDir, "ca.crt"),
-		path.Join(tempDir, "ca.key"),
-		path.Join(tempDir, "serial"),
-		"proxy-e2e-ca",
-		100*24*time.Hour,
-	)
-	o.Expect(err).NotTo(o.HaveOccurred())
-
-	caPEM, _, err := ca.Config.GetPEMBytes()
-	o.Expect(err).NotTo(o.HaveOccurred())
-
 	configMapName := "e2e-proxy-trusted-ca"
 	caConfigMap := &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      configMapName,
 			Namespace: "openshift-config",
 		},
 		Data: map[string]string{
-			"ca-bundle.crt": string(caPEM),
+			"ca-bundle.crt": string(caFile),
 		},
 	}
 	_, err = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, caConfigMap, metav1.CreateOptions{})
@@ -424,7 +405,7 @@ func testTrustedCAHotReload() {
 
 	g.By("Setting component-scoped proxy with trustedCA")
 	operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{
-		HTTPSProxy: proxyURL,
+		HTTPSProxy: httpsProxyURL,
 		TrustedCA: operatorv1.AuthenticationConfigMapReference{
 			Name: configMapName,
 		},
@@ -433,7 +414,7 @@ func testTrustedCAHotReload() {
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Waiting for operator to reconcile proxy config with trustedCA")
-	err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
+	err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication")
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Creating Keycloak test user and group")
@@ -464,22 +445,45 @@ func testTrustedCAHotReload() {
 		podNamesBefore.Insert(pod.Name)
 	}
 
-	g.By("Rotating CA: generating new self-signed CA and updating ConfigMap")
-	newCA, err := crypto.MakeSelfSignedCA(
-		path.Join(tempDir, "ca-new.crt"),
-		path.Join(tempDir, "ca-new.key"),
-		path.Join(tempDir, "serial-new"),
-		"proxy-e2e-ca-rotated",
-		100*24*time.Hour,
-	)
+	g.By("Rotating CA: generating new CA and server cert")
+	newCA := test.NewCertificateAuthorityCertificate(t, nil)
+	serviceDNS := fmt.Sprintf("squid-proxy.%s.svc.cluster.local", proxyNamespace)
+	newServerCert := test.NewServerCertificate(t, newCA, serviceDNS)
+
+	newCACertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: newCA.Certificate.Raw})
+	newServerCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: newServerCert.Certificate.Raw})
+	newServerKeyDER, err := x509.MarshalPKCS8PrivateKey(newServerCert.PrivateKey)
+	o.Expect(err).NotTo(o.HaveOccurred())
+	newServerKeyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: newServerKeyDER})
+
+	g.By("Updating squid-tls Secret with rotated cert")
+	tlsSecret, err := clients.KubeClient.CoreV1().Secrets(proxyNamespace).Get(ctx, "squid-tls", metav1.GetOptions{})
+	o.Expect(err).NotTo(o.HaveOccurred())
+	tlsSecret.Data["tls.crt"] = newServerCertPEM
+	tlsSecret.Data["tls.key"] = newServerKeyPEM
+	_, err = clients.KubeClient.CoreV1().Secrets(proxyNamespace).Update(ctx, tlsSecret, metav1.UpdateOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
 
-	newCAPEM, _, err := newCA.Config.GetPEMBytes()
+	g.By("Reconfiguring Squid to pick up new cert")
+	squidPods, err := clients.KubeClient.CoreV1().Pods(proxyNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: "app=squid-proxy",
+	})
 	o.Expect(err).NotTo(o.HaveOccurred())
+	o.Expect(squidPods.Items).NotTo(o.BeEmpty())
+
+	reconfigureCmd := exec.Command("oc", "exec",
+		"-n", proxyNamespace,
+		squidPods.Items[0].Name,
+		"-c", "squid",
+		"--", "squid", "-k", "reconfigure",
+	)
+	output, err := reconfigureCmd.CombinedOutput()
+	o.Expect(err).NotTo(o.HaveOccurred(), "squid reconfigure failed: %s", string(output))
 
+	g.By("Updating trustedCA ConfigMap with new CA")
 	cm, err := clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Get(ctx, configMapName, metav1.GetOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
-	cm.Data["ca-bundle.crt"] = string(newCAPEM)
+	cm.Data["ca-bundle.crt"] = string(newCACertPEM)
 	_, err = clients.KubeClient.CoreV1().ConfigMaps("openshift-config").Update(ctx, cm, metav1.UpdateOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
 
@@ -516,7 +520,7 @@ func testNoProxy() {
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Deploying Squid forward proxy")
-	proxyURL, _, proxyNS, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
+	_, httpsProxyURL, _, proxyNS, proxyCleanup := test.DeploySquidProxy(t, clients.KubeClient)
 	g.DeferCleanup(func() {
 		g.GinkgoWriter.Println("cleaning up: removing Squid proxy")
 		proxyCleanup()
@@ -540,15 +544,15 @@ func testNoProxy() {
 
 	g.By("Setting component-scoped proxy with noProxy")
 	operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{
-		HTTPSProxy: proxyURL,
+		HTTPSProxy: httpsProxyURL,
 		NoProxy:    []string{issuerURL.Hostname()},
 	}
 
 	_, err = clients.OperatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{})
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Waiting for operator to reconcile proxy config")
-	err = test.WaitForClusterOperatorAvailableNotProgressingNotDegraded(t, clients.ConfigClient.ConfigV1(), "authentication")
+	err = test.WaitForOperatorToPickUpChanges(t, clients.ConfigClient.ConfigV1(), "authentication")
 	o.Expect(err).NotTo(o.HaveOccurred())
 
 	g.By("Creating Keycloak test user and group")