From 4d280bd034e712336b663b40421948718a8c1b07 Mon Sep 17 00:00:00 2001 From: Ilias Rinis Date: Thu, 16 Oct 2025 11:00:21 +0200 Subject: [PATCH 1/2] externaloidc: return errors when node statuses cannot be used to determine oidc state --- pkg/controllers/common/external_oidc.go | 17 ++++++++++++-- pkg/controllers/common/external_oidc_test.go | 24 +++++++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/common/external_oidc.go b/pkg/controllers/common/external_oidc.go index f257e66b9..b59806006 100644 --- a/pkg/controllers/common/external_oidc.go +++ b/pkg/controllers/common/external_oidc.go @@ -67,13 +67,26 @@ func (c *AuthConfigChecker) OIDCAvailable() (bool, error) { return false, fmt.Errorf("getting kubeapiservers.operator.openshift.io/cluster: %v", err) } + if len(kas.Status.NodeStatuses) == 0 { + return false, fmt.Errorf("determining observed revisions in kubeapiservers.operator.openshift.io/cluster; no node statuses found") + } + observedRevisions := sets.New[int32]() + nodesWithEmptyRevision := false for _, nodeStatus := range kas.Status.NodeStatuses { - observedRevisions.Insert(nodeStatus.CurrentRevision) + if nodeStatus.CurrentRevision > 0 { + observedRevisions.Insert(nodeStatus.CurrentRevision) + } else { + nodesWithEmptyRevision = true + } + } + + if nodesWithEmptyRevision { + return false, fmt.Errorf("determining observed revisions in kubeapiservers.operator.openshift.io/cluster; some nodes do not have a valid CurrentRevision") } if observedRevisions.Len() == 0 { - return false, nil + return false, fmt.Errorf("determining observed revisions in kubeapiservers.operator.openshift.io/cluster; no observed revisions found") } for _, revision := range observedRevisions.UnsortedList() { diff --git a/pkg/controllers/common/external_oidc_test.go b/pkg/controllers/common/external_oidc_test.go index 5c84ed19a..975a169aa 100644 --- a/pkg/controllers/common/external_oidc_test.go +++ b/pkg/controllers/common/external_oidc_test.go @@ -32,7 +32,29 @@ func TestExternalOIDCConfigAvailable(t *testing.T) { name: "no node statuses observed", authType: configv1.AuthenticationTypeOIDC, expectAvailable: false, - expectError: false, + expectError: true, + }, + { + name: "some node revisions are zero", + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 10}, + {CurrentRevision: 10}, + {CurrentRevision: 0}, + }, + expectAvailable: false, + expectError: true, + }, + { + name: "node revisions are zero", + authType: configv1.AuthenticationTypeOIDC, + nodeStatuses: []operatorv1.NodeStatus{ + {CurrentRevision: 0}, + {CurrentRevision: 0}, + {CurrentRevision: 0}, + }, + expectAvailable: false, + expectError: true, }, { name: "oidc disabled, no rollout", From 702bf575d5832f835ff7c3678b7cd97c89524c64 Mon Sep 17 00:00:00 2001 From: Ilias Rinis Date: Mon, 3 Nov 2025 10:01:16 +0100 Subject: [PATCH 2/2] DO-NOT-MERGE: add debug logging --- pkg/controllers/common/external_oidc.go | 8 ++++++++ .../endpointaccessible/endpoint_accessible_controller.go | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/pkg/controllers/common/external_oidc.go b/pkg/controllers/common/external_oidc.go index b59806006..d0a1754da 100644 --- a/pkg/controllers/common/external_oidc.go +++ b/pkg/controllers/common/external_oidc.go @@ -12,6 +12,7 @@ import ( "github.com/openshift/library-go/pkg/controller/factory" corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/tools/cache" + "k8s.io/klog/v2" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/sets" @@ -75,6 +76,7 @@ func (c *AuthConfigChecker) OIDCAvailable() (bool, error) { nodesWithEmptyRevision := false for _, nodeStatus := range kas.Status.NodeStatuses { if nodeStatus.CurrentRevision > 0 { + klog.Infof("[debug-801] node '%s' is on revision %d", nodeStatus.NodeName, nodeStatus.CurrentRevision) observedRevisions.Insert(nodeStatus.CurrentRevision) } else { nodesWithEmptyRevision = true @@ -89,10 +91,15 @@ func (c *AuthConfigChecker) OIDCAvailable() (bool, error) { return false, fmt.Errorf("determining observed revisions in kubeapiservers.operator.openshift.io/cluster; no observed revisions found") } + if !c.kasNamespaceConfigMapsInformer.HasSynced() { + return false, fmt.Errorf("configmaps informer has not synced yet") + } + for _, revision := range observedRevisions.UnsortedList() { // ensure every observed revision includes an auth-config revisioned configmap _, err := c.kasConfigMapLister.ConfigMaps("openshift-kube-apiserver").Get(fmt.Sprintf("auth-config-%d", revision)) if errors.IsNotFound(err) { + klog.Infof("[debug-801] configmap auth-config-%d not found; informer HasSynced=%v", revision, c.kasNamespaceConfigMapsInformer.HasSynced()) return false, nil } else if err != nil { return false, fmt.Errorf("getting configmap openshift-kube-apiserver/auth-config-%d: %v", revision, err) @@ -109,6 +116,7 @@ func (c *AuthConfigChecker) OIDCAvailable() (bool, error) { if !strings.Contains(cm.Data["config.yaml"], `"oauthMetadataFile":""`) || strings.Contains(cm.Data["config.yaml"], `"authentication-token-webhook-config-file":`) || !strings.Contains(cm.Data["config.yaml"], `"authentication-config":["/etc/kubernetes/static-pod-resources/configmaps/auth-config/auth-config.json"]`) { + klog.Infof("[debug-801] configmap config-%d does not contain expected OIDC config", revision) return false, nil } } diff --git a/pkg/libs/endpointaccessible/endpoint_accessible_controller.go b/pkg/libs/endpointaccessible/endpoint_accessible_controller.go index 686d52630..b41266bfc 100644 --- a/pkg/libs/endpointaccessible/endpoint_accessible_controller.go +++ b/pkg/libs/endpointaccessible/endpoint_accessible_controller.go @@ -12,6 +12,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/klog/v2" operatorv1 "github.com/openshift/api/operator/v1" applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" @@ -78,14 +79,20 @@ func humanizeError(err error) error { func (c *endpointAccessibleController) sync(ctx context.Context, syncCtx factory.SyncContext) error { if c.endpointCheckDisabledFunc != nil { + klog.Infof("[debug-801] found non-nil endpointCheckDisabledFunc") if skip, err := c.endpointCheckDisabledFunc(); err != nil { + klog.Errorf("[debug-801] endpointCheckDisabledFunc returned an error: %v", err) return err } else if skip { // Server-Side-Apply with an empty operator status for the specific field manager // will effectively remove any conditions owned by it since the list type in the // API definition is 'map' + klog.Infof("[debug-801] endpointCheckDisabledFunc returned true; skipping endpoint check") return c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, applyoperatorv1.OperatorStatus()) } + klog.Infof("[debug-801] endpointCheckDisabledFunc returned false; will not skip endpoint check") + } else { + klog.Infof("[debug-801] endpointCheckDisabledFunc is nil; will not skip endpoint check") } endpoints, err := c.endpointListFn()