Added OIDC client

[dasarinaidu]

1. Added OIDC client
2. [RFE] Support OAUTH2 rancher#52716

🧠 Helper Classification — classify-helper-type Skill Applied

All new symbols were audited using @Priyashetty17's classify-helper-type-validation-tests skill before placement. Reference: Actions vs Extensions — Confluence

---

📦 shepherd PR #547 — Extensions & Clients

Symbol	Location	Bucket	Rubric Criterion
Discovery(httpClient, rancherURL)	extensions/auth/oidc/oidc.go	Extension	Bridges API/client gap (raw HTTP); generic params; no defaults
RefreshAccessToken(httpClient, rancherURL, ...)	extensions/auth/oidc/oidc.go	Extension	Same as Discovery — raw HTTP, generic params
GeneratePKCE()	extensions/auth/oidc/pkce.go	Extension	Pure utility; no defaults; reusable outside rancher/rancher
DecodeJWTPayload, TamperJWTSignature	extensions/auth/oidc/jwt.go	Extension	Pure utilities; no defaults
GVK consts, OIDC path consts, TokenSet, PKCEPair	extensions/auth/oidc/	Extension	Structs and Templates — plain data types reusable across projects
CreateOIDCClient(client, name, spec)	extensions/auth/oidcclient/oidcclient.go	Extension	Generic params; no defaults applied — caller passes spec values; AlreadyExists treated as idempotent
WaitForOIDCClientReady(client, name)	extensions/auth/oidcclient/oidcclient.go	Extension	"Waits are acceptable" — generic CRD-status wait, no test assertions
FetchOIDCClientSecret(client, clientID, key)	extensions/auth/oidcclient/oidcclient.go	Extension	Single Get from Rancher canonical namespace; no test defaults
DeleteOIDCClient(client, name)	extensions/auth/oidcclient/oidcclient.go	Extension	Generic delete + NotFound idempotency for cleanup paths
OIDCClientSecretNamespace const	extensions/auth/oidcclient/oidcclient.go	Extension	Rancher canonical namespace (not a test default)
IsFeatureEnabled(client, name)	extensions/features/features.go	Extension	Generic singular Get + status check
EnableFeatureFlag(client, name)	extensions/features/features.go	Extension	Generic params (any flag name); singular CRUD + cleanup registration
DisableFeatureFlag(client, name)	extensions/features/features.go	Extension	Generic params; single update
WaitForRancherReady(client)	extensions/features/features.go	Extension	Single resource wait on canonical Rancher deployment
APIClient + NewAPIClient(rancherURL, session)	clients/rancher/auth/oidc/oidc.go	clients/	Matches openldap.NewOLDAP / scim.NewClient pattern — test-config-aware HTTP wrapper
Config struct, ConfigurationFileKey, OIDCProviderFeatureFlag, Default* consts	clients/rancher/auth/oidc/config.go	clients/	Matches openldap.Config precedent
CompleteAuthCodeFlow(...) (method on APIClient)	clients/rancher/auth/oidc/oidc.go	clients/	Matches OLDAPClient.Enable() precedent — multi-step provider workflow as method on provider client
Do(client, method, url, body, headers), Response	pkg/clientbase/raw.go	pkg/	Lower-level shared utility; replaces SCIM's duplicate do (#1); placed alongside existing clientbase HTTP helpers per reviewer feedback (#13, #14, #18)

[caliskanugur]

A couple of questions and thoughts

[caliskanugur]

This appears to introduce a new HTTP client implementation similar to the SCIM client, rather than building on clientbase. I recall from the previous design meeting that the recommended approach for repeated HTTP client instantiation was to extend clientbase — could someone confirm if that's still the consensus?

I think leveraging and expanding on existing logic like clientbase would strengthen Shepherd's value as a shared tool. From the README:

Shepherd consists of a few core libraries used to make it easy to write homologous tests. See extensions and clients for more details.

It would be great to understand the reasoning if a different approach was chosen here.

[joesims22]

I think extending clientbase is still the consensus for how to handle this.

[dasarinaidu]

Thanks for the feedback. You are right that the standard Shepherd pattern is to extend clientbase.

