Skip to content
Open
169 changes: 169 additions & 0 deletions test/extended/authentication/component_proxy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
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"
operator "github.com/openshift/origin/test/extended/util/operator"
)

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("Saving auth state for restore after test")
authRestore, err := saveAndRestoreAuthState(ctx, oc)
g.DeferCleanup(authRestore)
o.Expect(err).NotTo(o.HaveOccurred())

g.By("Deploying Squid forward proxy")
var proxyCleanup removalFunc
httpProxyURL, httpsProxyURL, caCertPEM, proxyNamespace, proxyCleanup, err = deploySquidProxy(ctx, oc)
g.DeferCleanup(proxyCleanup)
o.Expect(err).NotTo(o.HaveOccurred())
Comment on lines +34 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Register cleanup functions only after you assert the error. Both files pass a cleanup function to g.DeferCleanup before checking the error from the helper that returned it. If the helper fails and returns a nil function, g.DeferCleanup receives nil and panics, which replaces the real assertion failure.

  • test/extended/authentication/component_proxy.go#L34-L43: move o.Expect(err).NotTo(o.HaveOccurred()) above g.DeferCleanup(authRestore) and above g.DeferCleanup(proxyCleanup).
  • test/extended/authentication/component_proxy_oauth.go#L88-L91: move o.Expect(err).NotTo(o.HaveOccurred()) above g.DeferCleanup(authRestore).
📍 Affects 2 files
  • test/extended/authentication/component_proxy.go#L34-L43 (this comment)
  • test/extended/authentication/component_proxy_oauth.go#L88-L91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/authentication/component_proxy.go` around lines 34 - 43,
Register each cleanup only after its helper error assertion succeeds: in
test/extended/authentication/component_proxy.go lines 34-43, move the assertions
before DeferCleanup(authRestore) and DeferCleanup(proxyCleanup); in
test/extended/authentication/component_proxy_oauth.go lines 88-91, move the
assertion before DeferCleanup(authRestore).


g.By("Deploying Keycloak (without registering IdP yet)")
var kcCleanups []removalFunc
kcSetup, kcCleanups, err = deployKeycloakForProxy(ctx, oc)
g.DeferCleanup(func() {
g.GinkgoWriter.Println("cleanup: removing Keycloak resources")
_ = removeResources(ctx, kcCleanups...)
})
o.Expect(err).NotTo(o.HaveOccurred())

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)
g.GinkgoWriter.Printf("Keycloak issuer URL: %s\n", kcSetup.issuerURL)
g.GinkgoWriter.Printf("Keycloak namespace: %s\n", kcSetup.namespace)
})
Comment on lines +34 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Restore the authentication state before you delete the proxy and Keycloak.

g.DeferCleanup runs in LIFO order. Here authRestore is registered first, so it runs last. Teardown therefore deletes the Squid namespace and Keycloak while the Authentication CR still points at the proxy URL and the Keycloak IdP. The authentication operator can degrade during teardown and affect later serial specs. component_proxy_oauth.go registers authRestore last, which produces the correct order. Align this file with that order.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended/authentication/component_proxy.go` around lines 34 - 61, The
cleanup registrations in the setup block currently restore authentication state
after deleting Squid and Keycloak resources. Move the
g.DeferCleanup(authRestore) registration to after the proxy and Keycloak cleanup
registrations, matching the ordering used by component_proxy_oauth.go so
authentication is restored first during LIFO teardown.


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)
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")
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)
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")
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())
}
Loading