Add Keycloak SAML auth provider automation - #874
Draft
dasarinaidu wants to merge 1 commit into
Draft
Conversation
dasarinaidu
force-pushed
the
keycloak-saml-automation
branch
2 times, most recently
from
September 3, 2026 20:25
3174293 to
2d258b5
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new session-revocation poll can return false positives on non-auth errors, and the committed replace directives to a personal Shepherd fork need a merge-safe resolution.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new Keycloak SAML auth-provider validation suite and supporting auth actions to bring Keycloak SAML coverage in line with existing OpenLDAP/Active Directory provider automation.
Changes:
- Introduces
validation/auth/provider/keycloaksamltest suite + README for running/configuring Keycloak SAML provider tests. - Adds Keycloak realm/client/user/group setup helpers in
actions/auth/keycloak.goand wires Keycloak SAML into shared auth-provider helpers. - Temporarily repins Shepherd (via
replace) to an alternate module path to pick up unmerged upstream changes.
File summaries
| File | Description |
|---|---|
| validation/auth/provider/keycloaksaml/README.md | Documents Keycloak SAML provider test prerequisites, config, and execution. |
| validation/auth/provider/keycloaksaml/keycloaksaml_test.go | Adds the Keycloak SAML provider suite with enable/disable, access-mode, principal, RBAC, and nested-group coverage. |
| actions/auth/keycloak.go | Implements Keycloak realm/client/mappers and fixture provisioning to support SAML tests. |
| actions/auth/authprovider.go | Adds Keycloak SAML provider constants and integrates SAML-specific login/principal-id/access-mode flows. |
| actions/auth/verify.go | Adds polling helper to detect when a provider-issued session has been invalidated. |
| go.mod | Updates Shepherd version and adds a temporary replace to an alternate module path. |
| go.sum | Records checksums for the alternate Shepherd module path. |
| actions/go.mod | Updates Shepherd version and adds a temporary replace to an alternate module path (actions module). |
| actions/go.sum | Records checksums for the alternate Shepherd module path (actions module). |
| interoperability/go.mod | Updates Shepherd version and adds a temporary replace to an alternate module path (interoperability module). |
| interoperability/go.sum | Records checksums for the alternate Shepherd module path (interoperability module). |
Review details
- Files reviewed: 8/11 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dasarinaidu
force-pushed
the
keycloak-saml-automation
branch
from
September 3, 2026 21:19
2d258b5 to
86e5e8e
Compare
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a large new auth-provider automation surface area and depends on a temporary fork/replace of shepherd, which warrants final human validation before approval.
Review details
- Files reviewed: 8/11 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+78
to
+79
| // SetupKeycloakSAML creates the Keycloak client, accounts and groups a SAML run needs and returns them as a fixture | ||
| func SetupKeycloakSAML(client *rancher.Client, keycloakClient *keycloak.Client) (*KeycloakSAMLFixture, error) { |
|
|
||
| ### Running the Tests | ||
| **Run Keycloak SAML Authentication Tests** | ||
| Your GO suite should be set to -run ^TestKeycloakSAMLAuthProviderSuite$ |
Adds a Keycloak SAML validation suite matching the coverage of the existing OpenLDAP and Active Directory provider suites, along with the actions that drive it. - validation/auth/provider/keycloaksaml: the suite, gated behind the validation build tag like the other provider suites - actions/auth/keycloak.go: Keycloak realm and client setup, test account provisioning, and provider enable/disable workflows - actions/auth/authprovider.go, verify.go: Keycloak cases alongside the existing providers The provider config key is keycloaksaml so that keycloakoidc can be added later without a rename. Depends on rancher/shepherd#705. The replace directive pointing at the shepherd fork is temporary and comes out once that PR merges.
dasarinaidu
force-pushed
the
keycloak-saml-automation
branch
from
September 3, 2026 21:25
86e5e8e to
e5e4b47
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a forked Shepherd dependency via replace directives (supply-chain/provenance risk) and the new suite does not follow the repository’s required test naming/organization pattern (Dynamic test + feature-name placement).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
actions/go.mod:252
- This replace directive points github.com/rancher/shepherd at a fork (github.com/dasarinaidu/shepherd). For supply-chain safety and reproducibility, avoid forks in module replace directives for shared libraries; prefer an upstream shepherd version that includes the required Keycloak SAML support.
replace github.com/rancher/shepherd => github.com/dasarinaidu/shepherd v0.0.0-20260903195701-8ef40de47b98
interoperability/go.mod:220
- interoperability/go.mod also replaces github.com/rancher/shepherd with a fork (github.com/dasarinaidu/shepherd). This creates the same supply-chain/provenance risk across modules; prefer an upstream shepherd version and remove the fork replace.
replace github.com/rancher/shepherd => github.com/dasarinaidu/shepherd v0.0.0-20260903195701-8ef40de47b98
go.mod:296
- The module replaces github.com/rancher/shepherd with github.com/dasarinaidu/shepherd, which introduces a non-upstream fork into the dependency chain and makes the build provenance unclear. Prefer consuming an upstream rancher/shepherd commit/tag (or upstreaming the needed changes) instead of pinning a fork via replace.
replace github.com/rancher/shepherd => github.com/dasarinaidu/shepherd v0.0.0-20260903195701-8ef40de47b98
- Files reviewed: 8/11 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+98
to
+102
| func (k *KeycloakSAMLAuthProviderSuite) TestKeycloakSAMLEnableProvider() { | ||
| subSession := k.session.NewSession() | ||
| defer subSession.Cleanup() | ||
|
|
||
| err := authactions.EnsureAuthProviderEnabled(k.client, authactions.KeycloakSAML) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Keycloak SAML validation suite matching the coverage of the existing OpenLDAP and Active Directory provider suites, along with the actions that drive it.
What is here
validation/auth/provider/keycloaksaml— the suite, gated behind thevalidationbuild tag like the other provider suitesactions/auth/keycloak.go— Keycloak realm and client setup, test account provisioning, and provider enable/disable workflowsactions/auth/authprovider.go,actions/auth/verify.go— Keycloak cases alongside the existing providersThe provider config key is
keycloaksamlso thatkeycloakoidccan be added later without a rename. The complete provider config lives in one file, following the OpenLDAP and Active Directory pattern.