From 4f62ee9a1becad2c43129566874d233fa1f8674a Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Thu, 30 Jul 2026 15:07:46 +0200 Subject: [PATCH 01/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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 c2dc765b0bf4a53fc85187fe9ab1ec0bebf9abb0 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Fri, 31 Jul 2026 14:48:22 +0100 Subject: [PATCH 10/10] add e2e tests for oauth-server functionality --- .../authentication/component_proxy_oauth.go | 489 ++++++++++++++++++ 1 file changed, 489 insertions(+) create mode 100644 test/extended/authentication/component_proxy_oauth.go diff --git a/test/extended/authentication/component_proxy_oauth.go b/test/extended/authentication/component_proxy_oauth.go new file mode 100644 index 000000000000..9dc261f41006 --- /dev/null +++ b/test/extended/authentication/component_proxy_oauth.go @@ -0,0 +1,489 @@ +package authentication + +import ( + "context" + "crypto/x509" + "encoding/pem" + "fmt" + "io" + "net/url" + "time" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/library-go/pkg/oauth/tokenrequest" + "github.com/openshift/library-go/pkg/oauth/tokenrequest/challengehandlers" + exutil "github.com/openshift/origin/test/extended/util" + operator "github.com/openshift/origin/test/extended/util/operator" + authnv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" +) + +var _ = g.Describe("[sig-auth][Suite:openshift/conformance/serial][OCPFeatureGate:AuthenticationComponentProxy][Serial][Slow]", g.Ordered, func() { + testNS := "authentication-proxy-config-oauth-server" + oc := exutil.NewCLIWithoutNamespace(testNS) + + var ( + ctx context.Context + httpProxyURL string + httpsProxyURL string + caCertPEM []byte + proxyNamespace string + kcSetup *keycloakProxySetup + kcCleanups []removalFunc + + kcUser, kcPass, kcGroup string + ) + + g.BeforeEach(func() { + ctx = context.Background() + // Is the authentication operator stable? + g.By("Waiting for operators to be stable before test") + err := operator.WaitForOperatorsToSettle(ctx, oc.AdminConfigClient(), 10) + o.Expect(err).NotTo(o.HaveOccurred()) + // If so, proceeed. + // Deploy Squid Proxy + 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) + + testID := rand.String(8) + + 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() { + _ = removeResources(ctx, kcCleanups...) + }) + + kcClient, err := kcSetup.client.GetClientByClientID(kcSetup.clientID) + o.Expect(err).NotTo(o.HaveOccurred()) + err = kcSetup.client.UpdateClientRaw(kcClient.ID, map[string]interface{}{ + "directAccessGrantsEnabled": true, + }) + o.Expect(err).NotTo(o.HaveOccurred()) + + kcGroup = fmt.Sprintf("e2e-proxy-kc-group-%s", testID) + kcUser = fmt.Sprintf("e2e-proxy-kc-user-%s", testID) + kcPass = fmt.Sprintf("e2e-proxy-kc-pass-%s", testID) + + err = kcSetup.client.CreateGroup(kcGroup) + o.Expect(err).NotTo(o.HaveOccurred()) + + err = kcSetup.client.CreateUser(kcUser, kcPass, kcGroup) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Saving auth state for restore after test") + authRestore, err := saveAndRestoreAuthState(ctx, oc) + g.DeferCleanup(authRestore) + o.Expect(err).NotTo(o.HaveOccurred()) + + 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) + }) + + // The tests can configure auth proxy as per their requirements. + g.It("should set partial and full env vars when configured", func() { + // Apply Auth Config with partial env vars + operatorClient := oc.AdminOperatorClient() + + operatorAuth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpProxyURL, + } + _, err = operatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + // Check if authentication operator stablises + g.By("Waiting for operator to reconcile proxy config") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying oauth-server has HTTPS_PROXY but not HTTP_PROXY") + err = verifyOAuthServerDeploymentProxyConfig( + ctx, oc, "", httpProxyURL, ".cluster.local,.svc,127.0.0.1,localhost", + false) + o.Expect(err).NotTo(o.HaveOccurred()) + + // Apply Auth Config with full env vars + configMapName := "e2e-proxy-trusted-ca" + caConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configMapName, + Namespace: "openshift-config", + }, + Data: map[string]string{ + "ca-bundle.crt": string(caCertPEM), + }, + } + + kubeClient := oc.AdminKubeClient() + _, err = kubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, caConfigMap, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to create trustedCA ConfigMap") + g.DeferCleanup(func() { + if err := kubeClient.CoreV1().ConfigMaps("openshift-config").Delete(ctx, configMapName, metav1.DeleteOptions{}); err != nil { + g.GinkgoWriter.Printf("failed to clean up ConfigMap %s: %v\n", configMapName, err) + } + }) + + noProxyHost := "noproxy.example.com" + + operatorAuth, err = operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Setting httpProxy, httpsProxy, noProxy, and trustedCA in component proxy config") + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPProxy: httpProxyURL, + HTTPSProxy: httpsProxyURL, + NoProxy: []string{noProxyHost}, + TrustedCA: operatorv1.AuthenticationConfigMapReference{ + Name: configMapName, + }, + } + _, err = operatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + // Check if authentication operator stabilises + g.By("Waiting for operator to reconcile proxy config") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Verifying oauth-server has HTTP_PROXY, HTTPS_PROXY, and NO_PROXY with custom entry") + err = verifyOAuthServerDeploymentProxyConfig(ctx, oc, httpProxyURL, httpsProxyURL, ".cluster.local,.svc,127.0.0.1,localhost,noproxy.example.com", true) + o.Expect(err).NotTo(o.HaveOccurred()) + }) + + g.It("should apply proxy config and perform full OIDC login flow", func() { + operatorClient := oc.AdminOperatorClient() + + operatorAuth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpProxyURL, + } + + _, err = operatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Registering Keycloak as OIDC IdP (operator discovers it through the proxy)") + idpCleans, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) + o.Expect(err).NotTo(o.HaveOccurred()) + kcCleanups = append(kcCleanups, idpCleans...) + + logCutOff := time.Now() + + g.By("Performing full OIDC login flow through component proxy") + assertOIDCLogin(ctx, oc, kcUser, kcPass, kcGroup) + + g.By("Verifying traffic went through the Squid proxy") + + issuerURL, err := url.Parse(kcSetup.issuerURL) + o.Expect(err).NotTo(o.HaveOccurred()) + keycloakHost := issuerURL.Hostname() + + g.By("Waiting for squid logs to settle before checking for proxy traffic") + time.Sleep(2 * time.Minute) + + logs, err := getSquidProxyLogsSince(ctx, oc, proxyNamespace, logCutOff) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(logs).To(o.ContainSubstring(keycloakHost), "squid logs should contain keycloak traffic after proxy login") + + g.By("Logging user out for next login") + deleteOIDCUserAndIdentities(ctx, oc, kcUser) + + g.By("Removing component-scoped proxy config") + operatorAuth, err = operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{} + _, err = operatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy removal") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + logCutoff := time.Now() + + g.By("Performing OIDC login flow via direct IdP connectivity after proxy removal") + assertOIDCLogin(ctx, oc, kcUser, kcPass, kcGroup) + + g.By("Waiting for squid logs to settle before checking for absence of proxy traffic") + time.Sleep(2 * time.Minute) + + postRemovalLogs, err := getSquidProxyLogsSince(ctx, oc, proxyNamespace, logCutoff) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(postRemovalLogs).NotTo(o.ContainSubstring(keycloakHost), "squid logs after proxy removal should not contain keycloak connect") + }) + + g.It("should hot-reload mounted CA file on change when spec.proxy.trustedCA is set", func() { + g.By("Creating config map with trustedCA") + caConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentProxyCAConfigMapName, + Namespace: "openshift-config", + }, + Data: map[string]string{ + "ca-bundle.crt": string(caCertPEM), + }, + } + kubeClient := oc.AdminKubeClient() + _, err := kubeClient.CoreV1().ConfigMaps("openshift-config").Create(ctx, caConfigMap, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to create trustedCA ConfigMap") + g.DeferCleanup(func() { + if err := kubeClient.CoreV1().ConfigMaps("openshift-config").Delete(ctx, componentProxyCAConfigMapName, metav1.DeleteOptions{}); err != nil { + g.GinkgoWriter.Printf("failed to clean up ConfigMap %s: %v\n", componentProxyCAConfigMapName, err) + } + }) + g.By("Setting component-scoped proxy with trustedCA") + operatorClient := oc.AdminOperatorClient() + operatorAuth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpsProxyURL, + TrustedCA: operatorv1.AuthenticationConfigMapReference{ + Name: componentProxyCAConfigMapName, + }, + } + _, err = operatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config with trustedCA") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Registering Keycloak as OIDC IdP (operator discovers it through the proxy)") + idpCleans, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) + o.Expect(err).NotTo(o.HaveOccurred()) + kcCleanups = append(kcCleanups, idpCleans...) + + logCutOff := time.Now() + + g.By("Verifying OIDC login works after setting proxy with trustedCA") + assertOIDCLogin(ctx, oc, kcUser, kcPass, kcGroup) + + g.By("Verifying traffic went through the Squid proxy") + issuerURL, err := url.Parse(kcSetup.issuerURL) + o.Expect(err).NotTo(o.HaveOccurred()) + keycloakHost := issuerURL.Hostname() + + time.Sleep(2 * time.Minute) + logs, err := getSquidProxyLogsSince(ctx, oc, proxyNamespace, logCutOff) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(logs).To(o.ContainSubstring(keycloakHost)) + + g.By("Verifying trustedCA ConfigMap is synced to openshift-authentication namespace") + err = verifyTrustedCAConfigMapSynced(ctx, oc) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Recording oauth-server pod names before CA rotation") + oauthServerPodList, err := kubeClient.CoreV1().Pods("openshift-authentication").List(ctx, metav1.ListOptions{LabelSelector: "app=oauth-openshift"}) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(oauthServerPodList.Items).NotTo(o.BeEmpty()) + + podNamesBefore := sets.New[string]() + for _, pod := range oauthServerPodList.Items { + podNamesBefore.Insert(pod.Name) + } + + g.By("Rotating CA: generating new CA and server cert") + newCA := mustNewCertificateAuthority(nil) + serviceDNS := fmt.Sprintf("squid-proxy.%s.svc.cluster.local", proxyNamespace) + newServerCert := mustNewServerCertificate(newCA, serviceDNS) + + newCACertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: newCA.Certificate.Raw}) + newServerCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: newServerCert.Certificate.Raw}) + newServerKeyDER, err := x509.MarshalPKCS8PrivateKey(newServerCert.PrivateKey) + o.Expect(err).NotTo(o.HaveOccurred()) + newServerKeyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: newServerKeyDER}) + + g.By("Updating squid-tls Secret with rotated cert") + tlsSecret, err := kubeClient.CoreV1().Secrets(proxyNamespace).Get(ctx, "squid-tls", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + tlsSecret.Data["tls.crt"] = newServerCertPEM + tlsSecret.Data["tls.key"] = newServerKeyPEM + _, err = kubeClient.CoreV1().Secrets(proxyNamespace).Update(ctx, tlsSecret, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Reconfiguring Squid to pick up new cert") + squidPods, err := kubeClient.CoreV1().Pods(proxyNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: "app=squid-proxy", + }) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(squidPods.Items).NotTo(o.BeEmpty()) + + output, err := oc.AsAdmin().Run("exec").Args( + "-n", proxyNamespace, + squidPods.Items[0].Name, + "-c", "squid", + "--", "/usr/sbin/squid", "-k", "reconfigure", + ).Output() + o.Expect(err).NotTo(o.HaveOccurred(), "squid reconfigure failed: %s", string(output)) + + g.By("Updating trustedCA ConfigMap with new CA") + cm, err := kubeClient.CoreV1().ConfigMaps("openshift-config").Get(ctx, componentProxyCAConfigMapName, metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + cm.Data["ca-bundle.crt"] = string(newCACertPEM) + _, err = kubeClient.CoreV1().ConfigMaps("openshift-config").Update(ctx, cm, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Logging user out for next login") + deleteOIDCUserAndIdentities(ctx, oc, kcUser) + + logCutOff = time.Now() + + g.By("Verifying OIDC login works after CA rotation") + assertOIDCLogin(ctx, oc, kcUser, kcPass, kcGroup) + + g.By("Verifying traffic went through the Squid proxy") + time.Sleep(2 * time.Minute) + logs, err = getSquidProxyLogsSince(ctx, oc, proxyNamespace, logCutOff) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(logs).To(o.ContainSubstring(keycloakHost)) + + g.By("Verifying oauth-server pods were NOT redeployed after CA rotation") + oauthServerPodListAfter, err := kubeClient.CoreV1().Pods("openshift-authentication").List(ctx, metav1.ListOptions{LabelSelector: "app=oauth-openshift"}) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(oauthServerPodListAfter.Items).NotTo(o.BeEmpty()) + + podNamesAfter := sets.New[string]() + for _, pod := range oauthServerPodListAfter.Items { + podNamesAfter.Insert(pod.Name) + } + + o.Expect(podNamesAfter.Equal(podNamesBefore)).To(o.BeTrue(), "oauth-server pods should not have been redeployed after CA file change") + }) + + g.It("should bypass proxy for noProxy hosts", func() { + g.By("Setting component-scoped proxy with noProxy") + operatorClient := oc.AdminOperatorClient() + operatorAuth, err := operatorClient.OperatorV1().Authentications().Get(ctx, "cluster", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + issuerURL, err := url.Parse(kcSetup.issuerURL) + o.Expect(err).NotTo(o.HaveOccurred()) + + keycloakHost := issuerURL.Hostname() + operatorAuth.Spec.Proxy = operatorv1.AuthenticationProxyConfig{ + HTTPSProxy: httpProxyURL, + NoProxy: []string{keycloakHost}, + } + + _, err = operatorClient.OperatorV1().Authentications().Update(ctx, operatorAuth, metav1.UpdateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Waiting for operator to reconcile proxy config") + err = waitForOperatorToPickUpChanges(ctx, oc, "authentication") + o.Expect(err).NotTo(o.HaveOccurred()) + + g.By("Registering Keycloak as OIDC IdP (operator discovers it through the proxy)") + idpCleans, err := addKeycloakOIDCIdPForProxy(ctx, oc, kcSetup) + o.Expect(err).NotTo(o.HaveOccurred()) + kcCleanups = append(kcCleanups, idpCleans...) + + g.By("Verifying OIDC login works after setting proxy with noProxy") + assertOIDCLogin(ctx, oc, kcUser, kcPass, kcGroup) + + g.By("Waiting for squid logs to settle before checking for absence of proxy traffic") + time.Sleep(2 * time.Minute) + + logs, err := getSquidProxyLogs(ctx, oc, proxyNamespace) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(logs).NotTo(o.ContainSubstring(keycloakHost), "squid logs should not contain keycloak connect") + }) +}) + +func assertOIDCLogin(ctx context.Context, oc *exutil.CLI, username, password, expectedGroup string) { + g.GinkgoHelper() + t := g.GinkgoTB() + + kubeConfig := oc.AdminConfig() + + routeClient := oc.AdminRouteClient() + route, err := routeClient.RouteV1().Routes("openshift-authentication").Get(ctx, "oauth-openshift", metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to get the OAuth server route") + oauthServerURL := fmt.Sprintf("https://%s", route.Spec.Host) + + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + tokenOpts := tokenrequest.NewRequestTokenOptions(rest.CopyConfig(kubeConfig), false) + tokenOpts, err := tokenOpts.WithChallengeHandlers( + challengehandlers.NewBasicChallengeHandler(oauthServerURL, "", nil, io.Discard, nil, username, password), + ) + if err != nil { + t.Logf("failed to create challenge handler: %v", err) + return false, nil + } + + token, err := tokenOpts.RequestToken() + if err != nil { + t.Logf("failed to request token: %v", err) + return false, nil + } + if token == "" { + t.Log("received empty token") + return false, nil + } + + tokenConfig := rest.AnonymousClientConfig(kubeConfig) + tokenConfig.BearerToken = token + tokenKubeClient, err := kubernetes.NewForConfig(tokenConfig) + if err != nil { + t.Logf("failed to create kube client with token: %v", err) + return false, nil + } + + ssr, err := tokenKubeClient.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + t.Logf("failed to create SelfSubjectReview: %v", err) + return false, nil + } + + if ssr.Status.UserInfo.Username == "" { + t.Log("SelfSubjectReview returned empty username") + return false, nil + } + + for _, g := range ssr.Status.UserInfo.Groups { + if g == expectedGroup { + return true, nil + } + } + t.Logf("expected group %q not found in groups: %v", expectedGroup, ssr.Status.UserInfo.Groups) + return false, nil + }) + o.Expect(err).NotTo(o.HaveOccurred(), "OIDC login flow should succeed") +} + +func deleteOIDCUserAndIdentities(ctx context.Context, oc *exutil.CLI, username string) { + g.GinkgoHelper() + userClient := oc.AdminUserClient().UserV1() + + user, err := userClient.Users().Get(ctx, username, metav1.GetOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to get user %q", username) + + for _, identity := range user.Identities { + err = userClient.Identities().Delete(ctx, identity, metav1.DeleteOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to delete identity %q", identity) + } + + err = userClient.Users().Delete(ctx, username, metav1.DeleteOptions{}) + o.Expect(err).NotTo(o.HaveOccurred(), "should be able to delete user %q", username) +}