Skip to content

DO-NOT-MERGE: POC: decouple oauthapiserver auth config generation for shared library use #942

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

Draft
liouk wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

DO-NOT-MERGE: POC: decouple oauthapiserver auth config generation for shared library use #942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 43 additions & 3 deletions pkg/controllers/externaloidc/externaloidc_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (
)

type authConfigGenerator interface {
GenerateAuthenticationConfiguration(*configv1.Authentication) (runtime.Object, error)
GenerateAuthenticationConfiguration(*configv1.AuthenticationSpec) (runtime.Object, error)
}

type externalOIDCController struct {
Expand All @@ -57,7 +57,19 @@ func NewExternalOIDCController(
authCfgGenerator = kubeapiserver.NewAuthenticationConfigurationGenerator(kubeInformersForNamespaces.ConfigMapLister(), featureGates)

if featureGates.Enabled(features.FeatureGateExternalOIDCExternalClaimsSourcing) {
authCfgGenerator = oauthapiserver.NewAuthenticationConfigurationGenerator(kubeInformersForNamespaces.ConfigMapLister(), kubeInformersForNamespaces.SecretLister(), featureGates)
gen := oauthapiserver.NewAuthenticationConfigurationGenerator(
newCABundleResolver(kubeInformersForNamespaces.ConfigMapLister(), configNamespace),
newClientSecretResolver(kubeInformersForNamespaces.SecretLister(), configNamespace),
).WithExternalClaimsSourcing()
Comment on lines +60 to +63

@coderabbitai coderabbitai Bot Jul 3, 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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the controller and related factory wiring
echo "== externaloidc_controller.go =="
cat -n pkg/controllers/externaloidc/externaloidc_controller.go | sed -n '1,180p'

echo
echo "== search for informer wiring and secret resolver usage =="
rg -n "newClientSecretResolver|WithInformers|Secrets\(\)\.Informer\(\)|SecretLister\(\)" pkg/controllers pkg/operator manifests -S

echo
echo "== search for RBAC references to openshift-config secrets =="
rg -n "openshift-config.*secrets|resources:.*secrets|verbs:.*(get|list|watch)" manifests pkg -S

Repository: openshift/cluster-authentication-operator

Length of output: 15186

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the controller factory and resolver helpers in smaller slices if needed
echo "== externaloidc_controller.go (selected range) =="
sed -n '1,180p' pkg/controllers/externaloidc/externaloidc_controller.go | cat -n

echo
echo "== client secret resolver helper =="
rg -n "func newClientSecretResolver|type .*clientSecretResolver|SecretLister" pkg/controllers/externaloidc -S

Repository: openshift/cluster-authentication-operator

Length of output: 9431

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== externaloidc_controller.go =="
cat -n pkg/controllers/externaloidc/externaloidc_controller.go | sed -n '1,180p'

echo
echo "== search for informer wiring and secret resolver usage =="
rg -n "newClientSecretResolver|WithInformers|Secrets\(\)\.Informer\(\)|SecretLister\(\)" pkg/controllers pkg/operator manifests -S

echo
echo "== search for RBAC references to openshift-config secrets =="
rg -n "openshift-config.*secrets|resources:.*secrets|verbs:.*(get|list|watch)" manifests pkg -S

Repository: openshift/cluster-authentication-operator

Length of output: 15186

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== KubeInformersForNamespaces definition =="
rg -n "type KubeInformersForNamespaces|func .*SecretLister|InformersFor\\(" $(git ls-files '*.go') -S

echo
echo "== starter wiring for openshift-config informers =="
cat -n pkg/operator/starter.go | sed -n '150,260p'

echo
echo "== inspect namespace informer implementation =="
rg -n "openshift-config.*Secrets\\(\\)|Core\\(\\)\\.V1\\(\\)\\.Secrets\\(\\)" pkg/operator pkg/controllers -S

Repository: openshift/cluster-authentication-operator

Length of output: 24801

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Focus on the factory/informer helper implementation that backs SecretLister.
fd -a ".*informers.*\.go$" pkg vendor | sed -n '1,120p'

Repository: openshift/cluster-authentication-operator

Length of output: 372

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate KubeInformersForNamespaces implementation in repo/vendor =="
rg -n "type KubeInformersForNamespaces|SecretLister\\(" . -S --glob '!**/.git/**'

echo
echo "== show starter informer factory setup =="
cat -n pkg/operator/starter.go | sed -n '1,260p'

Repository: openshift/cluster-authentication-operator

Length of output: 16992

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== controller registration and informer factories =="
rg -n "NewExternalOIDCController|kubeInformersForNamespaces|newInformerFactories|InformersFor\\(\"openshift-config\"" pkg/operator pkg/controllers -S

echo
echo "== snippet around external OIDC starter registration if present =="
cat -n pkg/operator/starter.go | sed -n '180,240p'

Repository: openshift/cluster-authentication-operator

Length of output: 15180

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== v1helpers informer implementation =="
cat -n vendor/github.com/openshift/library-go/pkg/operator/v1helpers/informers.go | sed -n '1,180p'

echo
echo "== replacement_starter.go informer factory setup =="
cat -n pkg/operator/replacement_starter.go | sed -n '230,330p'

echo
echo "== starter.go around externaloidc registration and openshift-config informers =="
cat -n pkg/operator/starter.go | sed -n '740,880p'

Repository: openshift/cluster-authentication-operator

Length of output: 20211

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== externaloidc resolver helpers =="
cat -n pkg/controllers/externaloidc/externaloidc_controller.go | sed -n '200,280p'

echo
echo "== RBAC for openshift-config secrets =="
rg -n "openshift-config|resources:.*secrets|verbs:.*(get|list|watch)" manifests pkg -S

echo
echo "== secret-related config/observer wiring =="
rg -n "SecretLister\\(|Secrets\\(\\)\\.Informer\\(\\)|newClientSecretResolver|ExternalClaimsSourcing" pkg/controllers/externaloidc pkg/operator -S

Repository: openshift/cluster-authentication-operator

Length of output: 19201

Add the openshift-config Secret informer
pkg/controllers/externaloidc/externaloidc_controller.go:85-94 — the shared SecretLister() cache is already available, but this controller never requeues on openshift-config secret updates. Client-secret rotations can therefore leave the generated auth config stale until some other watched resource changes. Add kubeInformersForNamespaces.InformersFor(configNamespace).Core().V1().Secrets().Informer() here.

🤖 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 `@pkg/controllers/externaloidc/externaloidc_controller.go` around lines 60 -
63, Add the missing openshift-config Secret informer to the external OIDC
controller so auth config regenerates when client secrets rotate. Update the
informer setup around NewAuthenticationConfigurationGenerator in
externaloidc_controller.go to include
kubeInformersForNamespaces.InformersFor(configNamespace).Core().V1().Secrets().Informer()
alongside the existing ConfigMap and Secret listers, ensuring the controller
requeues on secret updates.

Source: Coding guidelines


if featureGates.Enabled(features.FeatureGateExternalOIDCWithUpstreamParity) {
gen.WithUpstreamParity()
}
if featureGates.Enabled(features.FeatureGateExternalOIDCWithAdditionalClaimMappings) {
gen.WithAdditionalClaimMappings()
}

authCfgGenerator = gen
}

c := &externalOIDCController{
Expand Down Expand Up @@ -95,7 +107,7 @@ func (c *externalOIDCController) sync(ctx context.Context, syncCtx factory.SyncC
return c.deleteAuthConfig(ctx, syncCtx)
}

authConfig, err := c.authConfigGenerator.GenerateAuthenticationConfiguration(auth)
authConfig, err := c.authConfigGenerator.GenerateAuthenticationConfiguration(&auth.Spec)
if err != nil {
return err
}
Expand Down Expand Up @@ -174,3 +186,31 @@ func (c *externalOIDCController) getExistingApplyConfig() (*corev1ac.ConfigMapAp

return existingCMApplyConfig, nil
}

func newCABundleResolver(cmLister corev1listers.ConfigMapLister, namespace string) oauthapiserver.ResolverFunc {
return func(name string) (string, error) {
cm, err := cmLister.ConfigMaps(namespace).Get(name)
if err != nil {
return "", fmt.Errorf("could not retrieve configmap %s/%s: %v", namespace, name, err)
}
caData, ok := cm.Data["ca-bundle.crt"]
if !ok || len(caData) == 0 {
return "", fmt.Errorf("configmap %s/%s key \"ca-bundle.crt\" missing or empty", namespace, name)
}
return caData, nil
}
}

func newClientSecretResolver(secretLister corev1listers.SecretLister, namespace string) oauthapiserver.ResolverFunc {
return func(name string) (string, error) {
secret, err := secretLister.Secrets(namespace).Get(name)
if err != nil {
return "", fmt.Errorf("could not retrieve secret %s/%s: %v", namespace, name, err)
}
clientSecret, ok := secret.Data["client-secret"]
if !ok || len(clientSecret) == 0 {
return "", fmt.Errorf("secret %s/%s key \"client-secret\" missing or empty", namespace, name)
}
return string(clientSecret), nil
}
}
2 changes: 1 addition & 1 deletion pkg/controllers/externaloidc/externaloidc_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,6 @@ type mockAuthConfigGenerator[T runtime.Object] struct {
err error
}

func (macg *mockAuthConfigGenerator[T]) GenerateAuthenticationConfiguration(_ *configv1.Authentication) (runtime.Object, error) {
func (macg *mockAuthConfigGenerator[T]) GenerateAuthenticationConfiguration(_ *configv1.AuthenticationSpec) (runtime.Object, error) {
return macg.cfg, macg.err
}
6 changes: 3 additions & 3 deletions pkg/controllers/externaloidc/generation/kubeapiserver/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func NewAuthenticationConfigurationGenerator(cmlister corev1listers.ConfigMapLis

// GenerateAuthenticationConfiguration creates a structured JWT AuthenticationConfiguration for OIDC
// in the kube-apiserver from the configuration found in the authentication/cluster resource.
func (acg *AuthenticationConfigurationGenerator) GenerateAuthenticationConfiguration(auth *configv1.Authentication) (runtime.Object, error) {
func (acg *AuthenticationConfigurationGenerator) GenerateAuthenticationConfiguration(authSpec *configv1.AuthenticationSpec) (runtime.Object, error) {
authConfig := &apiserverv1beta1.AuthenticationConfiguration{
TypeMeta: metav1.TypeMeta{
Kind: kindAuthenticationConfiguration,
Expand All @@ -71,8 +71,8 @@ func (acg *AuthenticationConfigurationGenerator) GenerateAuthenticationConfigura
}

errs := []error{}
for _, provider := range auth.Spec.OIDCProviders {
jwt, err := generateJWTForProvider(provider, acg.configMapLister, acg.featureGates, auth.Spec.ServiceAccountIssuer)
for _, provider := range authSpec.OIDCProviders {
jwt, err := generateJWTForProvider(provider, acg.configMapLister, acg.featureGates, authSpec.ServiceAccountIssuer)
if err != nil {
errs = append(errs, err)
continue
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/externaloidc/generation/kubeapiserver/generate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ func TestAuthenticationConfigurationGeneratorGenerateAuthenticationConfiguration
c := NewAuthenticationConfigurationGenerator(corev1listers.NewConfigMapLister(tt.configMapIndexer), tt.featureGates)
c.validationFn = tt.configValidator

gotConfig, err := c.GenerateAuthenticationConfiguration(&tt.auth)
gotConfig, err := c.GenerateAuthenticationConfiguration(&tt.auth.Spec)
if tt.expectError && err == nil {
t.Fatalf("expected error but didn't get any")
}
Expand Down
Loading