-
Notifications
You must be signed in to change notification settings - Fork 4.8k
[WIP] CNTRLPLANE-3851: Oauth server proxy config e2e #31463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
4f62ee9
07369cb
d12c3dc
8d852b9
c31e2a4
930322d
4b4aa33
9f3ea8e
5f050ab
c2dc765
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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()) | ||
|
|
||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
🤖 Prompt for AI Agents |
||
|
|
||
| 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()) | ||
| } | ||
There was a problem hiding this comment.
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.DeferCleanupbefore checking the error from the helper that returned it. If the helper fails and returns a nil function,g.DeferCleanupreceives nil and panics, which replaces the real assertion failure.test/extended/authentication/component_proxy.go#L34-L43: moveo.Expect(err).NotTo(o.HaveOccurred())aboveg.DeferCleanup(authRestore)and aboveg.DeferCleanup(proxyCleanup).test/extended/authentication/component_proxy_oauth.go#L88-L91: moveo.Expect(err).NotTo(o.HaveOccurred())aboveg.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