From 4f62ee9a1becad2c43129566874d233fa1f8674a Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Thu, 30 Jul 2026 15:07:46 +0200 Subject: [PATCH 01/16] Add component-scoped proxy e2e test helpers and keycloak client extensions Add helper functions for deploying a Squid forward proxy, managing proxy-scoped network policies, configuring component-scoped proxy on the Authentication CR, and verifying OAuth server deployment alignment. Extend the keycloak client with group/audience mapper creation, client configuration, and client lookup by clientID. Add crypto helpers for generating self-signed CA and server certificates used by the Squid proxy's HTTPS listener. Refactor keycloak deployment cleanup to rely on namespace cascading, reducing cleanup API calls from 6 to 2 (namespace + CA configmap in openshift-config). All deploy helpers self-clean on error and return nil cleanups, preventing resource leaks when callers use Expect before DeferCleanup. --- .../authentication/component_proxy_helpers.go | 801 ++++++++++++++++++ .../extended/authentication/crypto_helpers.go | 89 ++ .../authentication/keycloak_client.go | 97 ++- .../authentication/keycloak_helpers.go | 52 +- .../authentication/operator_status_helpers.go | 77 ++ 5 files changed, 1090 insertions(+), 26 deletions(-) create mode 100644 test/extended/authentication/component_proxy_helpers.go create mode 100644 test/extended/authentication/crypto_helpers.go create mode 100644 test/extended/authentication/operator_status_helpers.go diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go new file mode 100644 index 000000000000..18f2a7bfd0f4 --- /dev/null +++ b/test/extended/authentication/component_proxy_helpers.go @@ -0,0 +1,801 @@ +package authentication + +import ( + "context" + "crypto/x509" + "encoding/pem" + "fmt" + "reflect" + "strings" + "time" + + g "github.com/onsi/ginkgo/v2" + configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/tools/cache" + watchtools "k8s.io/client-go/tools/watch" + "k8s.io/client-go/util/retry" + + exutil "github.com/openshift/origin/test/extended/util" + "github.com/openshift/origin/test/extended/util/image" +) + +const ( + squidImage = "registry.redhat.io/rhel10/squid:10.2-1784702318" + squidHTTPPort = int32(3128) + squidHTTPSPort = int32(3129) + squidServiceName = "squid-proxy" + + componentProxyCAConfigMapName = "v4-0-config-system-auth-proxy-ca" +) + +func componentProxyTestLabels() map[string]string { + return map[string]string{ + "e2e-test": "openshift-authentication-operator", + } +} + +// resetComponentProxyState clears leftover state from killed test runs so each +// test starts clean regardless of what a previous run left behind. +func resetComponentProxyState(ctx context.Context, oc *exutil.CLI) { + err := updateAuthenticationProxy(ctx, oc, operatorv1.AuthenticationProxyConfig{}) + if err != nil { + g.GinkgoWriter.Printf("pre-test: failed to clear spec.proxy: %v\n", err) + } + + oauth, err := oc.AdminConfigClient().ConfigV1().OAuths().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + g.GinkgoWriter.Printf("pre-test: failed to get oauth/cluster: %v\n", err) + return + } + var kept []configv1.IdentityProvider + for _, idp := range oauth.Spec.IdentityProviders { + if !strings.HasPrefix(idp.Name, "keycloak-proxy-test-") { + kept = append(kept, idp) + } + } + if len(kept) != len(oauth.Spec.IdentityProviders) { + oauth.Spec.IdentityProviders = kept + if _, err := oc.AdminConfigClient().ConfigV1().OAuths().Update(ctx, oauth, metav1.UpdateOptions{}); err != nil { + g.GinkgoWriter.Printf("pre-test: failed to clean leftover IdPs: %v\n", err) + } + } +} + +// saveAndRestoreProxyConfig snapshots spec.proxy on the operator Authentication +// CR and returns a cleanup function that restores it. +func saveAndRestoreProxyConfig(ctx context.Context, oc *exutil.CLI) (removalFunc, error) { + operatorClient := oc.AdminOperatorClient() + + auth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("getting operator authentication CR: %w", err) + } + originalProxy := auth.Spec.Proxy.DeepCopy() + + cleanup := func() { + g.GinkgoWriter.Println("cleanup: restoring original proxy config") + var changed bool + err := wait.PollUntilContextTimeout(ctx, 1*time.Second, 30*time.Second, true, func(ctx context.Context) (bool, error) { + fresh, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + g.GinkgoWriter.Printf("cleanup: failed to get operator auth: %v\n", err) + return false, nil + } + target := operatorv1.AuthenticationProxyConfig{} + if originalProxy != nil { + target = *originalProxy + } + if reflect.DeepEqual(fresh.Spec.Proxy, target) { + g.GinkgoWriter.Println("cleanup: proxy config already matches original, no update needed") + return true, nil + } + fresh.Spec.Proxy = target + if _, err := operatorClient.OperatorV1().Authentications().Update(ctx, fresh, metav1.UpdateOptions{}); err != nil { + g.GinkgoWriter.Printf("cleanup: failed to update operator auth (will retry): %v\n", err) + return false, nil + } + changed = true + return true, nil + }) + if err != nil { + g.GinkgoWriter.Printf("cleanup: failed to restore proxy config: %v\n", err) + return + } + if !changed { + return + } + g.GinkgoWriter.Println("cleanup: waiting for operator to pick up changes and stabilize") + if err := waitForOperatorToPickUpChanges(ctx, oc, "authentication"); err != nil { + g.GinkgoWriter.Printf("cleanup: operator did not recover: %v\n", err) + } + } + + return func(ctx context.Context) error { + cleanup() + return nil + }, nil +} + +// deploySquidProxy deploys a Squid forward proxy listening on HTTP (3128) and +// HTTPS (3129) with a self-signed CA and serving certificate. +func deploySquidProxy(ctx context.Context, oc *exutil.CLI) (httpProxyURL, httpsProxyURL string, caCertPEM []byte, namespace string, cleanup removalFunc, err error) { + kubeClient := oc.AdminKubeClient() + + nsLabels := componentProxyTestLabels() + nsLabels["pod-security.kubernetes.io/enforce"] = "baseline" + nsLabels["security.openshift.io/scc.podSecurityLabelSync"] = "false" + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "e2e-proxy-", + Labels: nsLabels, + }, + } + created, err := kubeClient.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + if err != nil { + return "", "", nil, "", nil, fmt.Errorf("creating Squid proxy namespace: %w", err) + } + namespace = created.Name + cleanup = func(ctx context.Context) error { + g.GinkgoWriter.Println("cleanup: removing Squid proxy namespace") + err := kubeClient.CoreV1().Namespaces().Delete(ctx, namespace, metav1.DeleteOptions{}) + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + success := false + defer func() { + if !success { + _ = cleanup(ctx) + cleanup = nil + } + }() + + ca := mustNewCertificateAuthority(nil) + serviceDNS := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) + serverCert := mustNewServerCertificate(ca, serviceDNS) + + caCertPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: ca.Certificate.Raw}) + serverCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: serverCert.Certificate.Raw}) + + serverKeyDER, err := x509.MarshalPKCS8PrivateKey(serverCert.PrivateKey) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("marshalling server private key: %w", err) + } + serverKeyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: serverKeyDER}) + + squidConfig := fmt.Sprintf(`http_port %d +https_port %d tls-cert=/etc/squid/tls/tls.crt tls-key=/etc/squid/tls/tls.key +pid_filename /tmp/squid.pid +acl all src all +http_access allow all +access_log stdio:/dev/stdout +cache_log stdio:/dev/stderr +cache deny all +buffered_logs off +`, squidHTTPPort, squidHTTPSPort) + + _, err = kubeClient.CoreV1().ConfigMaps(namespace).Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "squid-config"}, + Data: map[string]string{"squid.conf": squidConfig}, + }, metav1.CreateOptions{}) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("creating Squid config: %w", err) + } + + _, err = kubeClient.CoreV1().Secrets(namespace).Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "squid-tls"}, + Data: map[string][]byte{ + "tls.crt": serverCertPEM, + "tls.key": serverKeyPEM, + }, + }, metav1.CreateOptions{}) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("creating Squid TLS secret: %w", err) + } + + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: squidServiceName, + Labels: map[string]string{"app": squidServiceName}, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: new(int32(1)), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": squidServiceName}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": squidServiceName}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "squid", + Image: image.LocationFor(squidImage), + Ports: []corev1.ContainerPort{ + {ContainerPort: squidHTTPPort, Protocol: corev1.ProtocolTCP}, + {ContainerPort: squidHTTPSPort, Protocol: corev1.ProtocolTCP}, + }, + VolumeMounts: []corev1.VolumeMount{ + {Name: "squid-config", MountPath: "/etc/squid/squid.conf", SubPath: "squid.conf"}, + {Name: "squid-tls", MountPath: "/etc/squid/tls", ReadOnly: true}, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt32(squidHTTPPort), + }, + }, + InitialDelaySeconds: 5, + PeriodSeconds: 5, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "squid-config", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: "squid-config"}, + }, + }, + }, + { + Name: "squid-tls", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{SecretName: "squid-tls"}, + }, + }, + }, + }, + }, + }, + } + + _, err = kubeClient.AppsV1().Deployments(namespace).Create(ctx, deployment, metav1.CreateOptions{}) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("creating Squid deployment: %w", err) + } + + _, err = kubeClient.CoreV1().Services(namespace).Create(ctx, &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: squidServiceName, + Labels: map[string]string{"app": squidServiceName}, + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": squidServiceName}, + Ports: []corev1.ServicePort{ + {Name: "http", Port: squidHTTPPort, TargetPort: intstr.FromInt32(squidHTTPPort), Protocol: corev1.ProtocolTCP}, + {Name: "https", Port: squidHTTPSPort, TargetPort: intstr.FromInt32(squidHTTPSPort), Protocol: corev1.ProtocolTCP}, + }, + }, + }, metav1.CreateOptions{}) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("creating Squid service: %w", err) + } + + g.GinkgoWriter.Printf("waiting for Squid proxy deployment in %s to be ready\n", namespace) + timeLimitedCtx, cancel := context.WithTimeout(ctx, 5*time.Minute) + defer cancel() + _, err = watchtools.UntilWithSync(timeLimitedCtx, + cache.NewListWatchFromClient( + kubeClient.AppsV1().RESTClient(), "deployments", namespace, + fields.OneTermEqualSelector("metadata.name", squidServiceName)), + &appsv1.Deployment{}, + nil, + func(event watch.Event) (bool, error) { + if event.Type == watch.Error { + return false, fmt.Errorf("Squid deployment watch error: %w", event.Object) + } + if event.Type == watch.Bookmark { + return false, nil + } + d, ok := event.Object.(*appsv1.Deployment) + if !ok { + return false, nil + } + return d.Status.ReadyReplicas > 0, nil + }, + ) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("Squid proxy deployment did not become ready: %w", err) + } + + serviceHost := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) + httpProxyURL = fmt.Sprintf("http://%s:%d", serviceHost, squidHTTPPort) + httpsProxyURL = fmt.Sprintf("https://%s:%d", serviceHost, squidHTTPSPort) + success = true + g.GinkgoWriter.Printf("Squid proxy deployed: http=%s https=%s\n", httpProxyURL, httpsProxyURL) + return httpProxyURL, httpsProxyURL, caCertPEM, namespace, cleanup, nil +} + +func getSquidProxyLogs(ctx context.Context, oc *exutil.CLI, namespace string) (string, error) { + return getSquidProxyLogsSince(ctx, oc, namespace, time.Time{}) +} + +func getSquidProxyLogsSince(ctx context.Context, oc *exutil.CLI, namespace string, since time.Time) (string, error) { + kubeClient := oc.AdminKubeClient() + + pods, err := kubeClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("app=%s", squidServiceName), + }) + if err != nil { + return "", fmt.Errorf("listing squid pods in %s: %w", namespace, err) + } + if len(pods.Items) == 0 { + return "", fmt.Errorf("no squid proxy pods found in namespace %s", namespace) + } + + logOpts := &corev1.PodLogOptions{Container: "squid"} + if !since.IsZero() { + t := metav1.NewTime(since) + logOpts.SinceTime = &t + } + logBytes, err := kubeClient.CoreV1().Pods(namespace).GetLogs(pods.Items[0].Name, logOpts).DoRaw(ctx) + if err != nil { + return "", fmt.Errorf("getting logs from squid container: %w", err) + } + + return string(logBytes), nil +} + +func waitForSquidProxyTraffic(ctx context.Context, oc *exutil.CLI, namespace string, timeout time.Duration) error { + g.GinkgoWriter.Printf("waiting up to %s for traffic in squid proxy logs\n", timeout) + return wait.PollUntilContextTimeout(ctx, 10*time.Second, timeout, true, func(ctx context.Context) (bool, error) { + logs, err := getSquidProxyLogs(ctx, oc, namespace) + if err != nil { + g.GinkgoWriter.Printf("failed to read squid logs: %v\n", err) + return false, nil + } + if strings.Contains(logs, "CONNECT") || strings.Contains(logs, "TCP_") { + g.GinkgoWriter.Println("detected proxy traffic in squid logs") + return true, nil + } + return false, nil + }) +} + +// keycloakProxySetup holds the results of deploying Keycloak for proxy tests, +// before the IdP is registered in OpenShift. +type keycloakProxySetup struct { + client *keycloakClient + idpName string + namespace string + clientID string + clientSecret string + issuerURL string +} + +func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxySetup, []removalFunc, error) { + namespace := fmt.Sprintf("e2e-proxy-kc-%s", rand.String(8)) + cleanups, err := deployKeycloak(ctx, oc, namespace, g.GinkgoLogr) + if err != nil { + return nil, nil, fmt.Errorf("deploying keycloak: %w", err) + } + + success := false + defer func() { + if !success { + _ = removeResources(ctx, cleanups...) + cleanups = nil + } + }() + + setup := &keycloakProxySetup{ + idpName: fmt.Sprintf("keycloak-proxy-test-%s", namespace), + namespace: namespace, + } + + // Use the route for admin API calls (the test runner may be external). + routeURL, err := admittedURLForRoute(ctx, oc, keycloakResourceName, namespace) + if err != nil { + return nil, nil, fmt.Errorf("getting keycloak route URL: %w", err) + } + + kcClient, err := keycloakClientFor(routeURL) + if err != nil { + return nil, nil, fmt.Errorf("creating keycloak client: %w", err) + } + setup.client = kcClient + + // Use the in-cluster service URL as the OIDC issuer so the operator must + // go through the proxy to reach Keycloak (enforced by network policy). + setup.issuerURL = fmt.Sprintf("https://%s.%s.svc:%d/realms/master", keycloakResourceName, namespace, keycloakHTTPSPort) + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + err := kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword) + if err != nil { + g.GinkgoWriter.Printf("failed to authenticate to Keycloak: %v\n", err) + return false, nil + } + return true, nil + }) + if err != nil { + return nil, nil, fmt.Errorf("authenticating to keycloak: %w", err) + } + + clientList, err := kcClient.ListClients() + if err != nil { + return nil, nil, fmt.Errorf("listing keycloak clients: %w", err) + } + + var adminClientID, passwdClientID string + for _, c := range clientList { + if c.ClientID == "admin-cli" { + adminClientID = c.ID + } else if len(c.RedirectURIs) > 0 { + passwdClientID = c.ID + setup.clientID = c.ClientID + } + if len(passwdClientID) > 0 && len(adminClientID) > 0 { + break + } + } + + if adminClientID == "" { + return nil, nil, fmt.Errorf("admin-cli client not found in keycloak") + } + if passwdClientID == "" { + return nil, nil, fmt.Errorf("password-grant client (with redirectUris) not found in keycloak") + } + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + err := kcClient.UpdateClientAccessTokenTimeout(adminClientID, 60*30) + if err != nil { + g.GinkgoWriter.Printf("failed to update client access token timeout: %v, retrying\n", err) + if authErr := kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword); authErr != nil { + g.GinkgoWriter.Printf("failed to re-authenticate: %v\n", authErr) + } + return false, nil + } + return true, nil + }) + if err != nil { + return nil, nil, fmt.Errorf("updating admin-cli access token timeout: %w", err) + } + + err = kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword) + if err != nil { + return nil, nil, fmt.Errorf("re-authenticating to keycloak: %w", err) + } + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + var err error + setup.clientSecret, err = kcClient.RegenerateClientSecret(passwdClientID) + if err != nil { + g.GinkgoWriter.Printf("failed to regenerate client secret: %v, retrying\n", err) + if authErr := kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword); authErr != nil { + g.GinkgoWriter.Printf("failed to re-authenticate: %v\n", authErr) + } + return false, nil + } + return true, nil + }) + if err != nil { + return nil, nil, fmt.Errorf("regenerating client secret: %w", err) + } + + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + err := kcClient.CreateClientGroupMapper(passwdClientID, "test-groups-mapper", "groups") + if err != nil { + g.GinkgoWriter.Printf("failed to create client group mapper: %v, retrying\n", err) + if authErr := kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword); authErr != nil { + g.GinkgoWriter.Printf("failed to re-authenticate: %v\n", authErr) + } + return false, nil + } + return true, nil + }) + if err != nil { + return nil, nil, fmt.Errorf("creating client group mapper: %w", err) + } + + success = true + return setup, cleanups, nil +} + +func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keycloakProxySetup) ([]removalFunc, error) { + var cleanups []removalFunc + kubeClient := oc.AdminKubeClient() + + success := false + defer func() { + if !success { + _ = removeResources(ctx, cleanups...) + cleanups = nil + } + }() + + secretName := setup.idpName + "-secret" + _, err := kubeClient.CoreV1().Secrets("openshift-config").Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Labels: componentProxyTestLabels(), + }, + Data: map[string][]byte{ + "clientSecret": []byte(setup.clientSecret), + }, + }, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("creating keycloak client secret: %w", err) + } + cleanups = append(cleanups, func(ctx context.Context) error { + return kubeClient.CoreV1().Secrets("openshift-config").Delete(ctx, secretName, metav1.DeleteOptions{}) + }) + + caCMName := setup.idpName + "-ca" + caCleanup, err := createServiceCAConfigMap(ctx, oc, caCMName) + if err != nil { + return nil, fmt.Errorf("syncing default ingress CA: %w", err) + } + cleanups = append(cleanups, caCleanup) + + err = addIdentityProvider(ctx, oc, configv1.IdentityProvider{ + Name: setup.idpName, + MappingMethod: configv1.MappingMethodClaim, + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: configv1.IdentityProviderTypeOpenID, + OpenID: &configv1.OpenIDIdentityProvider{ + ClientID: setup.clientID, + ClientSecret: configv1.SecretNameReference{ + Name: secretName, + }, + ExtraScopes: []string{"profile", "email"}, + Claims: configv1.OpenIDClaims{ + PreferredUsername: []string{"preferred_username"}, + Groups: []configv1.OpenIDClaim{"groups"}, + }, + Issuer: setup.issuerURL, + CA: configv1.ConfigMapNameReference{ + Name: caCMName, + }, + }, + }, + }) + if err != nil { + return nil, fmt.Errorf("adding identity provider: %w", err) + } + + // Prepend IdP removal so it runs before CA/secret deletion — + // the operator must not see an IdP referencing a deleted CA. + cleanups = append([]removalFunc{func(ctx context.Context) error { + cleanIdentityProviderByName(ctx, oc, setup.idpName) + return nil + }}, cleanups...) + + success = true + return cleanups, nil +} + +// createServiceCAConfigMap copies the service-ca CA bundle into a new ConfigMap +// in openshift-config under the "ca.crt" key expected by the OAuth server for +// IdP CA references. The service CA signs serving certs for services annotated +// with service.beta.openshift.io/serving-cert-secret-name. +func createServiceCAConfigMap(ctx context.Context, oc *exutil.CLI, name string) (removalFunc, error) { + kubeClient := oc.AdminKubeClient() + + serviceCA, err := kubeClient.CoreV1().ConfigMaps("openshift-authentication").Get(ctx, "v4-0-config-system-service-ca", metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("getting openshift-authentication/v4-0-config-system-service-ca: %w", err) + } + caBundle := serviceCA.Data["service-ca.crt"] + if len(caBundle) == 0 { + return nil, fmt.Errorf("service-ca.crt is empty in openshift-authentication/v4-0-config-system-service-ca") + } + + _, err = kubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: componentProxyTestLabels(), + }, + Data: map[string]string{ + "ca.crt": caBundle, + }, + }, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("creating configmap openshift-config/%s: %w", name, err) + } + + return func(ctx context.Context) error { + return kubeClient.CoreV1().ConfigMaps("openshift-config").Delete(ctx, name, metav1.DeleteOptions{}) + }, nil +} + +// deployProxyNetworkPolicies restricts ingress to the Keycloak namespace so that +// only the proxy namespace can reach Keycloak pods. This forces the operator to +// go through the Squid proxy to contact the OIDC issuer. +// +// All Keycloak admin API setup (via the route) must happen before this policy is +// applied, since it blocks the ingress router as well. +func deployProxyNetworkPolicies(ctx context.Context, oc *exutil.CLI, proxyNamespace, keycloakNamespace string) (removalFunc, error) { + kubeClient := oc.AdminKubeClient() + + keycloakPolicy := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-e2e-allow-only-from-proxy", + Namespace: keycloakNamespace, + Labels: componentProxyTestLabels(), + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kubernetes.io/metadata.name": proxyNamespace, + }, + }, + }, + }, + }, + }, + }, + } + + _, err := kubeClient.NetworkingV1().NetworkPolicies(keycloakNamespace).Create(ctx, keycloakPolicy, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("creating NetworkPolicy in %s: %w", keycloakNamespace, err) + } + g.GinkgoWriter.Printf("created NetworkPolicy proxy-e2e-allow-only-from-proxy in %s\n", keycloakNamespace) + + return func(ctx context.Context) error { + err := kubeClient.NetworkingV1().NetworkPolicies(keycloakNamespace).Delete(ctx, "proxy-e2e-allow-only-from-proxy", metav1.DeleteOptions{}) + if apierrors.IsNotFound(err) { + return nil + } + return err + }, nil +} + +func updateAuthenticationProxy(ctx context.Context, oc *exutil.CLI, proxy operatorv1.AuthenticationProxyConfig) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + auth, err := oc.AdminOperatorClient().OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return err + } + auth.Spec.Proxy = proxy + _, err = oc.AdminOperatorClient().OperatorV1().Authentications().Update(ctx, auth, metav1.UpdateOptions{}) + return err + }) +} + +func addIdentityProvider(ctx context.Context, oc *exutil.CLI, idp configv1.IdentityProvider) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + oauth, err := oc.AdminConfigClient().ConfigV1().OAuths().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return err + } + oauth.Spec.IdentityProviders = append(oauth.Spec.IdentityProviders, idp) + _, err = oc.AdminConfigClient().ConfigV1().OAuths().Update(ctx, oauth, metav1.UpdateOptions{}) + return err + }) +} + +func cleanIdentityProviderByName(ctx context.Context, oc *exutil.CLI, idpName string) { + oauthClient := oc.AdminConfigClient().ConfigV1().OAuths() + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + config, err := oauthClient.Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return err + } + + idpIndex := -1 + for i, idp := range config.Spec.IdentityProviders { + if idp.Name == idpName { + idpIndex = i + break + } + } + if idpIndex < 0 { + return nil + } + + config.Spec.IdentityProviders = append(config.Spec.IdentityProviders[:idpIndex], config.Spec.IdentityProviders[idpIndex+1:]...) + _, err = oauthClient.Update(ctx, config, metav1.UpdateOptions{}) + return err + }) + if err != nil { + g.GinkgoWriter.Printf("cleanup: failed to remove IdP %q from oauth/cluster: %v\n", idpName, err) + } +} + +func verifyOAuthServerDeploymentProxyConfig(ctx context.Context, oc *exutil.CLI, expectedHTTPProxy, expectedHTTPSProxy, expectedNoProxy string, expectTrustedCAVolume bool) error { + kubeClient := oc.AdminKubeClient() + + return wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + deployment, err := kubeClient.AppsV1().Deployments("openshift-authentication").Get(ctx, "oauth-openshift", metav1.GetOptions{}) + if err != nil { + g.GinkgoWriter.Printf("failed to get oauth-openshift deployment: %v\n", err) + return false, nil + } + + envVars := make(map[string]string) + for _, container := range deployment.Spec.Template.Spec.Containers { + for _, env := range container.Env { + switch env.Name { + case "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY": + envVars[env.Name] = env.Value + } + } + } + + if envVars["HTTP_PROXY"] != expectedHTTPProxy || envVars["HTTPS_PROXY"] != expectedHTTPSProxy { + g.GinkgoWriter.Printf("proxy env mismatch: HTTP_PROXY=%q (want %q), HTTPS_PROXY=%q (want %q)\n", + envVars["HTTP_PROXY"], expectedHTTPProxy, envVars["HTTPS_PROXY"], expectedHTTPSProxy) + return false, nil + } + if expectedNoProxy == "" { + if envVars["NO_PROXY"] != "" { + g.GinkgoWriter.Printf("proxy env mismatch: NO_PROXY=%q (want empty)\n", envVars["NO_PROXY"]) + return false, nil + } + } else { + actualNoProxy := sets.New[string](strings.Split(envVars["NO_PROXY"], ",")...) + expectedNoProxyEntries := sets.New[string](strings.Split(expectedNoProxy, ",")...) + if !actualNoProxy.IsSuperset(expectedNoProxyEntries) { + g.GinkgoWriter.Printf("proxy env mismatch: NO_PROXY=%q does not contain all of %q\n", envVars["NO_PROXY"], expectedNoProxy) + return false, nil + } + } + + if !matchTrustedCAVolume(deployment, expectTrustedCAVolume) { + g.GinkgoWriter.Printf("trustedCA volume/mount present=%v (want present=%v)\n", !expectTrustedCAVolume, expectTrustedCAVolume) + return false, nil + } + return true, nil + }) +} + +func matchTrustedCAVolume(deployment *appsv1.Deployment, expectPresent bool) bool { + foundVolume := false + for _, vol := range deployment.Spec.Template.Spec.Volumes { + if vol.ConfigMap != nil && vol.ConfigMap.Name == componentProxyCAConfigMapName { + foundVolume = true + break + } + } + + foundMount := false + for _, container := range deployment.Spec.Template.Spec.Containers { + for _, mount := range container.VolumeMounts { + if mount.Name == componentProxyCAConfigMapName { + foundMount = true + break + } + } + } + + if expectPresent { + return foundVolume && foundMount + } + return !foundVolume && !foundMount +} + +func verifyTrustedCAConfigMapSynced(ctx context.Context, oc *exutil.CLI) error { + kubeClient := oc.AdminKubeClient() + + return wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + cm, err := kubeClient.CoreV1().ConfigMaps("openshift-authentication").Get(ctx, componentProxyCAConfigMapName, metav1.GetOptions{}) + if err != nil { + return false, nil + } + return len(cm.Data) > 0, nil + }) +} diff --git a/test/extended/authentication/crypto_helpers.go b/test/extended/authentication/crypto_helpers.go new file mode 100644 index 000000000000..e5f31bc78a65 --- /dev/null +++ b/test/extended/authentication/crypto_helpers.go @@ -0,0 +1,89 @@ +package authentication + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "math" + "math/big" + "time" +) + +type cryptoMaterials struct { + PrivateKey *rsa.PrivateKey + Certificate *x509.Certificate +} + +func mustNewServerCertificate(signer *cryptoMaterials, hosts ...string) *cryptoMaterials { + var server cryptoMaterials + var err error + if server.PrivateKey, err = rsa.GenerateKey(rand.Reader, 2048); err != nil { + panic(err) + } + + serialNumber, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64)) + if err != nil { + panic(err) + } + + template := &x509.Certificate{ + Subject: pkix.Name{CommonName: "server"}, + NotBefore: time.Now().AddDate(-1, 0, 0), + NotAfter: time.Now().AddDate(1, 0, 0), + SignatureAlgorithm: x509.SHA256WithRSA, + SerialNumber: serialNumber, + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + DNSNames: hosts, + } + der, err := x509.CreateCertificate(rand.Reader, template, signer.Certificate, server.PrivateKey.Public(), signer.PrivateKey) + if err != nil { + panic(err) + } + + if server.Certificate, err = x509.ParseCertificate(der); err != nil { + panic(err) + } + return &server +} + +func mustNewCertificateAuthority(parent *cryptoMaterials) *cryptoMaterials { + var ca cryptoMaterials + var err error + if ca.PrivateKey, err = rsa.GenerateKey(rand.Reader, 2048); err != nil { + panic(err) + } + + serialNumber, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64)) + if err != nil { + panic(err) + } + + template := &x509.Certificate{ + Subject: pkix.Name{CommonName: "ca"}, + NotBefore: time.Now().AddDate(-1, 0, 0), + NotAfter: time.Now().AddDate(1, 0, 0), + SignatureAlgorithm: x509.SHA256WithRSA, + SerialNumber: serialNumber, + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + BasicConstraintsValid: true, + IsCA: true, + } + signerCertificate := template + signerPrivateKey := ca.PrivateKey + if parent != nil { + signerCertificate = parent.Certificate + signerPrivateKey = parent.PrivateKey + } + der, err := x509.CreateCertificate(rand.Reader, template, signerCertificate, ca.PrivateKey.Public(), signerPrivateKey) + if err != nil { + panic(err) + } + + if ca.Certificate, err = x509.ParseCertificate(der); err != nil { + panic(err) + } + return &ca +} diff --git a/test/extended/authentication/keycloak_client.go b/test/extended/authentication/keycloak_client.go index a91283139f30..aff858b6be28 100644 --- a/test/extended/authentication/keycloak_client.go +++ b/test/extended/authentication/keycloak_client.go @@ -6,8 +6,10 @@ import ( "encoding/json" "fmt" "io" + "maps" "net/http" "net/url" + "strconv" "k8s.io/apimachinery/pkg/runtime" ) @@ -194,6 +196,96 @@ func (kc *keycloakClient) DoRequest(method, url, contentType string, authenticat return kc.client.Do(req) } +func (kc *keycloakClient) RegenerateClientSecret(id string) (string, error) { + regenURL := *kc.adminURL + regenURL.Path += fmt.Sprintf("/clients/%s/client-secret", id) + + resp, err := kc.DoRequest(http.MethodPost, regenURL.String(), runtime.ContentTypeJSON, true, nil) + if err != nil { + return "", err + } + defer resp.Body.Close() + + respBytes, err := io.ReadAll(resp.Body) + if err != nil { + return "", err + } + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("regenerating client %q secret failed: %s - %s", id, resp.Status, respBytes) + } + + secret := map[string]string{} + if err = json.Unmarshal(respBytes, &secret); err != nil { + return "", err + } + + secretVal, ok := secret["value"] + if !ok { + return "", fmt.Errorf("failed to retrieve new secret for client %q", id) + } + + return secretVal, nil +} + +func (kc *keycloakClient) UpdateClientAccessTokenTimeout(id string, timeout int32) error { + return kc.UpdateClientRaw(id, map[string]any{ + "attributes": map[string]any{ + "access.token.lifespan": strconv.FormatInt(int64(timeout), 10), + }, + }) +} + +func (kc *keycloakClient) UpdateClientRaw(id string, changes map[string]any) error { + existing, err := kc.GetClientRaw(id) + if err != nil { + return err + } + + maps.Copy(existing, changes) + + var body bytes.Buffer + if err := json.NewEncoder(&body).Encode(existing); err != nil { + return err + } + + clientURL := *kc.adminURL + clientURL.Path += fmt.Sprintf("/clients/%s", id) + resp, err := kc.DoRequest(http.MethodPut, clientURL.String(), runtime.ContentTypeJSON, true, &body) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent { + respBytes, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed updating client %q: %s - %s", id, resp.Status, respBytes) + } + return nil +} + +func (kc *keycloakClient) GetClientRaw(id string) (map[string]any, error) { + clientURL := *kc.adminURL + clientURL.Path += fmt.Sprintf("/clients/%s", id) + + resp, err := kc.DoRequest(http.MethodGet, clientURL.String(), runtime.ContentTypeJSON, true, nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + respBytes, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("getting client %q failed: %s - %s", id, resp.Status, respBytes) + } + + result := map[string]any{} + err = json.Unmarshal(respBytes, &result) + return result, err +} + func (kc *keycloakClient) AccessToken() string { return kc.accessToken } @@ -345,8 +437,9 @@ func (kc *keycloakClient) CreateClientAudienceMapper(clientId, name string) erro } type client struct { - ClientID string `json:"clientID"` - ID string `json:"id"` + ClientID string `json:"clientId"` + ID string `json:"id"` + RedirectURIs []string `json:"redirectUris"` } // ListClients retrieves all clients diff --git a/test/extended/authentication/keycloak_helpers.go b/test/extended/authentication/keycloak_helpers.go index a8c2465f6130..0695c887947f 100644 --- a/test/extended/authentication/keycloak_helpers.go +++ b/test/extended/authentication/keycloak_helpers.go @@ -41,47 +41,51 @@ const ( ) func deployKeycloak(ctx context.Context, client *exutil.CLI, namespace string, logger logr.Logger) ([]removalFunc, error) { - cleanups := []removalFunc{} - corev1Client := client.AdminKubeClient().CoreV1() - cleanup, err := createKeycloakNamespace(ctx, corev1Client.Namespaces(), namespace) + nsCleanup, err := createKeycloakNamespace(ctx, corev1Client.Namespaces(), namespace) if err != nil { - return cleanups, fmt.Errorf("creating namespace for keycloak: %w", err) + return nil, fmt.Errorf("creating namespace for keycloak: %w", err) } - cleanups = append(cleanups, cleanup) + cleanups := []removalFunc{nsCleanup} - cleanup, err = createKeycloakServiceAccount(ctx, corev1Client.ServiceAccounts(namespace)) - if err != nil { - return cleanups, fmt.Errorf("creating serviceaccount for keycloak: %w", err) + success := false + defer func() { + if !success { + _ = removeResources(ctx, cleanups...) + cleanups = nil + } + }() + + if _, err = createKeycloakServiceAccount(ctx, corev1Client.ServiceAccounts(namespace)); err != nil { + return nil, fmt.Errorf("creating serviceaccount for keycloak: %w", err) } - cleanups = append(cleanups, cleanup) - service, cleanup, err := createKeycloakService(ctx, corev1Client.Services(namespace)) + service, _, err := createKeycloakService(ctx, corev1Client.Services(namespace)) if err != nil { - return cleanups, fmt.Errorf("creating service for keycloak: %w", err) + return nil, fmt.Errorf("creating service for keycloak: %w", err) } - cleanups = append(cleanups, cleanup) - cleanup, err = createKeycloakDeployment(ctx, client.AdminKubeClient().AppsV1().Deployments(namespace)) - if err != nil { - return cleanups, fmt.Errorf("creating deployment for keycloak: %w", err) + if _, err = createKeycloakDeployment(ctx, client.AdminKubeClient().AppsV1().Deployments(namespace)); err != nil { + return nil, fmt.Errorf("creating deployment for keycloak: %w", err) } - cleanups = append(cleanups, cleanup) - cleanup, err = createKeycloakRoute(ctx, service, client.AdminRouteClient().RouteV1().Routes(namespace)) - if err != nil { - return cleanups, fmt.Errorf("creating route for keycloak: %w", err) + if _, err = createKeycloakRoute(ctx, service, client.AdminRouteClient().RouteV1().Routes(namespace)); err != nil { + return nil, fmt.Errorf("creating route for keycloak: %w", err) } - cleanups = append(cleanups, cleanup) - cleanup, err = createKeycloakCAConfigMap(ctx, corev1Client) + caCleanup, err := createKeycloakCAConfigMap(ctx, corev1Client) if err != nil { - return cleanups, fmt.Errorf("creating CA configmap for keycloak: %w", err) + return nil, fmt.Errorf("creating CA configmap for keycloak: %w", err) + } + cleanups = append(cleanups, caCleanup) + + if err := waitForKeycloakAvailable(ctx, client, namespace, logger); err != nil { + return nil, err } - cleanups = append(cleanups, cleanup) - return cleanups, waitForKeycloakAvailable(ctx, client, namespace, logger) + success = true + return cleanups, nil } func createKeycloakNamespace(ctx context.Context, client typedcorev1.NamespaceInterface, namespace string) (removalFunc, error) { diff --git a/test/extended/authentication/operator_status_helpers.go b/test/extended/authentication/operator_status_helpers.go new file mode 100644 index 000000000000..217bfa9c02d3 --- /dev/null +++ b/test/extended/authentication/operator_status_helpers.go @@ -0,0 +1,77 @@ +package authentication + +import ( + "context" + "time" + + g "github.com/onsi/ginkgo/v2" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + + configv1 "github.com/openshift/api/config/v1" + configv1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers" + + exutil "github.com/openshift/origin/test/extended/util" +) + +func waitForOperatorToPickUpChanges(ctx context.Context, oc *exutil.CLI, name string) error { + progressCtx, cancel := context.WithTimeout(ctx, 5*time.Minute) + defer cancel() + if err := exutil.WaitForOperatorProgressingTrue(progressCtx, oc.AdminConfigClient(), name); err != nil { + g.GinkgoWriter.Printf("operator %s did not become Progressing=True (may have reconciled quickly): %v\n", name, err) + } + return waitForClusterOperatorAvailableNotProgressingNotDegraded(ctx, oc, name) +} + +func waitForClusterOperatorAvailableNotProgressingNotDegraded(ctx context.Context, oc *exutil.CLI, name string) error { + return waitForClusterOperatorStatus(ctx, oc, name, + configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorAvailable, Status: configv1.ConditionTrue}, + configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse}, + configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorDegraded, Status: configv1.ConditionFalse}, + ) +} + +// waitForClusterOperatorStatus polls until all requiredConditions are met on a +// single named ClusterOperator. Unlike WaitForOperatorsToSettle (which checks +// all operators for the fixed Available+NotProgressing+NotDegraded state), this +// waits for an arbitrary set of conditions on one operator — needed for tests +// that assert Degraded=True or check a subset of conditions. +func waitForClusterOperatorStatus(ctx context.Context, oc *exutil.CLI, name string, requiredConditions ...configv1.ClusterOperatorStatusCondition) error { + var conditions []configv1.ClusterOperatorStatusCondition + + ts := time.Now() + err := wait.PollUntilContextTimeout(ctx, 10*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { + done, conds, checkErr := checkClusterOperatorStatus(ctx, oc, name, requiredConditions...) + if checkErr != nil { + g.GinkgoWriter.Printf("error checking clusteroperator/%s, will retry: %v\n", name, checkErr) + return false, nil + } + conditions = conds + return done, nil + }) + + if err == nil { + g.GinkgoWriter.Printf("clusteroperator/%s reached required status after %s\n", name, time.Since(ts)) + return nil + } + + g.GinkgoWriter.Printf("clusteroperator/%s did not reach required status after %s, current: %v\n", name, time.Since(ts), conditions) + return err +} + +func checkClusterOperatorStatus(ctx context.Context, oc *exutil.CLI, name string, requiredConditions ...configv1.ClusterOperatorStatusCondition) (bool, []configv1.ClusterOperatorStatusCondition, error) { + clusterOperator, err := oc.AdminConfigClient().ConfigV1().ClusterOperators().Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return false, nil, err + } + + conditions := clusterOperator.Status.Conditions + for _, required := range requiredConditions { + if len(required.Status) > 0 && !configv1helpers.IsStatusConditionPresentAndEqual(conditions, required.Type, required.Status) { + return false, conditions, nil + } + } + + return true, conditions, nil +} From 07369cb4742efba91864638718d7db5d34fdbe9b Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Thu, 30 Jul 2026 15:08:09 +0200 Subject: [PATCH 02/16] Add component-scoped proxy e2e tests with shared setup Use BeforeEach to deploy Squid proxy, Keycloak, and save/restore the proxy config, with DeferCleanup for teardown. Each test only handles its own IdP registration, network policies, and verification. Three tests validate: - OIDC IdP discovery through an HTTP proxy - OIDC IdP discovery through an HTTPS proxy with trustedCA - Fallback on spec.proxy removal (deletes the proxy to prove the operator no longer routes through it) --- .../authentication/component_proxy.go | 175 ++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100644 test/extended/authentication/component_proxy.go diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go new file mode 100644 index 000000000000..d1f1adf62d6e --- /dev/null +++ b/test/extended/authentication/component_proxy.go @@ -0,0 +1,175 @@ +package authentication + +import ( + "context" + "time" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + operatorv1 "github.com/openshift/api/operator/v1" + + exutil "github.com/openshift/origin/test/extended/util" +) + +var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGate:AuthenticationComponentProxy][Serial][Slow]", func() { + oc := exutil.NewCLIWithoutNamespace("component-proxy") + + var ( + ctx context.Context + httpProxyURL string + httpsProxyURL string + caCertPEM []byte + proxyNamespace string + kcSetup *keycloakProxySetup + ) + + g.BeforeEach(func() { + ctx = context.Background() + + g.By("Resetting component-scoped proxy state") + resetComponentProxyState(ctx, oc) + + g.By("Deploying Squid forward proxy") + var proxyCleanup removalFunc + var err error + httpProxyURL, httpsProxyURL, caCertPEM, proxyNamespace, proxyCleanup, err = deploySquidProxy(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(proxyCleanup) + + g.By("Deploying Keycloak (without registering IdP yet)") + var kcCleanups []removalFunc + kcSetup, kcCleanups, err = deployKeycloakForProxy(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(func() { + g.GinkgoWriter.Println("cleanup: removing Keycloak resources") + _ = removeResources(ctx, kcCleanups...) + }) + + // We wait for the operator a bit later in the startup sequence so that we can avoid + // unblocking too fast after resetComponentProxyState. We don't know whether there is any state change, + // so we can't really use waitForOperatorToPickUpChanges here. + g.By("Waiting for authentication operator to be stable before test") + err = waitForClusterOperatorAvailableNotProgressingNotDegraded(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Saving original proxy config for restore after test") + proxyRestore, err := saveAndRestoreProxyConfig(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(proxyRestore) + + g.GinkgoWriter.Printf("Squid proxy URL: http=%s https=%s\n", httpProxyURL, httpsProxyURL) + g.GinkgoWriter.Printf("Keycloak issuer URL: %s\n", kcSetup.issuerURL) + g.GinkgoWriter.Printf("Keycloak namespace: %s\n", kcSetup.namespace) + }) + + g.It("should validate OIDC IdP through component proxy", func() { + testOIDCIdPThroughComponentProxy(ctx, oc, kcSetup, httpProxyURL, nil, proxyNamespace) + }) + g.It("should validate OIDC IdP through component proxy with trustedCA", func() { + testOIDCIdPThroughComponentProxy(ctx, oc, kcSetup, httpsProxyURL, caCertPEM, proxyNamespace) + }) + g.It("should fall back on spec.proxy removal", func() { + testFallbackOnProxyRemoval(ctx, oc, kcSetup, httpProxyURL, proxyNamespace) + }) +}) + +func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSetup *keycloakProxySetup, proxyURL string, trustedCACertPEM []byte, proxyNamespace string) { + withTrustedCA := len(trustedCACertPEM) > 0 + + const trustedCAConfigMapName = "e2e-proxy-ca" + if withTrustedCA { + g.By("Creating trustedCA ConfigMap in openshift-config") + _, err := oc.AdminKubeClient().CoreV1().ConfigMaps("openshift-config").Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustedCAConfigMapName, + Labels: componentProxyTestLabels(), + }, + Data: map[string]string{ + "ca-bundle.crt": string(trustedCACertPEM), + }, + }, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(func(ctx context.Context) error { + return oc.AdminKubeClient().CoreV1().ConfigMaps("openshift-config").Delete(ctx, trustedCAConfigMapName, metav1.DeleteOptions{}) + }) + } + + g.By("Deploying NetworkPolicy to restrict Keycloak ingress to proxy namespace only") + networkPolicyCleanup, err := deployProxyNetworkPolicies(ctx, oc, proxyNamespace, kcSetup.namespace) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(networkPolicyCleanup) + + g.By("Setting component-scoped proxy") + proxyConfig := operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: proxyURL, + } + if withTrustedCA { + proxyConfig.TrustedCA = operatorv1.AuthenticationConfigMapReference{Name: trustedCAConfigMapName} + } + err = updateAuthenticationProxy(ctx, oc, proxyConfig) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Registering Keycloak as OIDC IdP (operator discovers it through the proxy)") + idpCleanups, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(func() { + _ = removeResources(ctx, idpCleanups...) + }) + + g.By("Waiting for operator to pick up IdP changes and stabilize") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying OAuth server deployment has proxy env vars and trustedCA volume/mount") + err = verifyOAuthServerDeploymentProxyConfig(ctx, oc, "", proxyURL, ".cluster.local,.svc,127.0.0.1,localhost", withTrustedCA) + o.Expect(err).NotTo(o.HaveOccurred()) + + if withTrustedCA { + g.By("Verifying trustedCA ConfigMap was synced to openshift-authentication") + err = verifyTrustedCAConfigMapSynced(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred()) + } + + g.By("Verifying traffic went through the Squid proxy") + err = waitForSquidProxyTraffic(ctx, oc, proxyNamespace, 5*time.Minute) + o.Expect(err).NotTo(o.HaveOccurred()) +} + +func testFallbackOnProxyRemoval(ctx context.Context, oc *exutil.CLI, kcSetup *keycloakProxySetup, httpProxyURL string, proxyNamespace string) { + g.By("Setting component-scoped proxy") + err := updateAuthenticationProxy(ctx, oc, operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpProxyURL, + }) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Registering Keycloak as OIDC IdP") + idpCleanups, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(func() { + _ = removeResources(ctx, idpCleanups...) + }) + + g.By("Waiting for operator to pick up IdP changes and stabilize") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Removing spec.proxy from Authentication CR") + err = updateAuthenticationProxy(ctx, oc, operatorv1.AuthenticationProxyConfig{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Deleting Squid to prove the operator no longer routes through it") + err = oc.AdminKubeClient().CoreV1().Namespaces().Delete(ctx, proxyNamespace, metav1.DeleteOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to pick up proxy removal and stabilize") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying proxy env vars are no longer set on OAuth server deployment") + err = verifyOAuthServerDeploymentProxyConfig(ctx, oc, "", "", "", false) + o.Expect(err).NotTo(o.HaveOccurred()) +} From d12c3dc660105c231ba2dc8d8c0031f76b5339c6 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 13:16:36 +0200 Subject: [PATCH 03/16] Improve proxy e2e test robustness and cleanup - Use Keycloak service URL as OIDC issuer instead of route URL so the network policy structurally enforces proxy usage (only the proxy namespace can reach Keycloak pods directly). - Remove ingress router rule from network policy since the service URL bypasses the router. - Use service CA for IdP CA configmap instead of ingress CA, matching the service serving cert signer. - Replace saveAndRestoreProxyConfig and saveAndRestoreIdPs with a single saveAndRestoreAuthState that snapshots both the Authentication operator CR and oauth/cluster, restoring both on cleanup and waiting for stabilization only if changes were made. - Remove ListClientsRaw in favor of typed ListClients with RedirectURIs field; fix clientId JSON tag; simplify UpdateClientRaw to flat merge. - Add must prefix to crypto helpers to make panic-on-error explicit. - Remove dead code: idpCleanupWrapper, cleanIdentityProviderByName, resetComponentProxyState. --- .../authentication/component_proxy.go | 16 +- .../authentication/component_proxy_helpers.go | 141 ++++++------------ 2 files changed, 51 insertions(+), 106 deletions(-) diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go index d1f1adf62d6e..b74dc3c85909 100644 --- a/test/extended/authentication/component_proxy.go +++ b/test/extended/authentication/component_proxy.go @@ -30,12 +30,13 @@ var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGat g.BeforeEach(func() { ctx = context.Background() - g.By("Resetting component-scoped proxy state") - resetComponentProxyState(ctx, oc) + g.By("Saving auth state for restore after test") + authRestore, err := saveAndRestoreAuthState(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(authRestore) g.By("Deploying Squid forward proxy") var proxyCleanup removalFunc - var err error httpProxyURL, httpsProxyURL, caCertPEM, proxyNamespace, proxyCleanup, err = deploySquidProxy(ctx, oc) o.Expect(err).NotTo(o.HaveOccurred()) g.DeferCleanup(proxyCleanup) @@ -50,17 +51,12 @@ var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGat }) // We wait for the operator a bit later in the startup sequence so that we can avoid - // unblocking too fast after resetComponentProxyState. We don't know whether there is any state change, - // so we can't really use waitForOperatorToPickUpChanges here. + // unblocking too fast after save-and-restore snapshots. We don't know whether there + // is any state change, so we can't really use waitForOperatorToPickUpChanges here. g.By("Waiting for authentication operator to be stable before test") err = waitForClusterOperatorAvailableNotProgressingNotDegraded(ctx, oc, "authentication") o.Expect(err).NotTo(o.HaveOccurred()) - g.By("Saving original proxy config for restore after test") - proxyRestore, err := saveAndRestoreProxyConfig(ctx, oc) - o.Expect(err).NotTo(o.HaveOccurred()) - g.DeferCleanup(proxyRestore) - g.GinkgoWriter.Printf("Squid proxy URL: http=%s https=%s\n", httpProxyURL, httpsProxyURL) g.GinkgoWriter.Printf("Keycloak issuer URL: %s\n", kcSetup.issuerURL) g.GinkgoWriter.Printf("Keycloak namespace: %s\n", kcSetup.namespace) diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index 18f2a7bfd0f4..07c187475b15 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -46,84 +46,68 @@ func componentProxyTestLabels() map[string]string { } } -// resetComponentProxyState clears leftover state from killed test runs so each -// test starts clean regardless of what a previous run left behind. -func resetComponentProxyState(ctx context.Context, oc *exutil.CLI) { - err := updateAuthenticationProxy(ctx, oc, operatorv1.AuthenticationProxyConfig{}) - if err != nil { - g.GinkgoWriter.Printf("pre-test: failed to clear spec.proxy: %v\n", err) - } +// saveAndRestoreAuthState snapshots the Authentication operator CR and +// oauth/cluster, returning a cleanup function that restores both. +// If either resource was modified, it waits for the operator to stabilize. +func saveAndRestoreAuthState(ctx context.Context, oc *exutil.CLI) (removalFunc, error) { + operatorClient := oc.AdminOperatorClient() + oauthClient := oc.AdminConfigClient().ConfigV1().OAuths() - oauth, err := oc.AdminConfigClient().ConfigV1().OAuths().Get(ctx, "cluster", metav1.GetOptions{}) + auth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - g.GinkgoWriter.Printf("pre-test: failed to get oauth/cluster: %v\n", err) - return - } - var kept []configv1.IdentityProvider - for _, idp := range oauth.Spec.IdentityProviders { - if !strings.HasPrefix(idp.Name, "keycloak-proxy-test-") { - kept = append(kept, idp) - } + return nil, fmt.Errorf("getting authentication/cluster: %w", err) } - if len(kept) != len(oauth.Spec.IdentityProviders) { - oauth.Spec.IdentityProviders = kept - if _, err := oc.AdminConfigClient().ConfigV1().OAuths().Update(ctx, oauth, metav1.UpdateOptions{}); err != nil { - g.GinkgoWriter.Printf("pre-test: failed to clean leftover IdPs: %v\n", err) - } - } -} + originalAuthSpec := auth.Spec.DeepCopy() -// saveAndRestoreProxyConfig snapshots spec.proxy on the operator Authentication -// CR and returns a cleanup function that restores it. -func saveAndRestoreProxyConfig(ctx context.Context, oc *exutil.CLI) (removalFunc, error) { - operatorClient := oc.AdminOperatorClient() - - auth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + oauth, err := oauthClient.Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - return nil, fmt.Errorf("getting operator authentication CR: %w", err) + return nil, fmt.Errorf("getting oauth/cluster: %w", err) } - originalProxy := auth.Spec.Proxy.DeepCopy() + originalOAuthSpec := oauth.Spec.DeepCopy() - cleanup := func() { - g.GinkgoWriter.Println("cleanup: restoring original proxy config") + return func(ctx context.Context) error { var changed bool - err := wait.PollUntilContextTimeout(ctx, 1*time.Second, 30*time.Second, true, func(ctx context.Context) (bool, error) { + + g.GinkgoWriter.Println("cleanup: restoring authentication/cluster") + if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { fresh, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - g.GinkgoWriter.Printf("cleanup: failed to get operator auth: %v\n", err) - return false, nil + return err } - target := operatorv1.AuthenticationProxyConfig{} - if originalProxy != nil { - target = *originalProxy + if reflect.DeepEqual(fresh.Spec, *originalAuthSpec) { + return nil } - if reflect.DeepEqual(fresh.Spec.Proxy, target) { - g.GinkgoWriter.Println("cleanup: proxy config already matches original, no update needed") - return true, nil + changed = true + fresh.Spec = *originalAuthSpec + _, err = operatorClient.OperatorV1().Authentications().Update(ctx, fresh, metav1.UpdateOptions{}) + return err + }); err != nil { + g.GinkgoWriter.Printf("cleanup: failed to restore Authentication CR: %v\n", err) + } + + g.GinkgoWriter.Println("cleanup: restoring oauth/cluster") + if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + fresh, err := oauthClient.Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return err } - fresh.Spec.Proxy = target - if _, err := operatorClient.OperatorV1().Authentications().Update(ctx, fresh, metav1.UpdateOptions{}); err != nil { - g.GinkgoWriter.Printf("cleanup: failed to update operator auth (will retry): %v\n", err) - return false, nil + if reflect.DeepEqual(fresh.Spec, *originalOAuthSpec) { + return nil } changed = true - return true, nil - }) - if err != nil { - g.GinkgoWriter.Printf("cleanup: failed to restore proxy config: %v\n", err) - return - } - if !changed { - return - } - g.GinkgoWriter.Println("cleanup: waiting for operator to pick up changes and stabilize") - if err := waitForOperatorToPickUpChanges(ctx, oc, "authentication"); err != nil { - g.GinkgoWriter.Printf("cleanup: operator did not recover: %v\n", err) + fresh.Spec = *originalOAuthSpec + _, err = oauthClient.Update(ctx, fresh, metav1.UpdateOptions{}) + return err + }); err != nil { + g.GinkgoWriter.Printf("cleanup: failed to restore oauth/cluster: %v\n", err) } - } - return func(ctx context.Context) error { - cleanup() + if changed { + g.GinkgoWriter.Println("cleanup: waiting for operator to stabilize") + if err := waitForOperatorToPickUpChanges(ctx, oc, "authentication"); err != nil { + g.GinkgoWriter.Printf("cleanup: operator did not recover: %v\n", err) + } + } return nil }, nil } @@ -570,13 +554,6 @@ func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keyc return nil, fmt.Errorf("adding identity provider: %w", err) } - // Prepend IdP removal so it runs before CA/secret deletion — - // the operator must not see an IdP referencing a deleted CA. - cleanups = append([]removalFunc{func(ctx context.Context) error { - cleanIdentityProviderByName(ctx, oc, setup.idpName) - return nil - }}, cleanups...) - success = true return cleanups, nil } @@ -688,34 +665,6 @@ func addIdentityProvider(ctx context.Context, oc *exutil.CLI, idp configv1.Ident }) } -func cleanIdentityProviderByName(ctx context.Context, oc *exutil.CLI, idpName string) { - oauthClient := oc.AdminConfigClient().ConfigV1().OAuths() - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - config, err := oauthClient.Get(ctx, "cluster", metav1.GetOptions{}) - if err != nil { - return err - } - - idpIndex := -1 - for i, idp := range config.Spec.IdentityProviders { - if idp.Name == idpName { - idpIndex = i - break - } - } - if idpIndex < 0 { - return nil - } - - config.Spec.IdentityProviders = append(config.Spec.IdentityProviders[:idpIndex], config.Spec.IdentityProviders[idpIndex+1:]...) - _, err = oauthClient.Update(ctx, config, metav1.UpdateOptions{}) - return err - }) - if err != nil { - g.GinkgoWriter.Printf("cleanup: failed to remove IdP %q from oauth/cluster: %v\n", idpName, err) - } -} - func verifyOAuthServerDeploymentProxyConfig(ctx context.Context, oc *exutil.CLI, expectedHTTPProxy, expectedHTTPSProxy, expectedNoProxy string, expectTrustedCAVolume bool) error { kubeClient := oc.AdminKubeClient() From 8d852b93a5a337bf475f1a4931bdc01c26d27a6b Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 13:37:59 +0200 Subject: [PATCH 04/16] Always return cleanup on error, register before Expect Deploy helpers no longer self-clean on error. Instead they always return accumulated cleanups, and callers register DeferCleanup before calling Expect. This prevents resource leaks when BeforeEach aborts mid-setup. --- .../authentication/component_proxy.go | 8 +-- .../authentication/component_proxy_helpers.go | 55 +++++-------------- .../authentication/keycloak_helpers.go | 21 ++----- 3 files changed, 24 insertions(+), 60 deletions(-) diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go index b74dc3c85909..fc3734cdf9e5 100644 --- a/test/extended/authentication/component_proxy.go +++ b/test/extended/authentication/component_proxy.go @@ -38,17 +38,17 @@ var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGat g.By("Deploying Squid forward proxy") var proxyCleanup removalFunc httpProxyURL, httpsProxyURL, caCertPEM, proxyNamespace, proxyCleanup, err = deploySquidProxy(ctx, oc) - o.Expect(err).NotTo(o.HaveOccurred()) g.DeferCleanup(proxyCleanup) + o.Expect(err).NotTo(o.HaveOccurred()) g.By("Deploying Keycloak (without registering IdP yet)") var kcCleanups []removalFunc kcSetup, kcCleanups, err = deployKeycloakForProxy(ctx, oc) - o.Expect(err).NotTo(o.HaveOccurred()) g.DeferCleanup(func() { g.GinkgoWriter.Println("cleanup: removing Keycloak resources") _ = removeResources(ctx, kcCleanups...) }) + o.Expect(err).NotTo(o.HaveOccurred()) // We wait for the operator a bit later in the startup sequence so that we can avoid // unblocking too fast after save-and-restore snapshots. We don't know whether there @@ -111,10 +111,10 @@ func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSet g.By("Registering Keycloak as OIDC IdP (operator discovers it through the proxy)") idpCleanups, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) - o.Expect(err).NotTo(o.HaveOccurred()) g.DeferCleanup(func() { _ = removeResources(ctx, idpCleanups...) }) + o.Expect(err).NotTo(o.HaveOccurred()) g.By("Waiting for operator to pick up IdP changes and stabilize") err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") @@ -144,10 +144,10 @@ func testFallbackOnProxyRemoval(ctx context.Context, oc *exutil.CLI, kcSetup *ke g.By("Registering Keycloak as OIDC IdP") idpCleanups, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) - o.Expect(err).NotTo(o.HaveOccurred()) g.DeferCleanup(func() { _ = removeResources(ctx, idpCleanups...) }) + o.Expect(err).NotTo(o.HaveOccurred()) g.By("Waiting for operator to pick up IdP changes and stabilize") err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index 07c187475b15..9cd56e09f0ec 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -140,14 +140,6 @@ func deploySquidProxy(ctx context.Context, oc *exutil.CLI) (httpProxyURL, httpsP return err } - success := false - defer func() { - if !success { - _ = cleanup(ctx) - cleanup = nil - } - }() - ca := mustNewCertificateAuthority(nil) serviceDNS := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) serverCert := mustNewServerCertificate(ca, serviceDNS) @@ -302,7 +294,6 @@ buffered_logs off serviceHost := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) httpProxyURL = fmt.Sprintf("http://%s:%d", serviceHost, squidHTTPPort) httpsProxyURL = fmt.Sprintf("https://%s:%d", serviceHost, squidHTTPSPort) - success = true g.GinkgoWriter.Printf("Squid proxy deployed: http=%s https=%s\n", httpProxyURL, httpsProxyURL) return httpProxyURL, httpsProxyURL, caCertPEM, namespace, cleanup, nil } @@ -368,17 +359,9 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy namespace := fmt.Sprintf("e2e-proxy-kc-%s", rand.String(8)) cleanups, err := deployKeycloak(ctx, oc, namespace, g.GinkgoLogr) if err != nil { - return nil, nil, fmt.Errorf("deploying keycloak: %w", err) + return nil, cleanups, fmt.Errorf("deploying keycloak: %w", err) } - success := false - defer func() { - if !success { - _ = removeResources(ctx, cleanups...) - cleanups = nil - } - }() - setup := &keycloakProxySetup{ idpName: fmt.Sprintf("keycloak-proxy-test-%s", namespace), namespace: namespace, @@ -387,12 +370,12 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy // Use the route for admin API calls (the test runner may be external). routeURL, err := admittedURLForRoute(ctx, oc, keycloakResourceName, namespace) if err != nil { - return nil, nil, fmt.Errorf("getting keycloak route URL: %w", err) + return nil, cleanups, fmt.Errorf("getting keycloak route URL: %w", err) } kcClient, err := keycloakClientFor(routeURL) if err != nil { - return nil, nil, fmt.Errorf("creating keycloak client: %w", err) + return nil, cleanups, fmt.Errorf("creating keycloak client: %w", err) } setup.client = kcClient @@ -409,12 +392,12 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy return true, nil }) if err != nil { - return nil, nil, fmt.Errorf("authenticating to keycloak: %w", err) + return nil, cleanups, fmt.Errorf("authenticating to keycloak: %w", err) } clientList, err := kcClient.ListClients() if err != nil { - return nil, nil, fmt.Errorf("listing keycloak clients: %w", err) + return nil, cleanups, fmt.Errorf("listing keycloak clients: %w", err) } var adminClientID, passwdClientID string @@ -431,10 +414,10 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy } if adminClientID == "" { - return nil, nil, fmt.Errorf("admin-cli client not found in keycloak") + return nil, cleanups, fmt.Errorf("admin-cli client not found in keycloak") } if passwdClientID == "" { - return nil, nil, fmt.Errorf("password-grant client (with redirectUris) not found in keycloak") + return nil, cleanups, fmt.Errorf("password-grant client (with redirectUris) not found in keycloak") } err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { @@ -449,12 +432,12 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy return true, nil }) if err != nil { - return nil, nil, fmt.Errorf("updating admin-cli access token timeout: %w", err) + return nil, cleanups, fmt.Errorf("updating admin-cli access token timeout: %w", err) } err = kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword) if err != nil { - return nil, nil, fmt.Errorf("re-authenticating to keycloak: %w", err) + return nil, cleanups, fmt.Errorf("re-authenticating to keycloak: %w", err) } err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { @@ -470,7 +453,7 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy return true, nil }) if err != nil { - return nil, nil, fmt.Errorf("regenerating client secret: %w", err) + return nil, cleanups, fmt.Errorf("regenerating client secret: %w", err) } err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { @@ -485,10 +468,9 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy return true, nil }) if err != nil { - return nil, nil, fmt.Errorf("creating client group mapper: %w", err) + return nil, cleanups, fmt.Errorf("creating client group mapper: %w", err) } - success = true return setup, cleanups, nil } @@ -496,14 +478,6 @@ func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keyc var cleanups []removalFunc kubeClient := oc.AdminKubeClient() - success := false - defer func() { - if !success { - _ = removeResources(ctx, cleanups...) - cleanups = nil - } - }() - secretName := setup.idpName + "-secret" _, err := kubeClient.CoreV1().Secrets("openshift-config").Create(ctx, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -515,7 +489,7 @@ func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keyc }, }, metav1.CreateOptions{}) if err != nil { - return nil, fmt.Errorf("creating keycloak client secret: %w", err) + return cleanups, fmt.Errorf("creating keycloak client secret: %w", err) } cleanups = append(cleanups, func(ctx context.Context) error { return kubeClient.CoreV1().Secrets("openshift-config").Delete(ctx, secretName, metav1.DeleteOptions{}) @@ -524,7 +498,7 @@ func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keyc caCMName := setup.idpName + "-ca" caCleanup, err := createServiceCAConfigMap(ctx, oc, caCMName) if err != nil { - return nil, fmt.Errorf("syncing default ingress CA: %w", err) + return cleanups, fmt.Errorf("syncing default ingress CA: %w", err) } cleanups = append(cleanups, caCleanup) @@ -551,10 +525,9 @@ func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keyc }, }) if err != nil { - return nil, fmt.Errorf("adding identity provider: %w", err) + return cleanups, fmt.Errorf("adding identity provider: %w", err) } - success = true return cleanups, nil } diff --git a/test/extended/authentication/keycloak_helpers.go b/test/extended/authentication/keycloak_helpers.go index 0695c887947f..a40574258dde 100644 --- a/test/extended/authentication/keycloak_helpers.go +++ b/test/extended/authentication/keycloak_helpers.go @@ -49,42 +49,33 @@ func deployKeycloak(ctx context.Context, client *exutil.CLI, namespace string, l } cleanups := []removalFunc{nsCleanup} - success := false - defer func() { - if !success { - _ = removeResources(ctx, cleanups...) - cleanups = nil - } - }() - if _, err = createKeycloakServiceAccount(ctx, corev1Client.ServiceAccounts(namespace)); err != nil { - return nil, fmt.Errorf("creating serviceaccount for keycloak: %w", err) + return cleanups, fmt.Errorf("creating serviceaccount for keycloak: %w", err) } service, _, err := createKeycloakService(ctx, corev1Client.Services(namespace)) if err != nil { - return nil, fmt.Errorf("creating service for keycloak: %w", err) + return cleanups, fmt.Errorf("creating service for keycloak: %w", err) } if _, err = createKeycloakDeployment(ctx, client.AdminKubeClient().AppsV1().Deployments(namespace)); err != nil { - return nil, fmt.Errorf("creating deployment for keycloak: %w", err) + return cleanups, fmt.Errorf("creating deployment for keycloak: %w", err) } if _, err = createKeycloakRoute(ctx, service, client.AdminRouteClient().RouteV1().Routes(namespace)); err != nil { - return nil, fmt.Errorf("creating route for keycloak: %w", err) + return cleanups, fmt.Errorf("creating route for keycloak: %w", err) } caCleanup, err := createKeycloakCAConfigMap(ctx, corev1Client) if err != nil { - return nil, fmt.Errorf("creating CA configmap for keycloak: %w", err) + return cleanups, fmt.Errorf("creating CA configmap for keycloak: %w", err) } cleanups = append(cleanups, caCleanup) if err := waitForKeycloakAvailable(ctx, client, namespace, logger); err != nil { - return nil, err + return cleanups, err } - success = true return cleanups, nil } From c31e2a4a3d07d396524fda1a025b886422e1b323 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 13:41:49 +0200 Subject: [PATCH 05/16] Use net.JoinHostPort for host:port URL construction Fixes nosprintfhostport linter warning. --- test/extended/authentication/component_proxy_helpers.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index 9cd56e09f0ec..a26f86d2b34d 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -5,7 +5,9 @@ import ( "crypto/x509" "encoding/pem" "fmt" + "net" "reflect" + "strconv" "strings" "time" @@ -292,8 +294,8 @@ buffered_logs off } serviceHost := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) - httpProxyURL = fmt.Sprintf("http://%s:%d", serviceHost, squidHTTPPort) - httpsProxyURL = fmt.Sprintf("https://%s:%d", serviceHost, squidHTTPSPort) + httpProxyURL = "http://" + net.JoinHostPort(serviceHost, strconv.Itoa(int(squidHTTPPort))) + httpsProxyURL = "https://" + net.JoinHostPort(serviceHost, strconv.Itoa(int(squidHTTPSPort))) g.GinkgoWriter.Printf("Squid proxy deployed: http=%s https=%s\n", httpProxyURL, httpsProxyURL) return httpProxyURL, httpsProxyURL, caCertPEM, namespace, cleanup, nil } @@ -381,7 +383,8 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy // Use the in-cluster service URL as the OIDC issuer so the operator must // go through the proxy to reach Keycloak (enforced by network policy). - setup.issuerURL = fmt.Sprintf("https://%s.%s.svc:%d/realms/master", keycloakResourceName, namespace, keycloakHTTPSPort) + serviceHost := fmt.Sprintf("%s.%s.svc", keycloakResourceName, namespace) + setup.issuerURL = "https://" + net.JoinHostPort(serviceHost, strconv.Itoa(int(keycloakHTTPSPort))) + "/realms/master" err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { err := kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword) From 930322d23c39f2db119d03415903d01cf4092f93 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 13:47:48 +0200 Subject: [PATCH 06/16] Reuse WaitForOperatorsToSettle instead of custom operator status checks Replace waitForClusterOperatorAvailableNotProgressingNotDegraded and its supporting functions with the existing operator.WaitForOperatorsToSettle utility. This checks all operators for the same three conditions (Available, NotProgressing, NotDegraded), which is appropriate for serial conformance tests. --- .../authentication/component_proxy.go | 10 ++- .../authentication/operator_status_helpers.go | 61 +------------------ 2 files changed, 6 insertions(+), 65 deletions(-) diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go index fc3734cdf9e5..72b3248bb952 100644 --- a/test/extended/authentication/component_proxy.go +++ b/test/extended/authentication/component_proxy.go @@ -13,6 +13,7 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" exutil "github.com/openshift/origin/test/extended/util" + operator "github.com/openshift/origin/test/extended/util/operator" ) var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGate:AuthenticationComponentProxy][Serial][Slow]", func() { @@ -32,8 +33,8 @@ var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGat g.By("Saving auth state for restore after test") authRestore, err := saveAndRestoreAuthState(ctx, oc) - o.Expect(err).NotTo(o.HaveOccurred()) g.DeferCleanup(authRestore) + o.Expect(err).NotTo(o.HaveOccurred()) g.By("Deploying Squid forward proxy") var proxyCleanup removalFunc @@ -50,11 +51,8 @@ var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGat }) o.Expect(err).NotTo(o.HaveOccurred()) - // We wait for the operator a bit later in the startup sequence so that we can avoid - // unblocking too fast after save-and-restore snapshots. We don't know whether there - // is any state change, so we can't really use waitForOperatorToPickUpChanges here. - g.By("Waiting for authentication operator to be stable before test") - err = waitForClusterOperatorAvailableNotProgressingNotDegraded(ctx, oc, "authentication") + g.By("Waiting for operators to be stable before test") + err = operator.WaitForOperatorsToSettle(ctx, oc.AdminConfigClient(), 10) o.Expect(err).NotTo(o.HaveOccurred()) g.GinkgoWriter.Printf("Squid proxy URL: http=%s https=%s\n", httpProxyURL, httpsProxyURL) diff --git a/test/extended/authentication/operator_status_helpers.go b/test/extended/authentication/operator_status_helpers.go index 217bfa9c02d3..4b92f0de0c4a 100644 --- a/test/extended/authentication/operator_status_helpers.go +++ b/test/extended/authentication/operator_status_helpers.go @@ -6,13 +6,8 @@ import ( g "github.com/onsi/ginkgo/v2" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/wait" - - configv1 "github.com/openshift/api/config/v1" - configv1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers" - exutil "github.com/openshift/origin/test/extended/util" + operator "github.com/openshift/origin/test/extended/util/operator" ) func waitForOperatorToPickUpChanges(ctx context.Context, oc *exutil.CLI, name string) error { @@ -21,57 +16,5 @@ func waitForOperatorToPickUpChanges(ctx context.Context, oc *exutil.CLI, name st if err := exutil.WaitForOperatorProgressingTrue(progressCtx, oc.AdminConfigClient(), name); err != nil { g.GinkgoWriter.Printf("operator %s did not become Progressing=True (may have reconciled quickly): %v\n", name, err) } - return waitForClusterOperatorAvailableNotProgressingNotDegraded(ctx, oc, name) -} - -func waitForClusterOperatorAvailableNotProgressingNotDegraded(ctx context.Context, oc *exutil.CLI, name string) error { - return waitForClusterOperatorStatus(ctx, oc, name, - configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorAvailable, Status: configv1.ConditionTrue}, - configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse}, - configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorDegraded, Status: configv1.ConditionFalse}, - ) -} - -// waitForClusterOperatorStatus polls until all requiredConditions are met on a -// single named ClusterOperator. Unlike WaitForOperatorsToSettle (which checks -// all operators for the fixed Available+NotProgressing+NotDegraded state), this -// waits for an arbitrary set of conditions on one operator — needed for tests -// that assert Degraded=True or check a subset of conditions. -func waitForClusterOperatorStatus(ctx context.Context, oc *exutil.CLI, name string, requiredConditions ...configv1.ClusterOperatorStatusCondition) error { - var conditions []configv1.ClusterOperatorStatusCondition - - ts := time.Now() - err := wait.PollUntilContextTimeout(ctx, 10*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { - done, conds, checkErr := checkClusterOperatorStatus(ctx, oc, name, requiredConditions...) - if checkErr != nil { - g.GinkgoWriter.Printf("error checking clusteroperator/%s, will retry: %v\n", name, checkErr) - return false, nil - } - conditions = conds - return done, nil - }) - - if err == nil { - g.GinkgoWriter.Printf("clusteroperator/%s reached required status after %s\n", name, time.Since(ts)) - return nil - } - - g.GinkgoWriter.Printf("clusteroperator/%s did not reach required status after %s, current: %v\n", name, time.Since(ts), conditions) - return err -} - -func checkClusterOperatorStatus(ctx context.Context, oc *exutil.CLI, name string, requiredConditions ...configv1.ClusterOperatorStatusCondition) (bool, []configv1.ClusterOperatorStatusCondition, error) { - clusterOperator, err := oc.AdminConfigClient().ConfigV1().ClusterOperators().Get(ctx, name, metav1.GetOptions{}) - if err != nil { - return false, nil, err - } - - conditions := clusterOperator.Status.Conditions - for _, required := range requiredConditions { - if len(required.Status) > 0 && !configv1helpers.IsStatusConditionPresentAndEqual(conditions, required.Type, required.Status) { - return false, conditions, nil - } - } - - return true, conditions, nil + return operator.WaitForOperatorsToSettle(ctx, oc.AdminConfigClient(), 10) } From 4b4aa33d7345bcf8c525ba85ac383e5f116af939 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 13:50:55 +0200 Subject: [PATCH 07/16] Add comment explaining why client secret is regenerated --- test/extended/authentication/component_proxy_helpers.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index a26f86d2b34d..cddf0184e06f 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -443,6 +443,8 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy return nil, cleanups, fmt.Errorf("re-authenticating to keycloak: %w", err) } + // Regenerate the client secret so we have a known value to pass to the + // OAuth IdP configuration — the initial secret is auto-generated by Keycloak. err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { var err error setup.clientSecret, err = kcClient.RegenerateClientSecret(passwdClientID) From 9f3ea8e2add612c05796c70945e7916612c2a8fa Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 13:57:30 +0200 Subject: [PATCH 08/16] Add comments explaining admin-cli token timeout and secret regeneration --- test/extended/authentication/component_proxy_helpers.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index cddf0184e06f..b093f78c3223 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -423,6 +423,8 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy return nil, cleanups, fmt.Errorf("password-grant client (with redirectUris) not found in keycloak") } + // Extend admin-cli token lifetime to 30 minutes so the token doesn't + // expire during subsequent Keycloak API calls. err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { err := kcClient.UpdateClientAccessTokenTimeout(adminClientID, 60*30) if err != nil { From 5f050abc9a67701f1f78b084966a24e711aca76b Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 14:20:31 +0200 Subject: [PATCH 09/16] Add comment explaining NO_PROXY superset check --- test/extended/authentication/component_proxy_helpers.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index b093f78c3223..0a11cdb214dc 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -676,6 +676,8 @@ func verifyOAuthServerDeploymentProxyConfig(ctx context.Context, oc *exutil.CLI, return false, nil } } else { + // Use IsSuperset rather than exact match because the operator appends + // the apiserver IP to NO_PROXY beyond the entries we configure. actualNoProxy := sets.New[string](strings.Split(envVars["NO_PROXY"], ",")...) expectedNoProxyEntries := sets.New[string](strings.Split(expectedNoProxy, ",")...) if !actualNoProxy.IsSuperset(expectedNoProxyEntries) { From 487ed42965993952718a02d745e549d81d2869c4 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 15:56:53 +0200 Subject: [PATCH 10/16] Fix vet errors --- test/extended/authentication/component_proxy_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index 0a11cdb214dc..10fa922ef39f 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -277,7 +277,7 @@ buffered_logs off nil, func(event watch.Event) (bool, error) { if event.Type == watch.Error { - return false, fmt.Errorf("Squid deployment watch error: %w", event.Object) + return false, fmt.Errorf("Squid deployment watch error: %v", event.Object) } if event.Type == watch.Bookmark { return false, nil From 2224a7ddcc382a595718c1ec638b6985c69dcc8d Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 16:09:13 +0200 Subject: [PATCH 11/16] Use AfterEach for cleanup and remove redundant network policy cleanup Replace scattered DeferCleanup calls in BeforeEach with a shared cleanups slice and explicit AfterEach. Drop the network policy cleanup since the keycloak namespace deletion cascades to it. --- .../authentication/component_proxy.go | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go index 72b3248bb952..aab73e2613b6 100644 --- a/test/extended/authentication/component_proxy.go +++ b/test/extended/authentication/component_proxy.go @@ -26,29 +26,28 @@ var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGat caCertPEM []byte proxyNamespace string kcSetup *keycloakProxySetup + cleanups []removalFunc ) g.BeforeEach(func() { ctx = context.Background() + cleanups = nil g.By("Saving auth state for restore after test") authRestore, err := saveAndRestoreAuthState(ctx, oc) - g.DeferCleanup(authRestore) + cleanups = append(cleanups, authRestore) o.Expect(err).NotTo(o.HaveOccurred()) g.By("Deploying Squid forward proxy") var proxyCleanup removalFunc httpProxyURL, httpsProxyURL, caCertPEM, proxyNamespace, proxyCleanup, err = deploySquidProxy(ctx, oc) - g.DeferCleanup(proxyCleanup) + cleanups = append(cleanups, proxyCleanup) o.Expect(err).NotTo(o.HaveOccurred()) g.By("Deploying Keycloak (without registering IdP yet)") var kcCleanups []removalFunc kcSetup, kcCleanups, err = deployKeycloakForProxy(ctx, oc) - g.DeferCleanup(func() { - g.GinkgoWriter.Println("cleanup: removing Keycloak resources") - _ = removeResources(ctx, kcCleanups...) - }) + cleanups = append(cleanups, kcCleanups...) o.Expect(err).NotTo(o.HaveOccurred()) g.By("Waiting for operators to be stable before test") @@ -60,6 +59,14 @@ var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGat g.GinkgoWriter.Printf("Keycloak namespace: %s\n", kcSetup.namespace) }) + g.AfterEach(func() { + _ = removeResources(ctx, cleanups...) + + g.By("Waiting for operators to be stable after test") + err := operator.WaitForOperatorsToSettle(ctx, oc.AdminConfigClient(), 10) + o.Expect(err).NotTo(o.HaveOccurred()) + }) + g.It("should validate OIDC IdP through component proxy", func() { testOIDCIdPThroughComponentProxy(ctx, oc, kcSetup, httpProxyURL, nil, proxyNamespace) }) @@ -93,9 +100,9 @@ func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSet } g.By("Deploying NetworkPolicy to restrict Keycloak ingress to proxy namespace only") - networkPolicyCleanup, err := deployProxyNetworkPolicies(ctx, oc, proxyNamespace, kcSetup.namespace) + // We don't need to call the cleanup function since the whole namespace is removed in AfterEach. + _, err := deployProxyNetworkPolicies(ctx, oc, proxyNamespace, kcSetup.namespace) o.Expect(err).NotTo(o.HaveOccurred()) - g.DeferCleanup(networkPolicyCleanup) g.By("Setting component-scoped proxy") proxyConfig := operatorv1.AuthenticationProxyConfig{ From 94ea07e809c0ebccaa817a0ce5bdb03dc9140827 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 16:15:15 +0200 Subject: [PATCH 12/16] Use library-go/pkg/crypto for proxy TLS cert generation Replace custom crypto_helpers.go with library-go's MakeSelfSignedCAConfigForDuration and CA.MakeServerCert. This reuses existing, well-tested crypto utilities instead of maintaining a separate implementation. --- .../authentication/component_proxy_helpers.go | 27 +++--- .../extended/authentication/crypto_helpers.go | 89 ------------------- 2 files changed, 17 insertions(+), 99 deletions(-) delete mode 100644 test/extended/authentication/crypto_helpers.go diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index 10fa922ef39f..7c1d7bd6406b 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -2,8 +2,6 @@ package authentication import ( "context" - "crypto/x509" - "encoding/pem" "fmt" "net" "reflect" @@ -14,6 +12,7 @@ import ( g "github.com/onsi/ginkgo/v2" configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" + libcrypto "github.com/openshift/library-go/pkg/crypto" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -142,18 +141,26 @@ func deploySquidProxy(ctx context.Context, oc *exutil.CLI) (httpProxyURL, httpsP return err } - ca := mustNewCertificateAuthority(nil) - serviceDNS := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) - serverCert := mustNewServerCertificate(ca, serviceDNS) + caConfig, err := libcrypto.MakeSelfSignedCAConfigForDuration("squid-proxy-ca", 2*time.Hour) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("creating proxy CA: %w", err) + } + ca := &libcrypto.CA{Config: caConfig, SerialGenerator: &libcrypto.RandomSerialGenerator{}} - caCertPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: ca.Certificate.Raw}) - serverCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: serverCert.Certificate.Raw}) + serviceDNS := fmt.Sprintf("%s.%s.svc.cluster.local", squidServiceName, namespace) + serverCertConfig, err := ca.MakeServerCert(sets.New(serviceDNS), 2*time.Hour) + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("creating proxy server cert: %w", err) + } - serverKeyDER, err := x509.MarshalPKCS8PrivateKey(serverCert.PrivateKey) + caCertPEM, _, err = caConfig.GetPEMBytes() + if err != nil { + return "", "", nil, "", cleanup, fmt.Errorf("encoding proxy CA cert: %w", err) + } + serverCertPEM, serverKeyPEM, err := serverCertConfig.GetPEMBytes() if err != nil { - return "", "", nil, "", cleanup, fmt.Errorf("marshalling server private key: %w", err) + return "", "", nil, "", cleanup, fmt.Errorf("encoding proxy server cert: %w", err) } - serverKeyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: serverKeyDER}) squidConfig := fmt.Sprintf(`http_port %d https_port %d tls-cert=/etc/squid/tls/tls.crt tls-key=/etc/squid/tls/tls.key diff --git a/test/extended/authentication/crypto_helpers.go b/test/extended/authentication/crypto_helpers.go deleted file mode 100644 index e5f31bc78a65..000000000000 --- a/test/extended/authentication/crypto_helpers.go +++ /dev/null @@ -1,89 +0,0 @@ -package authentication - -import ( - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "crypto/x509/pkix" - "math" - "math/big" - "time" -) - -type cryptoMaterials struct { - PrivateKey *rsa.PrivateKey - Certificate *x509.Certificate -} - -func mustNewServerCertificate(signer *cryptoMaterials, hosts ...string) *cryptoMaterials { - var server cryptoMaterials - var err error - if server.PrivateKey, err = rsa.GenerateKey(rand.Reader, 2048); err != nil { - panic(err) - } - - serialNumber, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64)) - if err != nil { - panic(err) - } - - template := &x509.Certificate{ - Subject: pkix.Name{CommonName: "server"}, - NotBefore: time.Now().AddDate(-1, 0, 0), - NotAfter: time.Now().AddDate(1, 0, 0), - SignatureAlgorithm: x509.SHA256WithRSA, - SerialNumber: serialNumber, - KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, - BasicConstraintsValid: true, - DNSNames: hosts, - } - der, err := x509.CreateCertificate(rand.Reader, template, signer.Certificate, server.PrivateKey.Public(), signer.PrivateKey) - if err != nil { - panic(err) - } - - if server.Certificate, err = x509.ParseCertificate(der); err != nil { - panic(err) - } - return &server -} - -func mustNewCertificateAuthority(parent *cryptoMaterials) *cryptoMaterials { - var ca cryptoMaterials - var err error - if ca.PrivateKey, err = rsa.GenerateKey(rand.Reader, 2048); err != nil { - panic(err) - } - - serialNumber, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64)) - if err != nil { - panic(err) - } - - template := &x509.Certificate{ - Subject: pkix.Name{CommonName: "ca"}, - NotBefore: time.Now().AddDate(-1, 0, 0), - NotAfter: time.Now().AddDate(1, 0, 0), - SignatureAlgorithm: x509.SHA256WithRSA, - SerialNumber: serialNumber, - KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - BasicConstraintsValid: true, - IsCA: true, - } - signerCertificate := template - signerPrivateKey := ca.PrivateKey - if parent != nil { - signerCertificate = parent.Certificate - signerPrivateKey = parent.PrivateKey - } - der, err := x509.CreateCertificate(rand.Reader, template, signerCertificate, ca.PrivateKey.Public(), signerPrivateKey) - if err != nil { - panic(err) - } - - if ca.Certificate, err = x509.ParseCertificate(der); err != nil { - panic(err) - } - return &ca -} From a2b9c93a0cb94e508ac1b69d28778dc7288db39e Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 16:26:06 +0200 Subject: [PATCH 13/16] Revert keycloak_helpers.go changes to reduce PR scope --- .../authentication/keycloak_helpers.go | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/test/extended/authentication/keycloak_helpers.go b/test/extended/authentication/keycloak_helpers.go index a40574258dde..a8c2465f6130 100644 --- a/test/extended/authentication/keycloak_helpers.go +++ b/test/extended/authentication/keycloak_helpers.go @@ -41,42 +41,47 @@ const ( ) func deployKeycloak(ctx context.Context, client *exutil.CLI, namespace string, logger logr.Logger) ([]removalFunc, error) { + cleanups := []removalFunc{} + corev1Client := client.AdminKubeClient().CoreV1() - nsCleanup, err := createKeycloakNamespace(ctx, corev1Client.Namespaces(), namespace) + cleanup, err := createKeycloakNamespace(ctx, corev1Client.Namespaces(), namespace) if err != nil { - return nil, fmt.Errorf("creating namespace for keycloak: %w", err) + return cleanups, fmt.Errorf("creating namespace for keycloak: %w", err) } - cleanups := []removalFunc{nsCleanup} + cleanups = append(cleanups, cleanup) - if _, err = createKeycloakServiceAccount(ctx, corev1Client.ServiceAccounts(namespace)); err != nil { + cleanup, err = createKeycloakServiceAccount(ctx, corev1Client.ServiceAccounts(namespace)) + if err != nil { return cleanups, fmt.Errorf("creating serviceaccount for keycloak: %w", err) } + cleanups = append(cleanups, cleanup) - service, _, err := createKeycloakService(ctx, corev1Client.Services(namespace)) + service, cleanup, err := createKeycloakService(ctx, corev1Client.Services(namespace)) if err != nil { return cleanups, fmt.Errorf("creating service for keycloak: %w", err) } + cleanups = append(cleanups, cleanup) - if _, err = createKeycloakDeployment(ctx, client.AdminKubeClient().AppsV1().Deployments(namespace)); err != nil { + cleanup, err = createKeycloakDeployment(ctx, client.AdminKubeClient().AppsV1().Deployments(namespace)) + if err != nil { return cleanups, fmt.Errorf("creating deployment for keycloak: %w", err) } + cleanups = append(cleanups, cleanup) - if _, err = createKeycloakRoute(ctx, service, client.AdminRouteClient().RouteV1().Routes(namespace)); err != nil { + cleanup, err = createKeycloakRoute(ctx, service, client.AdminRouteClient().RouteV1().Routes(namespace)) + if err != nil { return cleanups, fmt.Errorf("creating route for keycloak: %w", err) } + cleanups = append(cleanups, cleanup) - caCleanup, err := createKeycloakCAConfigMap(ctx, corev1Client) + cleanup, err = createKeycloakCAConfigMap(ctx, corev1Client) if err != nil { return cleanups, fmt.Errorf("creating CA configmap for keycloak: %w", err) } - cleanups = append(cleanups, caCleanup) - - if err := waitForKeycloakAvailable(ctx, client, namespace, logger); err != nil { - return cleanups, err - } + cleanups = append(cleanups, cleanup) - return cleanups, nil + return cleanups, waitForKeycloakAvailable(ctx, client, namespace, logger) } func createKeycloakNamespace(ctx context.Context, client typedcorev1.NamespaceInterface, namespace string) (removalFunc, error) { From 4e27a4f07807a69c5d8b3691b88a88e2fe13cbdd Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 31 Jul 2026 16:31:36 +0200 Subject: [PATCH 14/16] Remove squid log traffic check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The network policy structurally enforces proxy usage — only the proxy namespace can reach Keycloak pods. If the operator successfully discovers the OIDC issuer, it must have gone through the proxy. The log check was redundant. --- .../authentication/component_proxy.go | 4 -- .../authentication/component_proxy_helpers.go | 46 ------------------- 2 files changed, 50 deletions(-) diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go index aab73e2613b6..1d17401880b0 100644 --- a/test/extended/authentication/component_proxy.go +++ b/test/extended/authentication/component_proxy.go @@ -2,7 +2,6 @@ package authentication import ( "context" - "time" g "github.com/onsi/ginkgo/v2" o "github.com/onsi/gomega" @@ -135,9 +134,6 @@ func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSet o.Expect(err).NotTo(o.HaveOccurred()) } - g.By("Verifying traffic went through the Squid proxy") - err = waitForSquidProxyTraffic(ctx, oc, proxyNamespace, 5*time.Minute) - o.Expect(err).NotTo(o.HaveOccurred()) } func testFallbackOnProxyRemoval(ctx context.Context, oc *exutil.CLI, kcSetup *keycloakProxySetup, httpProxyURL string, proxyNamespace string) { diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index 7c1d7bd6406b..b6a3e3cd5b33 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -307,52 +307,6 @@ buffered_logs off return httpProxyURL, httpsProxyURL, caCertPEM, namespace, cleanup, nil } -func getSquidProxyLogs(ctx context.Context, oc *exutil.CLI, namespace string) (string, error) { - return getSquidProxyLogsSince(ctx, oc, namespace, time.Time{}) -} - -func getSquidProxyLogsSince(ctx context.Context, oc *exutil.CLI, namespace string, since time.Time) (string, error) { - kubeClient := oc.AdminKubeClient() - - pods, err := kubeClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{ - LabelSelector: fmt.Sprintf("app=%s", squidServiceName), - }) - if err != nil { - return "", fmt.Errorf("listing squid pods in %s: %w", namespace, err) - } - if len(pods.Items) == 0 { - return "", fmt.Errorf("no squid proxy pods found in namespace %s", namespace) - } - - logOpts := &corev1.PodLogOptions{Container: "squid"} - if !since.IsZero() { - t := metav1.NewTime(since) - logOpts.SinceTime = &t - } - logBytes, err := kubeClient.CoreV1().Pods(namespace).GetLogs(pods.Items[0].Name, logOpts).DoRaw(ctx) - if err != nil { - return "", fmt.Errorf("getting logs from squid container: %w", err) - } - - return string(logBytes), nil -} - -func waitForSquidProxyTraffic(ctx context.Context, oc *exutil.CLI, namespace string, timeout time.Duration) error { - g.GinkgoWriter.Printf("waiting up to %s for traffic in squid proxy logs\n", timeout) - return wait.PollUntilContextTimeout(ctx, 10*time.Second, timeout, true, func(ctx context.Context) (bool, error) { - logs, err := getSquidProxyLogs(ctx, oc, namespace) - if err != nil { - g.GinkgoWriter.Printf("failed to read squid logs: %v\n", err) - return false, nil - } - if strings.Contains(logs, "CONNECT") || strings.Contains(logs, "TCP_") { - g.GinkgoWriter.Println("detected proxy traffic in squid logs") - return true, nil - } - return false, nil - }) -} - // keycloakProxySetup holds the results of deploying Keycloak for proxy tests, // before the IdP is registered in OpenShift. type keycloakProxySetup struct { From 29c067d4d9cf1753e33a68c8e8b595d2769006e1 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Mon, 3 Aug 2026 10:48:14 +0200 Subject: [PATCH 15/16] Use route IP whitelist instead of network policy for proxy enforcement The service URL approach doesn't work because .svc is in the operator's NO_PROXY list, bypassing the proxy entirely. Revert to the route URL as the OIDC issuer and use haproxy.router.openshift.io/ip_whitelist on the Keycloak route to restrict access to only the Squid proxy pod IP. This structurally enforces proxy usage: the ingress router rejects requests from any IP other than the proxy, so the operator must route through the proxy to reach Keycloak. Also reverts to the default ingress CA (instead of service CA) since the route's TLS cert is signed by the ingress CA. --- .../authentication/component_proxy.go | 6 +- .../authentication/component_proxy_helpers.go | 110 +++++++++--------- 2 files changed, 57 insertions(+), 59 deletions(-) diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go index 1d17401880b0..d323e6eac98d 100644 --- a/test/extended/authentication/component_proxy.go +++ b/test/extended/authentication/component_proxy.go @@ -98,10 +98,10 @@ func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSet }) } - g.By("Deploying NetworkPolicy to restrict Keycloak ingress to proxy namespace only") - // We don't need to call the cleanup function since the whole namespace is removed in AfterEach. - _, err := deployProxyNetworkPolicies(ctx, oc, proxyNamespace, kcSetup.namespace) + g.By("Restricting Keycloak route to proxy IP only") + whitelistCleanup, err := restrictKeycloakRouteToProxy(ctx, oc, proxyNamespace, kcSetup.namespace) o.Expect(err).NotTo(o.HaveOccurred()) + g.DeferCleanup(whitelistCleanup) g.By("Setting component-scoped proxy") proxyConfig := operatorv1.AuthenticationProxyConfig{ diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index b6a3e3cd5b33..8674ac7691bd 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -15,7 +15,6 @@ import ( libcrypto "github.com/openshift/library-go/pkg/crypto" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -341,11 +340,7 @@ func deployKeycloakForProxy(ctx context.Context, oc *exutil.CLI) (*keycloakProxy return nil, cleanups, fmt.Errorf("creating keycloak client: %w", err) } setup.client = kcClient - - // Use the in-cluster service URL as the OIDC issuer so the operator must - // go through the proxy to reach Keycloak (enforced by network policy). - serviceHost := fmt.Sprintf("%s.%s.svc", keycloakResourceName, namespace) - setup.issuerURL = "https://" + net.JoinHostPort(serviceHost, strconv.Itoa(int(keycloakHTTPSPort))) + "/realms/master" + setup.issuerURL = routeURL + "/realms/master" err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { err := kcClient.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword) @@ -464,7 +459,7 @@ func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keyc }) caCMName := setup.idpName + "-ca" - caCleanup, err := createServiceCAConfigMap(ctx, oc, caCMName) + caCleanup, err := syncDefaultIngressCAToConfig(ctx, oc, caCMName) if err != nil { return cleanups, fmt.Errorf("syncing default ingress CA: %w", err) } @@ -499,20 +494,14 @@ func addKeycloakOIDCIdPForProxy(ctx context.Context, oc *exutil.CLI, setup *keyc return cleanups, nil } -// createServiceCAConfigMap copies the service-ca CA bundle into a new ConfigMap -// in openshift-config under the "ca.crt" key expected by the OAuth server for -// IdP CA references. The service CA signs serving certs for services annotated -// with service.beta.openshift.io/serving-cert-secret-name. -func createServiceCAConfigMap(ctx context.Context, oc *exutil.CLI, name string) (removalFunc, error) { +// syncDefaultIngressCAToConfig copies the default ingress CA into a new +// ConfigMap in openshift-config so it can be referenced by an IdP's CA field. +func syncDefaultIngressCAToConfig(ctx context.Context, oc *exutil.CLI, name string) (removalFunc, error) { kubeClient := oc.AdminKubeClient() - serviceCA, err := kubeClient.CoreV1().ConfigMaps("openshift-authentication").Get(ctx, "v4-0-config-system-service-ca", metav1.GetOptions{}) + ca, err := kubeClient.CoreV1().ConfigMaps("openshift-config-managed").Get(ctx, "default-ingress-cert", metav1.GetOptions{}) if err != nil { - return nil, fmt.Errorf("getting openshift-authentication/v4-0-config-system-service-ca: %w", err) - } - caBundle := serviceCA.Data["service-ca.crt"] - if len(caBundle) == 0 { - return nil, fmt.Errorf("service-ca.crt is empty in openshift-authentication/v4-0-config-system-service-ca") + return nil, fmt.Errorf("getting openshift-config-managed/default-ingress-cert: %w", err) } _, err = kubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, &corev1.ConfigMap{ @@ -521,7 +510,7 @@ func createServiceCAConfigMap(ctx context.Context, oc *exutil.CLI, name string) Labels: componentProxyTestLabels(), }, Data: map[string]string{ - "ca.crt": caBundle, + "ca.crt": ca.Data["ca-bundle.crt"], }, }, metav1.CreateOptions{}) if err != nil { @@ -533,52 +522,61 @@ func createServiceCAConfigMap(ctx context.Context, oc *exutil.CLI, name string) }, nil } -// deployProxyNetworkPolicies restricts ingress to the Keycloak namespace so that -// only the proxy namespace can reach Keycloak pods. This forces the operator to -// go through the Squid proxy to contact the OIDC issuer. +// restrictKeycloakRouteToProxy annotates the Keycloak route with an IP whitelist +// so only the Squid proxy pod can reach it. This forces the operator to go +// through the proxy to contact the OIDC issuer. // -// All Keycloak admin API setup (via the route) must happen before this policy is -// applied, since it blocks the ingress router as well. -func deployProxyNetworkPolicies(ctx context.Context, oc *exutil.CLI, proxyNamespace, keycloakNamespace string) (removalFunc, error) { +// All Keycloak admin API setup (via the route) must happen before this is +// called, since it blocks all other traffic to the route. +func restrictKeycloakRouteToProxy(ctx context.Context, oc *exutil.CLI, proxyNamespace, keycloakNamespace string) (removalFunc, error) { kubeClient := oc.AdminKubeClient() - keycloakPolicy := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "proxy-e2e-allow-only-from-proxy", - Namespace: keycloakNamespace, - Labels: componentProxyTestLabels(), - }, - Spec: networkingv1.NetworkPolicySpec{ - PodSelector: metav1.LabelSelector{}, - PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, - Ingress: []networkingv1.NetworkPolicyIngressRule{ - { - From: []networkingv1.NetworkPolicyPeer{ - { - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "kubernetes.io/metadata.name": proxyNamespace, - }, - }, - }, - }, - }, - }, - }, + pods, err := kubeClient.CoreV1().Pods(proxyNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("app=%s", squidServiceName), + }) + if err != nil { + return nil, fmt.Errorf("listing proxy pods in %s: %w", proxyNamespace, err) + } + if len(pods.Items) == 0 { + return nil, fmt.Errorf("no proxy pods found in %s", proxyNamespace) } - _, err := kubeClient.NetworkingV1().NetworkPolicies(keycloakNamespace).Create(ctx, keycloakPolicy, metav1.CreateOptions{}) - if err != nil { - return nil, fmt.Errorf("creating NetworkPolicy in %s: %w", keycloakNamespace, err) + proxyIP := pods.Items[0].Status.PodIP + if len(proxyIP) == 0 { + return nil, fmt.Errorf("proxy pod %s has no IP", pods.Items[0].Name) } - g.GinkgoWriter.Printf("created NetworkPolicy proxy-e2e-allow-only-from-proxy in %s\n", keycloakNamespace) - return func(ctx context.Context) error { - err := kubeClient.NetworkingV1().NetworkPolicies(keycloakNamespace).Delete(ctx, "proxy-e2e-allow-only-from-proxy", metav1.DeleteOptions{}) - if apierrors.IsNotFound(err) { - return nil + routeClient := oc.AdminRouteClient().RouteV1().Routes(keycloakNamespace) + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + route, err := routeClient.Get(ctx, keycloakResourceName, metav1.GetOptions{}) + if err != nil { + return err + } + if route.Annotations == nil { + route.Annotations = map[string]string{} } + route.Annotations["haproxy.router.openshift.io/ip_whitelist"] = proxyIP + _, err = routeClient.Update(ctx, route, metav1.UpdateOptions{}) return err + }) + if err != nil { + return nil, fmt.Errorf("annotating keycloak route with IP whitelist: %w", err) + } + g.GinkgoWriter.Printf("restricted keycloak route to proxy IP %s\n", proxyIP) + + return func(ctx context.Context) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + route, err := routeClient.Get(ctx, keycloakResourceName, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + delete(route.Annotations, "haproxy.router.openshift.io/ip_whitelist") + _, err = routeClient.Update(ctx, route, metav1.UpdateOptions{}) + return err + }) }, nil } From 5aeda144a4a2c68a215db86c70ba6adf935bb60d Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Mon, 3 Aug 2026 16:43:21 +0200 Subject: [PATCH 16/16] Use route IP whitelist for proxy enforcement and verify reachability Replace network policy with haproxy.router.openshift.io/ip_whitelist on the Keycloak route, restricting access to the Squid proxy pod IP. This structurally enforces proxy usage since the ingress router rejects requests from any other source IP. Add a reachability check that verifies Keycloak is blocked without the proxy (expects EOF from the whitelisted route). Wait for trustedCA sync before registering the IdP to avoid a race where the operator tries to use the HTTPS proxy before the CA is available. Revert to route URL as issuer and ingress CA for the IdP CA configmap, since .svc addresses are in the operator's NO_PROXY list. --- .../authentication/component_proxy.go | 22 +++++++++----- .../authentication/component_proxy_helpers.go | 30 +++++-------------- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/test/extended/authentication/component_proxy.go b/test/extended/authentication/component_proxy.go index d323e6eac98d..88e501b3b72d 100644 --- a/test/extended/authentication/component_proxy.go +++ b/test/extended/authentication/component_proxy.go @@ -2,6 +2,7 @@ package authentication import ( "context" + "time" g "github.com/onsi/ginkgo/v2" o "github.com/onsi/gomega" @@ -99,9 +100,14 @@ func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSet } g.By("Restricting Keycloak route to proxy IP only") - whitelistCleanup, err := restrictKeycloakRouteToProxy(ctx, oc, proxyNamespace, kcSetup.namespace) + err := restrictKeycloakRouteToProxy(ctx, oc, proxyNamespace, kcSetup.namespace) o.Expect(err).NotTo(o.HaveOccurred()) - g.DeferCleanup(whitelistCleanup) + + g.By("Verifying Keycloak route is not reachable without the proxy") + o.Eventually(func() error { + return kcSetup.client.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword) + }, 1*time.Minute, 5*time.Second).Should(o.MatchError(o.ContainSubstring("EOF")), + "Keycloak route should be blocked for non-proxy IPs") g.By("Setting component-scoped proxy") proxyConfig := operatorv1.AuthenticationProxyConfig{ @@ -113,6 +119,12 @@ func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSet err = updateAuthenticationProxy(ctx, oc, proxyConfig) o.Expect(err).NotTo(o.HaveOccurred()) + if withTrustedCA { + g.By("Waiting for trustedCA ConfigMap to be synced before registering IdP") + err = verifyTrustedCAConfigMapSynced(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred()) + } + g.By("Registering Keycloak as OIDC IdP (operator discovers it through the proxy)") idpCleanups, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) g.DeferCleanup(func() { @@ -128,12 +140,6 @@ func testOIDCIdPThroughComponentProxy(ctx context.Context, oc *exutil.CLI, kcSet err = verifyOAuthServerDeploymentProxyConfig(ctx, oc, "", proxyURL, ".cluster.local,.svc,127.0.0.1,localhost", withTrustedCA) o.Expect(err).NotTo(o.HaveOccurred()) - if withTrustedCA { - g.By("Verifying trustedCA ConfigMap was synced to openshift-authentication") - err = verifyTrustedCAConfigMapSynced(ctx, oc) - o.Expect(err).NotTo(o.HaveOccurred()) - } - } func testFallbackOnProxyRemoval(ctx context.Context, oc *exutil.CLI, kcSetup *keycloakProxySetup, httpProxyURL string, proxyNamespace string) { diff --git a/test/extended/authentication/component_proxy_helpers.go b/test/extended/authentication/component_proxy_helpers.go index 8674ac7691bd..e08225f3cda3 100644 --- a/test/extended/authentication/component_proxy_helpers.go +++ b/test/extended/authentication/component_proxy_helpers.go @@ -528,22 +528,22 @@ func syncDefaultIngressCAToConfig(ctx context.Context, oc *exutil.CLI, name stri // // All Keycloak admin API setup (via the route) must happen before this is // called, since it blocks all other traffic to the route. -func restrictKeycloakRouteToProxy(ctx context.Context, oc *exutil.CLI, proxyNamespace, keycloakNamespace string) (removalFunc, error) { +func restrictKeycloakRouteToProxy(ctx context.Context, oc *exutil.CLI, proxyNamespace, keycloakNamespace string) error { kubeClient := oc.AdminKubeClient() pods, err := kubeClient.CoreV1().Pods(proxyNamespace).List(ctx, metav1.ListOptions{ LabelSelector: fmt.Sprintf("app=%s", squidServiceName), }) if err != nil { - return nil, fmt.Errorf("listing proxy pods in %s: %w", proxyNamespace, err) + return fmt.Errorf("listing proxy pods in %s: %w", proxyNamespace, err) } if len(pods.Items) == 0 { - return nil, fmt.Errorf("no proxy pods found in %s", proxyNamespace) + return fmt.Errorf("no proxy pods found in %s", proxyNamespace) } proxyIP := pods.Items[0].Status.PodIP if len(proxyIP) == 0 { - return nil, fmt.Errorf("proxy pod %s has no IP", pods.Items[0].Name) + return fmt.Errorf("proxy pod %s has no IP", pods.Items[0].Name) } routeClient := oc.AdminRouteClient().RouteV1().Routes(keycloakNamespace) @@ -555,29 +555,15 @@ func restrictKeycloakRouteToProxy(ctx context.Context, oc *exutil.CLI, proxyName if route.Annotations == nil { route.Annotations = map[string]string{} } - route.Annotations["haproxy.router.openshift.io/ip_whitelist"] = proxyIP + route.Annotations["haproxy.router.openshift.io/ip_allowlist"] = proxyIP _, err = routeClient.Update(ctx, route, metav1.UpdateOptions{}) return err }) if err != nil { - return nil, fmt.Errorf("annotating keycloak route with IP whitelist: %w", err) + return fmt.Errorf("annotating Keycloak route with IP allowlist: %w", err) } - g.GinkgoWriter.Printf("restricted keycloak route to proxy IP %s\n", proxyIP) - - return func(ctx context.Context) error { - return retry.RetryOnConflict(retry.DefaultRetry, func() error { - route, err := routeClient.Get(ctx, keycloakResourceName, metav1.GetOptions{}) - if err != nil { - if apierrors.IsNotFound(err) { - return nil - } - return err - } - delete(route.Annotations, "haproxy.router.openshift.io/ip_whitelist") - _, err = routeClient.Update(ctx, route, metav1.UpdateOptions{}) - return err - }) - }, nil + g.GinkgoWriter.Printf("restricted Keycloak route to proxy IP %s\n", proxyIP) + return nil } func updateAuthenticationProxy(ctx context.Context, oc *exutil.CLI, proxy operatorv1.AuthenticationProxyConfig) error {