From 4f62ee9a1becad2c43129566874d233fa1f8674a Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Thu, 30 Jul 2026 15:07:46 +0200 Subject: [PATCH 01/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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 {