The reason I introduced raw http.Client fields here is that the OIDC endpoints (/oidc/authorize, /oidc/token, /oidc/.well-known/*) are not registered in Shepherd's clientbase schema — they're custom Rancher OIDC paths outside the standard management/steve API surface. Clientbase is designed around schema-registered resources, while the OIDC flow requires direct HTTP control (e.g., no-redirect client for the auth code flow, custom headers for PKCE).

[caliskanugur]

I understand the limitations — it sounds like the same challenges you experienced with the SCIM client. The consensus from the design meeting if this HTTP client instantiation was repeated, adding a new method on APIOperations that bypasses schema-related checks while giving more freedom on headers. Something like:

func (a *APIOperations) RawHttpRequest(method, url string, body io.Reader, headers map[string]string) (*http.Response, error) {
    req, err := http.NewRequest(method, url, body)
    if err != nil {
        return nil, err
    }
    for k, v := range headers {
        req.Header.Set(k, v)
    }
    return a.Client.Do(req)
}

Would something like this work both for SCIM and OIDC?

I think the most important question for understanding Shepherd's design is this: if you were writing these tests as integration tests in rancher/rancher instead of validation tests in rancher/tests, how would you achieve it?

The current implementation would require copy-pasting the same rancher/tests functions (such as enabling, disabling or other operations related to SCIM or OIDC) into rancher/rancher. Shepherd is supposed to be the centralized framework that enables anyone to write tests against Rancher while reusing this shared tooling.

[caliskanugur]

Also related to my last point: #547 (comment)

[caliskanugur]

I haven't had a chance to see how these are used on the test side yet, but I'm curious about the session implementation for these calls. One of Shepherd's core design principles is session-based resource management — from the README:

...and aids in cleaning up resources after a test is completed.

Could you clarify whether, after these tests run, the clusters are returned to the same state as if the tests had never run?

[caliskanugur]

For reference, here's an example of how OpenLDAP uses session management to handle enabling and disabling cleanly: https://github.com/rancher/shepherd/blob/main/clients/rancher/auth/openldap/openldap.go#L57-L59

[dasarinaidu]

The OpenLDAP pattern registers Disable() as a cleanup in Enable() so the auth provider is automatically disabled when the session ends. For OIDC this doesn't apply directly because our APIClient doesn't own the feature flag lifecycle — EnableOIDCFeatureFlag and DisableOIDCFeatureFlag live in rancher-tests/actions/oidc/oidc_setup.go and cleanup runs in TearDownSuite. The APIClient only makes HTTP calls to OIDC endpoints, it doesn't enable/disable anything.

[igomez06]

I don't quite understand what you're getting at Das. If we're adding a new client, we should ensure our session cleanup works for it as well. Let's please try to keep the design in mind when adding this. So when we teardown regardless if it's in extensions or not session.Cleanup() should return the cluster in the previous state.

[igomez06]

we register deletes here FYI https://github.com/rancher/shepherd/blob/main/pkg/clientbase/ops.go#L233-L242

[igomez06]

Why doesn't the enable flag function live here? Why is it an action in rancher/tests? That doesn't make sense to me. The registration delete should happen with the client not outside shepherd

[dasarinaidu]

This is not like openLdap/ ActiveDirectory auth provider; The EnableOIDCFeatureFlag & DisableOIDCFeatureFlag are just a flag which we managed in rancher settings. The feature flag lifecycle can't live in shepherd because featuresactions.UpdateFeatureFlag is in rancher/tests/actions/features, not in shepherd. Shepherd has no dependency on rancher-tests. The session cleanup registration for DisableOIDCFeatureFlag does happen inside EnableOIDCFeatureFlag in rancher-tests.

[caliskanugur]

This is really solid context, thanks Das.

Shepherd already has UpdateFeatureFlag in extensions/features/update.go — is there a reason this can't be used for enabling/disabling the OIDC feature flag? If not, could you clarify what's missing or different about the OIDC case that makes it unusable here?

[igomez06]

https://github.com/rancher/tests/pull/602/changes#diff-eae14b793e1f34c5ab2d7ba572da2d9dea3f631b2e95adae34fad310f478a014R125
why is this an action?
Also could you use this https://github.com/rancher/shepherd/blob/main/extensions/features/update.go#L9-L24
to enable the flag das?
https://github.com/rancher/tests/pull/602/changes#diff-eae14b793e1f34c5ab2d7ba572da2d9dea3f631b2e95adae34fad310f478a014R46
this should definitely be an extension

[dasarinaidu]

Israel, Thanks for the review, Just want to give you little more about this feature and work flow

How the oidc-provider feature flag works differently from other flags:
Enabling the oidc-provider flag is not a simple toggle — it triggers an automatic Rancher pod restart as a side effect (unlike SCIM or other flags which take effect in-process without a restart). The OIDC provider only initializes after the pod comes back up. This means any code that runs immediately after the flag enable will hit a 503/502 if the pod is still restarting.

Why EnableOIDCFeatureFlag is an action:
It combines four steps that are specific to OIDC and too complex to inline in SetupSuite:

1. Check if already enabled (featuresactions.IsEnabled) — skip restart if already ON
2. Update the flag — using featuresactions.UpdateFeatureFlag from rancher/tests
3. Wait for Rancher to be fully ready using a direct k8s client (kubernetes.NewForConfig) — required because the Steve-based pod watch in featuresactions.UpdateFeatureFlag can miss rapid restarts and return early with "Rancher restart was not observed", leaving Rancher mid-restart
4. Register DisableOIDCFeatureFlag as a session cleanup via session.RegisterCleanupFunc

On using shepherd's extensions/features/update.go UpdateFeatureFlag:
We could use it for step 2, but it only does the Steve update — it does not wait for the restart. We still need steps 1, 3, and 4 which don't exist anywhere else. So the wrapper action is still needed regardless of which update function is used underneath. Happy to switch the update step to use the shepherd function if preferred.

On IsEnabled returning bool:
Agreed — I use it purely for the check in step 1, not for setting anything.

[Priyashetty17]

I'm a bit confused what we are doing here because the OIDCClient public API already exists in Rancher (management.cattle.io/v3), and this file’s comment says we are working with that CRD. However, I don’t see v3.OIDCClient being used. Should we be using the typed client and OIDCClientSpec directly instead of introducing a separate dynamic wrapper and duplicated spec?
Could you try updating the pkg/apis version in the go.mod file to latest version (in shepherd) and run "go mod tidy" to use the new public API OIDCClient in rancher/rancher?

this can be simplified with wrangler context after updating the api version in go.mod, something like this:

package oidcclient

import (
	"context"
	"fmt"

	v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
	"github.com/rancher/shepherd/clients/rancher"
	"github.com/rancher/shepherd/extensions/defaults"
	k8serrors "k8s.io/apimachinery/pkg/api/errors"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	kwait "k8s.io/apimachinery/pkg/util/wait"
)

const OIDCClientSecretNamespace = "cattle-oidc-client-secrets"

// CreateOIDCClient creates an OIDCClient using wrangler context and registers cleanup. AlreadyExists is ignored.
func CreateOIDCClient(client *rancher.Client, name string, spec v3.OIDCClientSpec) error {
	oidcClient := &v3.OIDCClient{
		ObjectMeta: metav1.ObjectMeta{
			Name: name,
		},
		Spec: spec,
	}

	_, err := client.WranglerContext.Mgmt.OIDCClient().Create(oidcClient)
	if err != nil {
		if !k8serrors.IsAlreadyExists(err) {
			return fmt.Errorf("creating OIDCClient %q: %w", name, err)
		}
		return nil
	}

	client.Session.RegisterCleanupFunc(func() error {
		return DeleteOIDCClient(client, name)
	})

	return nil
}

// WaitForOIDCClientReady polls until status.clientID and status.clientSecrets are populated
func WaitForOIDCClientReady(client *rancher.Client, name string) (clientID, secretKeyName string, err error) {
	err = kwait.PollUntilContextTimeout(context.Background(), defaults.FiveSecondTimeout, defaults.TwoMinuteTimeout, true, func(ctx context.Context) (bool, error) {
		oidcClient, getErr := client.WranglerContext.Mgmt.OIDCClient().Get(name, metav1.GetOptions{})
		if getErr != nil {
			return false, nil
		}

		if oidcClient.Status.ClientID == "" {
			return false, nil
		}

		if len(oidcClient.Status.ClientSecrets) == 0 {
			return false, nil
		}

		for k := range oidcClient.Status.ClientSecrets {
			secretKeyName = k
			break
		}

		clientID = oidcClient.Status.ClientID

		return true, nil
	},
	)

	if err != nil {
		return "", "", fmt.Errorf("timed out waiting for OIDCClient %q status.clientID: %w", name, err)
	}

	return clientID, secretKeyName, nil
}

// FetchOIDCClientSecret retrieves the OIDCClient secret from the secrets namespace using wrangler context
func FetchOIDCClientSecret(client *rancher.Client, clientID, secretKeyName string) (string, error) {
	secret, err := client.WranglerContext.Core.Secret().Get(OIDCClientSecretNamespace, clientID, metav1.GetOptions{})
	if err != nil {
		return "", fmt.Errorf("getting OIDCClient secret %s/%s: %w",
			OIDCClientSecretNamespace, clientID, err)
	}

	value, ok := secret.Data[secretKeyName]
	if !ok || len(value) == 0 {
		return "", fmt.Errorf("key %q not found or empty in secret %s/%s",
			secretKeyName, OIDCClientSecretNamespace, clientID)
	}

	return string(value), nil
}

// DeleteOIDCClient deletes the OIDCClient by name using wrangler context
func DeleteOIDCClient(client *rancher.Client, name string) error {
	err := client.WranglerContext.Mgmt.OIDCClient().Delete(name, &metav1.DeleteOptions{})
	if err != nil {
		if k8serrors.IsNotFound(err) {
			return nil
		}
		return fmt.Errorf("deleting OIDCClient %q: %w", name, err)
	}

	return nil
}

[dasarinaidu]

Agreed in principle on the typed v3.OIDCClient. Bumping pkg/apis to expose OIDCClientSpec.Scopes transitively requires bumping sigs.k8s.io/cluster-api from v1.10.6 → v1.12.x. cluster-api v1.12 reorganized its API path (api/v1beta1 → api/core/v1beta1), which breaks shepherd's existing cluster-api imports in pkg/codegen/main.go and the generated pkg/generated/controllers/cluster.x-k8s.io/v1beta1/ tree.

I like to handle that bump (and the corresponding shepherd-side fixes + wrangler regeneration) as a separate PR, then come back here with the typed rewrite.

[Priyashetty17]

sounds good, you may create a separate PR for that. Once it is merged, you can update this PR.

[Priyashetty17]

this can be removed later when you update OIDCClient CRUD to use wrangler context after the pkg/apis bump in go.mod