diff --git a/argocd-operator/controllers/argocd/sso.go b/argocd-operator/controllers/argocd/sso.go index d7c601a8679..203669a2523 100644 --- a/argocd-operator/controllers/argocd/sso.go +++ b/argocd-operator/controllers/argocd/sso.go @@ -55,6 +55,9 @@ func (r *ReconcileArgoCD) reconcileSSO(cr *argoproj.ArgoCD, argocdStatus *argopr // https://github.com/argoproj-labs/argocd-operator/pull/615 ==> conflict errMsg = "must supply valid dex configuration when requested SSO provider is dex" isError = true + } else if cr.Spec.SSO.Dex != nil && cr.Spec.SSO.Dex.OpenShiftOAuth && !IsOpenShiftCluster() { + errMsg = "openShiftOAuth is only supported on OpenShift clusters. Please disable the openShiftOAuth configuration." + isError = true } else if cr.Spec.SSO.Keycloak != nil { errMsg = "keycloak configuration is specified even though Dex is enabled. Keycloak support has been deprecated and is no longer available." isError = true diff --git a/argocd-operator/controllers/argocd/sso_test.go b/argocd-operator/controllers/argocd/sso_test.go index eee725c9564..50fdfecdf0d 100644 --- a/argocd-operator/controllers/argocd/sso_test.go +++ b/argocd-operator/controllers/argocd/sso_test.go @@ -39,6 +39,7 @@ func TestReconcile_illegalSSOConfiguration(t *testing.T) { tests := []struct { name string argoCD *argoproj.ArgoCD + openShiftCluster bool wantErr bool Err error wantSSOConfigLegalStatus string @@ -98,6 +99,34 @@ func TestReconcile_illegalSSOConfiguration(t *testing.T) { Err: errors.New("illegal SSO configuration: must supply valid dex configuration when requested SSO provider is dex"), wantSSOConfigLegalStatus: "Failed", }, + { + name: "openShiftOAuth true on non-OpenShift cluster", + argoCD: makeTestArgoCD(func(ac *argoproj.ArgoCD) { + ac.Spec.SSO = &argoproj.ArgoCDSSOSpec{ + Provider: argoproj.SSOProviderTypeDex, + Dex: &argoproj.ArgoCDDexSpec{ + OpenShiftOAuth: true, + }, + } + }), + wantErr: true, + Err: errors.New("illegal SSO configuration: openShiftOAuth is only supported on OpenShift clusters. Please disable the openShiftOAuth configuration."), + wantSSOConfigLegalStatus: "Failed", + }, + { + name: "openShiftOAuth true on OpenShift cluster", + argoCD: makeTestArgoCD(func(ac *argoproj.ArgoCD) { + ac.Spec.SSO = &argoproj.ArgoCDSSOSpec{ + Provider: argoproj.SSOProviderTypeDex, + Dex: &argoproj.ArgoCDDexSpec{ + OpenShiftOAuth: true, + }, + } + }), + openShiftCluster: true, + wantErr: false, + wantSSOConfigLegalStatus: "", + }, { name: "sso provider missing but sso.dex/keycloak supplied", argoCD: makeTestArgoCD(func(ac *argoproj.ArgoCD) { @@ -134,6 +163,10 @@ func TestReconcile_illegalSSOConfiguration(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + original := versionAPIFound + versionAPIFound = test.openShiftCluster + t.Cleanup(func() { versionAPIFound = original }) + resObjs := []client.Object{test.argoCD} subresObjs := []client.Object{test.argoCD} runtimeObjs := []runtime.Object{} diff --git a/test/openshift/e2e/ginkgo/fixture/argocd/fixture.go b/test/openshift/e2e/ginkgo/fixture/argocd/fixture.go index 7edfe805317..1be466c2a75 100644 --- a/test/openshift/e2e/ginkgo/fixture/argocd/fixture.go +++ b/test/openshift/e2e/ginkgo/fixture/argocd/fixture.go @@ -324,16 +324,16 @@ func LogInToDefaultArgoCDInstance() error { // NOTE: this should only be called from sequential tests. If you call it from a parallel test, there is a risk that another test will login to a different Argo CD instance. // -// Unlike LogInToDefaultArgoCDInstance, this logs in via the 'openshift-gitops-server' Route rather than a +// Unlike LogInToDefaultArgoCDInstance, this logs in via the Argo CD server Route rather than a // port-forward. Only use this if the test specifically needs to exercise the OpenShift Router code path (for // example, a regression test for a bug that only reproduces when traffic passes through the Route). -func LogInToDefaultArgoCDInstanceViaRoute() error { +func LogInToArgoCDInstanceViaRoute(argoCD *argov1beta1api.ArgoCD) error { k8sClient, _, err := utils.GetE2ETestKubeClientWithError() if err != nil { return err } - route := &routev1.Route{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-server", Namespace: "openshift-gitops"}} + route := &routev1.Route{ObjectMeta: metav1.ObjectMeta{Name: argoCD.Name + "-server", Namespace: argoCD.Namespace}} Eventually(func() error { return k8sClient.Get(context.Background(), client.ObjectKeyFromObject(route), route) @@ -341,9 +341,10 @@ func LogInToDefaultArgoCDInstanceViaRoute() error { Eventually(route, "3m", "2s").Should(routeFixture.HaveAdmittedIngress()) - secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-cluster", Namespace: "openshift-gitops"}} + secretName := argoCD.Name + "-cluster" + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: argoCD.Namespace}} if err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(secret), secret); err != nil { - return fmt.Errorf("unable to locate 'openshift-gitops-cluster' Secret") + return fmt.Errorf("unable to locate %q Secret", secretName) } // Note: '--skip-test-tls' parameter was added in Feb 2025, to work around OpenShift Routes not supporting HTTP2 by default, along with Argo CD upstream bugs https://github.com/argoproj/argo-cd/issues/21764, and https://github.com/argoproj/argo-cd/issues/20121 diff --git a/test/openshift/e2e/ginkgo/parallel/1-007_validate_volume_mounts_test.go b/test/openshift/e2e/ginkgo/parallel/1-007_validate_volume_mounts_test.go index 8bca3ab943a..e2f43100813 100644 --- a/test/openshift/e2e/ginkgo/parallel/1-007_validate_volume_mounts_test.go +++ b/test/openshift/e2e/ginkgo/parallel/1-007_validate_volume_mounts_test.go @@ -153,7 +153,15 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { SSO: &argov1beta1api.ArgoCDSSOSpec{ Provider: argov1beta1api.SSOProviderTypeDex, Dex: &argov1beta1api.ArgoCDDexSpec{ - OpenShiftOAuth: true, + Config: `connectors: + - type: github + id: github + name: github-using-first-class + config: + clientID: first-class + clientSecret: $dex.github.clientSecret + orgs: + - name: first-class`, Volumes: []corev1.Volume{ {Name: "custom-dex-volume", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}, }, diff --git a/test/openshift/e2e/ginkgo/parallel/1-042_restricted_pss_compliant_test.go b/test/openshift/e2e/ginkgo/parallel/1-042_restricted_pss_compliant_test.go index a42d0d23f2e..da0b34bc469 100644 --- a/test/openshift/e2e/ginkgo/parallel/1-042_restricted_pss_compliant_test.go +++ b/test/openshift/e2e/ginkgo/parallel/1-042_restricted_pss_compliant_test.go @@ -94,7 +94,15 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { SSO: &argov1beta1api.ArgoCDSSOSpec{ Provider: argov1beta1api.SSOProviderTypeDex, Dex: &argov1beta1api.ArgoCDDexSpec{ - OpenShiftOAuth: true, + Config: `connectors: + - type: github + id: github + name: github-using-first-class + config: + clientID: first-class + clientSecret: $dex.github.clientSecret + orgs: + - name: first-class`, }, }, }, diff --git a/test/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.go b/test/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.go index 169e389e05c..fd8fe6b9a0c 100644 --- a/test/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.go +++ b/test/openshift/e2e/ginkgo/parallel/1-050_validate_sso_test.go @@ -126,7 +126,7 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { ns, cleanupFunc = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() - By("creating a new Argo CD instance with dex and openshift oauth enabled") + By("creating a new Argo CD instance with dex") newArgoCD := &argov1beta1api.ArgoCD{ ObjectMeta: metav1.ObjectMeta{ @@ -137,7 +137,15 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { SSO: &argov1beta1api.ArgoCDSSOSpec{ Provider: "dex", Dex: &argov1beta1api.ArgoCDDexSpec{ - OpenShiftOAuth: true, + Config: `connectors: + - type: github + id: github + name: github-using-first-class + config: + clientID: first-class + clientSecret: $dex.github.clientSecret + orgs: + - name: first-class`, }, }, }, diff --git a/test/openshift/e2e/ginkgo/parallel/1-095_validate_dex_clientsecret_test.go b/test/openshift/e2e/ginkgo/parallel/1-095_validate_dex_clientsecret_test.go index b9fbfb370af..8bf17eecfe3 100644 --- a/test/openshift/e2e/ginkgo/parallel/1-095_validate_dex_clientsecret_test.go +++ b/test/openshift/e2e/ginkgo/parallel/1-095_validate_dex_clientsecret_test.go @@ -72,7 +72,7 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { ctx = context.Background() }) - It("verifies that the Dex client secret is sourced from a short-lived TokenRequest token and is correctly set in argocd-secret", func() { + It("verifies that the Dex client secret is sourced from a short-lived TokenRequest token and is correctly set in argocd-secret", Label("openshift"), func() { By("creating simple Argo CD instance with Dex and Openshift OAuth enabled") ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() @@ -173,7 +173,8 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { }, "2m", "5s").Should(BeTrue(), "Dex client secret in argocd-secret must match the token in the dedicated Dex token Secret") }) - It("verifies the operator deletes legacy non-expiring Dex kubernetes.io/service-account-token Secrets and drops them from the Dex SA", func() { + // Openshift OAuth is not supported in xKS + It("verifies the operator deletes legacy non-expiring Dex kubernetes.io/service-account-token Secrets and drops them from the Dex SA", Label("openshift"), func() { By("creating simple Argo CD instance with Dex and Openshift OAuth enabled") ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() diff --git a/test/openshift/e2e/ginkgo/parallel/1-098_validate_dex_clientsecret_deprecated.go b/test/openshift/e2e/ginkgo/parallel/1-098_validate_dex_clientsecret_deprecated.go index f38c9346647..2290df79483 100644 --- a/test/openshift/e2e/ginkgo/parallel/1-098_validate_dex_clientsecret_deprecated.go +++ b/test/openshift/e2e/ginkgo/parallel/1-098_validate_dex_clientsecret_deprecated.go @@ -48,7 +48,7 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { ctx = context.Background() }) - It("validates that dex client secret is properly copied from service account token to argocd-secret", func() { + It("validates that dex client secret is properly copied from service account token to argocd-secret", Label("openshift"), func() { // Create namespace for this test and ensure cleanup namespace, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() diff --git a/test/openshift/e2e/ginkgo/parallel/1-143_dex_OpenshiftOAuth_test.go b/test/openshift/e2e/ginkgo/parallel/1-143_dex_OpenshiftOAuth_test.go new file mode 100644 index 00000000000..d16a91bd7e5 --- /dev/null +++ b/test/openshift/e2e/ginkgo/parallel/1-143_dex_OpenshiftOAuth_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package parallel + +import ( + "context" + + argov1beta1api "github.com/argoproj-labs/gitops-operator/argocd-operator/api/v1beta1" + argocdFixture "github.com/argoproj-labs/gitops-operator/argocd-operator/tests/ginkgo/fixture/argocd" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture" + fixtureUtils "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("GitOps Operator Parallel E2E Tests", func() { + + Context("1-143_dex_OpenshiftOAuth_on_xKS_OpenShift", func() { + + var ( + k8sClient client.Client + ctx context.Context + ) + + BeforeEach(func() { + fixture.EnsureParallelCleanSlate() + k8sClient, _ = fixtureUtils.GetE2ETestKubeClient() + ctx = context.Background() + }) + + It("should fail reconciliation when OpenShiftOAuth is enabled on a non-OpenShift cluster", Label("xks"), func() { + + By("creating namespace for test") + ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() + defer cleanupFunc() + + By("creating an ArgoCD instance with OpenShiftOAuth enabled") + argoCD := &argov1beta1api.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{Name: "argocd", Namespace: ns.Name}, + Spec: argov1beta1api.ArgoCDSpec{ + SSO: &argov1beta1api.ArgoCDSSOSpec{ + Provider: argov1beta1api.SSOProviderTypeDex, + Dex: &argov1beta1api.ArgoCDDexSpec{ + OpenShiftOAuth: true, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, argoCD)).To(Succeed()) + + By("verifying the ArgoCD instance fails due to illegal SSO configuration for OpenShiftOAuth") + Eventually(argoCD, "2m", "5s").Should( + And(argocdFixture.HavePhase("Failed"), + argocdFixture.HaveSSOStatus("Failed"), + )) + + Eventually(argoCD, "2m", "5s").Should(argocdFixture.HaveCondition(metav1.Condition{ + Message: "illegal SSO configuration: openShiftOAuth is only supported on OpenShift clusters. Please disable the openShiftOAuth configuration.", + Reason: "ErrorOccurred", + Status: "False", + Type: "Reconciled", + })) + }) + }) +}) diff --git a/test/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.go b/test/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.go index a56afed73a5..2e12e48c531 100644 --- a/test/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.go +++ b/test/openshift/e2e/ginkgo/sequential/1-064_validate_tcp_reset_error_test.go @@ -59,9 +59,6 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { fixture.OutputDebugOnFail("openshift-gitops", "test-1-27-custom") - if app != nil { - Expect(k8sClient.Delete(ctx, app)).To(Succeed()) - } if test_1_27_customNS != nil { Expect(k8sClient.Delete(ctx, test_1_27_customNS)).To(Succeed()) } @@ -71,15 +68,30 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { // This test is VERY similar to 1-027. - openshiftgitopsArgoCD, err := argocdFixture.GetOpenShiftGitOpsNSArgoCD() - Expect(err).ToNot(HaveOccurred()) + By("getting creating Argo CD instance in new namespace") + _, cleanup := fixture.CreateNamespaceWithCleanupFunc("argocd-027") + defer cleanup() + + ArgoCD := &v1beta1.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{Name: "argocd-027", Namespace: "argocd-027"}, + Spec: v1beta1.ArgoCDSpec{ + Server: v1beta1.ArgoCDServerSpec{ + Route: v1beta1.ArgoCDRouteSpec{ + Enabled: true, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, ArgoCD)).To(Succeed()) + + fixture.SetEnvInOperatorSubscriptionOrDeployment("ARGOCD_CLUSTER_CONFIG_NAMESPACES", "argocd-027") - By("verifying openshift-gitops Argo CD instance is available") - Eventually(openshiftgitopsArgoCD, "5m", "5s").Should(argocdFixture.BeAvailable()) + By("verifying argocd-027 Argo CD instance is available") + Eventually(ArgoCD, "5m", "5s").Should(argocdFixture.BeAvailable()) By("creating Argo CD Application in openshift-gitops namespace") app = &argocdv1alpha1.Application{ - ObjectMeta: metav1.ObjectMeta{Name: "1-27-argocd", Namespace: openshiftgitopsArgoCD.Namespace}, + ObjectMeta: metav1.ObjectMeta{Name: "1-27-argocd", Namespace: ArgoCD.Namespace}, Spec: argocdv1alpha1.ApplicationSpec{ Source: &argocdv1alpha1.ApplicationSource{ Path: "./test/examples/1-027_operand-from-git", @@ -87,7 +99,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { TargetRevision: "HEAD", }, Destination: argocdv1alpha1.ApplicationDestination{ - Namespace: openshiftgitopsArgoCD.Namespace, + Namespace: ArgoCD.Namespace, Server: "https://kubernetes.default.svc", }, Project: "default", @@ -101,13 +113,13 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { } Expect(k8sClient.Create(ctx, app)).To(Succeed()) - By("verifying test-1-27-custom NS is created and is managed by openshift-gitops, and Application deploys successfully") + By("verifying test-1-27-custom NS is created and is managed by argocd-027, and Application deploys successfully") test_1_27_customNS = &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{Name: "test-1-27-custom"}, } Eventually(test_1_27_customNS, "5m", "5s").Should(k8sFixture.ExistByName()) - Eventually(test_1_27_customNS).Should(namespaceFixture.HaveLabel("argocd.argoproj.io/managed-by", "openshift-gitops")) + Eventually(test_1_27_customNS).Should(namespaceFixture.HaveLabel("argocd.argoproj.io/managed-by", "argocd-027")) Eventually(app, "4m", "5s").Should(appFixture.HaveHealthStatusCode(health.HealthStatusHealthy)) Eventually(app, "4m", "5s").Should(appFixture.HaveSyncStatusCode(argocdv1alpha1.SyncStatusCodeSynced)) @@ -149,7 +161,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { Eventually(guestbookApp, "4m", "5s").Should(appFixture.HaveSyncStatusCode(argocdv1alpha1.SyncStatusCodeSynced)) By("verifying we can log in to Argo CD via CLI, via the Route (this test specifically validates behavior of Argo CD CLI when going through the OpenShift Router)") - Expect(argocdFixture.LogInToDefaultArgoCDInstanceViaRoute()).To(Succeed()) + Expect(argocdFixture.LogInToArgoCDInstanceViaRoute(ArgoCD)).To(Succeed()) By("retrieving the Argo CD app manifests via CLI, and verifying the command succeeds and that there is no 'TCP reset error' error")