Skip to content

Feat: disable GitOps Service and default instance on xKS clusters #1172

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.

Sign up for GitHub

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

Open
anandrkskd wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Feat: disable GitOps Service and default instance on xKS clusters #1172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1e8a7c0
disable default instance on xKS clusters
anandrkskd Jun 9, 2026
2b77b2b
Dex with OpenShiftOAuth: true fails on vanilla K8s because oauth.open…
anandrkskd Jun 10, 2026
495b3fb
use IsOpenshitCluster to gate openshift specific capabilities
anandrkskd Jun 12, 2026
00b53d8
add proper logs, remove deadcode, fix test
anandrkskd Jun 12, 2026
aecf34c
fix unit test
anandrkskd Jun 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions cmd/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ func main() {
registerComponentOrExit(mgr, argov1beta1api.AddToScheme)

// Setup Scheme for OpenShift Config if available
// Disables default Argo CD instance if the cluster doesn't contain OpenShift config API

if util.IsConfigAPIFound() {
registerComponentOrExit(mgr, configv1.AddToScheme)
}
Expand Down Expand Up @@ -254,13 +256,18 @@ func main() {
}
}

if err = (&controllers.ReconcileGitopsService{
Client: client,
Scheme: mgr.GetScheme(),
DisableDefaultInstall: strings.ToLower(os.Getenv(common.DisableDefaultInstallEnvVar)) == "true",
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "GitopsService")
os.Exit(1)
if util.IsOpenShiftCluster() {
if err = (&controllers.ReconcileGitopsService{
Client: client,
Scheme: mgr.GetScheme(),
DisableDefaultInstall: strings.ToLower(os.Getenv(common.DisableDefaultInstallEnvVar)) == "true",
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "GitopsService")
os.Exit(1)
}
} else {
setupLog.Info("skipping GitopsService controller setup", "reason", "OpenShift Config API not available")

}

if util.IsRouteAPIFound() {
Expand Down
11 changes: 10 additions & 1 deletion controllers/argocd/argocd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ import (

argoapp "github.com/argoproj-labs/argocd-operator/api/v1beta1"
argoappController "github.com/argoproj-labs/argocd-operator/controllers/argocd"
"github.com/redhat-developer/gitops-operator/controllers/util"
v1 "k8s.io/api/core/v1"
resourcev1 "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/yaml"
)

var log = logf.Log.WithName("controller_argocd")

var (
defaultAdminPolicy = "g, system:cluster-admins, role:admin\ng, cluster-admins, role:admin\n"
defaultScope = "[groups]"
Expand Down Expand Up @@ -90,7 +94,12 @@ func getArgoDexSpec() *argoapp.ArgoCDDexSpec {
}

func getArgoSSOSpec(client client.Client) *argoapp.ArgoCDSSOSpec {
if argoappController.IsOpenShiftCluster() && argoappController.IsExternalAuthenticationEnabledOnCluster(context.TODO(), client) {
if !util.IsOpenShiftCluster() {
log.Info("non-OpenShift cluster detected, skipping SSO/Dex configuration")
return nil
}
if argoappController.IsExternalAuthenticationEnabledOnCluster(context.TODO(), client) {
log.Info("external authentication enabled on cluster, skipping SSO/Dex configuration")
return nil
}
return &argoapp.ArgoCDSSOSpec{
Expand Down
24 changes: 24 additions & 0 deletions controllers/argocd/argocd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

argoapp "github.com/argoproj-labs/argocd-operator/api/v1beta1"
configv1 "github.com/openshift/api/config/v1"
"github.com/redhat-developer/gitops-operator/controllers/util"
"gotest.tools/assert"
v1 "k8s.io/api/core/v1"
resourcev1 "k8s.io/apimachinery/pkg/api/resource"
Expand All @@ -30,6 +31,9 @@ import (
)

func TestArgoCD(t *testing.T) {
util.SetConfigAPIFound(true)
defer util.SetConfigAPIFound(false)

scheme := runtime.NewScheme()
_ = argoapp.AddToScheme(scheme)
_ = configv1.AddToScheme(scheme)
Expand Down Expand Up @@ -199,6 +203,9 @@ func TestArgoCD(t *testing.T) {
}

func TestDexConfiguration(t *testing.T) {
util.SetConfigAPIFound(true)
defer util.SetConfigAPIFound(false)

scheme := runtime.NewScheme()
_ = argoapp.AddToScheme(scheme)
_ = configv1.AddToScheme(scheme)
Expand All @@ -223,3 +230,20 @@ func TestDexConfiguration(t *testing.T) {
}
assert.DeepEqual(t, testArgoCD.Spec.RBAC, testRBAC)
}

// kubernetes environment test, no defer required as the Config API is false by default
func TestSSOSkippedOnNonOpenShift(t *testing.T) {
util.SetConfigAPIFound(false)

scheme := runtime.NewScheme()
_ = argoapp.AddToScheme(scheme)
_ = configv1.AddToScheme(scheme)

fakeClient := fake.NewClientBuilder().
WithScheme(scheme).
Build()

testArgoCD, _ := NewCR("openshift-gitops", "openshift-gitops", fakeClient)

assert.Assert(t, testArgoCD.Spec.SSO == nil, "SSO should be nil on non-OpenShift clusters")
}
Comment on lines +235 to +249

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Inconsistent test cleanup: defer should reset to default value.

The defer on line 236 sets configAPIFound to true, but the default value in util.go is false. This is inconsistent with the other tests (TestArgoCD and TestDexConfiguration) which defer to false. For consistency and to avoid potential test pollution, the defer should reset to the actual default value.

🧪 Proposed fix 
 func TestSSOSkippedOnNonOpenShift(t *testing.T) {
 	util.SetConfigAPIFound(false)
-	defer util.SetConfigAPIFound(true)
+	defer util.SetConfigAPIFound(false)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestSSOSkippedOnNonOpenShift(t *testing.T) {
util.SetConfigAPIFound(false)
defer util.SetConfigAPIFound(true)
scheme := runtime.NewScheme()
_ = argoapp.AddToScheme(scheme)
_ = configv1.AddToScheme(scheme)
fakeClient := fake.NewClientBuilder().
WithScheme(scheme).
Build()
testArgoCD, _ := NewCR("openshift-gitops", "openshift-gitops", fakeClient)
assert.Assert(t, testArgoCD.Spec.SSO == nil, "SSO should be nil on non-OpenShift clusters")
}
func TestSSOSkippedOnNonOpenShift(t *testing.T) {
util.SetConfigAPIFound(false)
defer util.SetConfigAPIFound(false)
scheme := runtime.NewScheme()
_ = argoapp.AddToScheme(scheme)
_ = configv1.AddToScheme(scheme)
fakeClient := fake.NewClientBuilder().
WithScheme(scheme).
Build()
testArgoCD, _ := NewCR("openshift-gitops", "openshift-gitops", fakeClient)
assert.Assert(t, testArgoCD.Spec.SSO == nil, "SSO should be nil on non-OpenShift clusters")
}
🤖 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 `@controllers/argocd/argocd_test.go` around lines 234 - 249, The test
TestSSOSkippedOnNonOpenShift sets util.SetConfigAPIFound(false) but defers
util.SetConfigAPIFound(true), which mismatches the actual default (false) in
util.go and other tests (TestArgoCD, TestDexConfiguration); change the deferred
call in TestSSOSkippedOnNonOpenShift to util.SetConfigAPIFound(false) so the
test restores the real default and avoids cross-test pollution.

7 changes: 6 additions & 1 deletion controllers/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,16 @@ func InspectCluster() error {
return stderrors.Join(errs...)
}

// used as a shortcut to check if the cluster is an OpenShift cluster
// used as a shortcut to check if the Config.Openshift.io API is found
func IsConfigAPIFound() bool {
return configAPIFound
}

// used as a shortcut to check if the cluster is an OpenShift cluster
func IsOpenShiftCluster() bool {
return IsConfigAPIFound()
}

// verify if the Config.Openshift.io API is found
func verifyConfigAPI() error {
found, err := argoutil.VerifyAPI(configv1.GroupName, configv1.GroupVersion.Version)
Expand Down
Loading