From 90cee57215107c7b690968acc8a37e18685c15c1 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 25 Sep 2026 19:43:20 +0000 Subject: [PATCH 1/3] fix: detect Route API before Config API gate in InspectCluster Move verifyRouteAPI() to before the `if !configAPIFound { return nil }` gate in controllers/util/util.go InspectCluster(). On xKS clusters where route.openshift.io CRD is present without config.openshift.io, the Route API check was never reached because the early return skipped it. This caused a mismatch with argocd-operator's InspectCluster which checks Route API unconditionally, leading to a crash when the inner check detected Route and set up watches but the outer check had never registered Route in the scheme. Refs: GITOPS-11466 Co-Authored-By: Claude Opus 4.6 Signed-off-by: Chai Bot --- controllers/util/util.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/controllers/util/util.go b/controllers/util/util.go index b7cc6e89032..8675c2e7418 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -96,6 +96,9 @@ func InspectCluster() error { if err := verifyMonitoringAPI(); err != nil { errs = append(errs, err) } + if err := verifyRouteAPI(); err != nil { + errs = append(errs, err) + } if err := verifyConfigAPI(); err != nil { errs = append(errs, err) @@ -106,7 +109,6 @@ func InspectCluster() error { } for _, check := range []func() error{ - verifyRouteAPI, verifyConsoleAPI, verifyTemplateAPI, verifyAppsAPI, From 067bc8b804d7acdda51ae8ec59db0b204a0ddb56 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 25 Sep 2026 19:53:50 +0000 Subject: [PATCH 2/3] test: add unit test for Route API detection on xKS clusters Add TestInspectCluster_RouteDetectedWithoutConfigAPI to verify that InspectCluster() detects the Route API even when config.openshift.io is absent. This covers the xKS scenario fixed in the previous commit. To make InspectCluster testable, introduce a package-level verifyAPI function variable (defaulting to argoutil.VerifyAPI) that can be overridden in tests via SetVerifyAPI(). This follows the existing test helper pattern in test_util.go. Refs: GITOPS-11466 Co-Authored-By: Claude Opus 4.6 Signed-off-by: Chai Bot --- controllers/util/test_util.go | 13 ++++++++++ controllers/util/util.go | 20 +++++++++------ controllers/util/util_test.go | 46 +++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/controllers/util/test_util.go b/controllers/util/test_util.go index 962a5ddbed2..995497de05a 100644 --- a/controllers/util/test_util.go +++ b/controllers/util/test_util.go @@ -54,3 +54,16 @@ func SetOAuthAPIFound(found bool) { func SetOLMAPIFound(found bool) { olmAPIFound = found } + +// *** THIS SHOULD ONLY BE USED FOR UNIT TESTING *** +// SetVerifyAPI overrides the API verification function used by InspectCluster. +// Call with nil to restore the default (argoutil.VerifyAPI). +func SetVerifyAPI(fn func(string, string) (bool, error)) { + if fn == nil { + fn = defaultVerifyAPI + } + verifyAPI = fn +} + +// defaultVerifyAPI stores the original verifyAPI value for restoring in tests. +var defaultVerifyAPI = verifyAPI diff --git a/controllers/util/util.go b/controllers/util/util.go index 8675c2e7418..4d663aa906e 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -56,6 +56,10 @@ var ( appsAPIFound = false oauthAPIFound = false olmAPIFound = false + + // verifyAPI is the function used to check API group availability. + // It defaults to argoutil.VerifyAPI and can be overridden in tests. + verifyAPI = argoutil.VerifyAPI ) // GetClusterVersion returns the OpenShift Cluster version in which the operator is installed @@ -133,7 +137,7 @@ func IsOpenShiftCluster() bool { // verify if the Config.Openshift.io API is found func verifyConfigAPI() error { - found, err := argoutil.VerifyAPI(configv1.GroupName, configv1.GroupVersion.Version) + found, err := verifyAPI(configv1.GroupName, configv1.GroupVersion.Version) if err != nil { return err } @@ -147,7 +151,7 @@ func IsConsoleAPIFound() bool { } func verifyConsoleAPI() error { - found, err := argoutil.VerifyAPI(console.GroupName, console.GroupVersion.Version) + found, err := verifyAPI(console.GroupName, console.GroupVersion.Version) if err != nil { return err } @@ -161,7 +165,7 @@ func IsRouteAPIFound() bool { } func verifyRouteAPI() error { - found, err := argoutil.VerifyAPI(routev1.GroupName, routev1.GroupVersion.Version) + found, err := verifyAPI(routev1.GroupName, routev1.GroupVersion.Version) if err != nil { return err } @@ -170,7 +174,7 @@ func verifyRouteAPI() error { } func verifyMonitoringAPI() error { - found, err := argoutil.VerifyAPI( + found, err := verifyAPI( monitoringv1.SchemeGroupVersion.Group, monitoringv1.SchemeGroupVersion.Version, ) @@ -192,7 +196,7 @@ func IsTemplateAPIFound() bool { } func verifyTemplateAPI() error { - found, err := argoutil.VerifyAPI(templatev1.GroupName, templatev1.GroupVersion.Version) + found, err := verifyAPI(templatev1.GroupName, templatev1.GroupVersion.Version) if err != nil { return err } @@ -206,7 +210,7 @@ func IsAppsAPIFound() bool { } func verifyAppsAPI() error { - found, err := argoutil.VerifyAPI(oappsv1.GroupName, oappsv1.GroupVersion.Version) + found, err := verifyAPI(oappsv1.GroupName, oappsv1.GroupVersion.Version) if err != nil { return err } @@ -220,7 +224,7 @@ func IsOAuthAPIFound() bool { } func verifyOAuthAPI() error { - found, err := argoutil.VerifyAPI(oauthv1.GroupName, oauthv1.GroupVersion.Version) + found, err := verifyAPI(oauthv1.GroupName, oauthv1.GroupVersion.Version) if err != nil { return err } @@ -234,7 +238,7 @@ func IsOLMAPIFound() bool { } func verifyOLMAPI() error { - found, err := argoutil.VerifyAPI(operatorsv1.GroupVersion.Group, operatorsv1.GroupVersion.Version) + found, err := verifyAPI(operatorsv1.GroupVersion.Group, operatorsv1.GroupVersion.Version) if err != nil { return err } diff --git a/controllers/util/util_test.go b/controllers/util/util_test.go index 2a254894519..485d2a68ed2 100644 --- a/controllers/util/util_test.go +++ b/controllers/util/util_test.go @@ -20,6 +20,7 @@ import ( "testing" configv1 "github.com/openshift/api/config/v1" + routev1 "github.com/openshift/api/route/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -48,6 +49,51 @@ func TestGetClusterVersion(t *testing.T) { }) } +func TestInspectCluster_RouteDetectedWithoutConfigAPI(t *testing.T) { + // Save and restore package-level state so this test is hermetic. + origRoute := routeAPIFound + origConfig := configAPIFound + origOLM := olmAPIFound + origMonitoring := monitoringAPIFound + t.Cleanup(func() { + routeAPIFound = origRoute + configAPIFound = origConfig + olmAPIFound = origOLM + monitoringAPIFound = origMonitoring + SetVerifyAPI(nil) // restore default + }) + + // Reset all flags before the test. + routeAPIFound = false + configAPIFound = false + olmAPIFound = false + monitoringAPIFound = false + + // Mock API verification: route.openshift.io is present, config.openshift.io is not. + SetVerifyAPI(func(group, version string) (bool, error) { + if group == routev1.GroupName { + return true, nil + } + if group == configv1.GroupName { + return false, nil + } + // All other API groups are absent. + return false, nil + }) + + err := InspectCluster() + assertNoError(t, err) + + // The bug: before the fix, verifyRouteAPI was gated behind configAPIFound, + // so on xKS clusters with Route but without Config, Route was never detected. + if !IsRouteAPIFound() { + t.Fatal("IsRouteAPIFound() = false; want true when route.openshift.io is available without config.openshift.io") + } + if IsConfigAPIFound() { + t.Fatal("IsConfigAPIFound() = true; want false because config.openshift.io is absent") + } +} + func addKnownTypesToScheme(scheme *runtime.Scheme) { scheme.AddKnownTypes(configv1.GroupVersion, &configv1.ClusterVersion{}) } From 35d19d2e3b97ad73a79c519cb2bfb8ccb54a380e Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Fri, 25 Sep 2026 19:56:12 +0000 Subject: [PATCH 3/3] fix: propagate discovery errors when Config API is absent Change the `if !configAPIFound` early-return path in InspectCluster() from `return nil` to `return stderrors.Join(errs...)` so that any accumulated OLM, Monitoring, or Route discovery errors are propagated to the caller even on non-OpenShift clusters. Also add a Go doc comment to InspectCluster explaining its behaviour. Refs: GITOPS-11466 Co-Authored-By: Claude Opus 4.6 Signed-off-by: Chai Bot --- controllers/util/util.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/controllers/util/util.go b/controllers/util/util.go index 4d663aa906e..5714553868c 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -92,6 +92,11 @@ func NewClusterVersion(version string) *configv1.ClusterVersion { } } +// InspectCluster probes the API server to determine which optional API groups +// (OLM, Monitoring, Route, Config, Console, Template, Apps, OAuth) are available +// in the cluster and sets the corresponding package-level flags. On non-OpenShift +// clusters where config.openshift.io is absent, only OLM, Monitoring, and Route +// APIs are checked; remaining OpenShift-specific groups are skipped. func InspectCluster() error { var errs []error if err := verifyOLMAPI(); err != nil { @@ -109,7 +114,7 @@ func InspectCluster() error { return stderrors.Join(errs...) } if !configAPIFound { - return nil + return stderrors.Join(errs...) } for _, check := range []func() error{