Skip to content

allow set_key_use through the SpireOIDCDiscoveryProvider CR - #109

Open
kabicin wants to merge 2 commits into
openshift:mainfrom
kabicin:set-key-use
Open

allow set_key_use through the SpireOIDCDiscoveryProvider CR#109
kabicin wants to merge 2 commits into
openshift:mainfrom
kabicin:set-key-use

Conversation

@kabicin

@kabicin kabicin commented Apr 24, 2026

Copy link
Copy Markdown

Fixes #108

  • exposes .spec.setKeyUse in SpireOIDCDiscoveryProvider CR to pass set_key_use to the OIDC discovery provider config map.

Summary by CodeRabbit

Release Notes

  • New Features
    • Added setKeyUse configuration option to SpireOIDCDiscoveryProvider that allows enabling the "use": "sig" parameter on JWKS keys.

@coderabbitai

coderabbitai Bot commented Apr 24, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@kabicin has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 45 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 45 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6856d67e-5057-441a-8949-78479c5d8e77

📥 Commits

Reviewing files that changed from the base of the PR and between 55802ef and f259947.

📒 Files selected for processing (1)
  • config/crd/bases/operator.openshift.io_spireoidcdiscoveryproviders.yaml

Walkthrough

Adds a new optional setKeyUse boolean field to the SpireOIDCDiscoveryProviderSpec custom resource, allowing users to configure whether JWKS keys include the "use": "sig" parameter. The field value is propagated to the generated OIDC Discovery Provider ConfigMap during reconciliation.

Changes

Cohort / File(s) Summary
OIDC Discovery Provider setKeyUse Configuration
api/v1alpha1/spire_oidc_discovery_provider_types.go, pkg/controller/spire-oidc-discovery-provider/configmaps.go
Adds optional SetKeyUse boolean field to spec type and passes its value to the set_key_use configuration parameter in the generated ConfigMap.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 11 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning The PR adds a SetKeyUse field but the corresponding test file configmaps_test.go was not updated with tests for this new field. Add test cases to configmaps_test.go to verify SetKeyUse field propagation to the ConfigMap with key set_key_use.
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly describes the main change: exposing the set_key_use field through the SpireOIDCDiscoveryProvider custom resource.
Linked Issues check ✅ Passed The PR fully implements the requirement from issue #108 by adding the optional .spec.setKeyUse boolean field and propagating it to the ConfigMap.
Out of Scope Changes check ✅ Passed All changes are directly related to exposing and implementing the setKeyUse field as specified in issue #108; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Stable And Deterministic Test Names ✅ Passed The PR modifies only non-test files and contains no Ginkgo test definitions or dynamic test names.
Microshift Test Compatibility ✅ Passed This PR does not add any new Ginkgo e2e tests; it only modifies API type definitions and controller logic.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR does not add any new Ginkgo e2e tests. Changes only add an optional setKeyUse field to SpireOIDCDiscoveryProviderSpec type and its usage in ConfigMap population.
Topology-Aware Scheduling Compatibility ✅ Passed Pull request adds optional boolean field SetKeyUse to SpireOIDCDiscoveryProviderSpec without introducing Kubernetes scheduling constraints or deployment manifests.
Ote Binary Stdout Contract ✅ Passed PR modifies only type definitions and controller configuration without introducing process-level code (main, init, TestMain, BeforeSuite, AfterSuite) or stdout writes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR does not add any new Ginkgo e2e tests, only API type definitions and controller logic modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
openshift-ci Bot requested review from bharath-b-rh and swghosh April 24, 2026 03:50
@openshift-ci

openshift-ci Bot commented Apr 24, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kabicin
Once this PR has been reviewed and has the lgtm label, please assign trilokgeer for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 24, 2026
@openshift-ci

openshift-ci Bot commented Apr 24, 2026

Copy link
Copy Markdown

Hi @kabicin. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/v1alpha1/spire_oidc_discovery_provider_types.go`:
- Around line 65-69: The new SetKeyUse bool field on the
SpireOIDCDiscoveryProvider type is not present in the generated CRD OpenAPI
schema, so regenerate and commit the CRD: ensure the SetKeyUse field retains the
kubebuilder tags (e.g., +kubebuilder:validation:Optional and
+kubebuilder:default:=false) in spire_oidc_discovery_provider_types.go, run the
manifests generation (make manifests or controller-gen as used by the repo) to
update the CRD schema so spec.setKeyUse is included, then add and commit the
updated CRD manifest to the repo so the API server will accept the field at
apply time.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 97d72f23-adda-4138-abc5-9b9e3535d7d7

📥 Commits

Reviewing files that changed from the base of the PR and between b631460 and 55802ef.

📒 Files selected for processing (2)
  • api/v1alpha1/spire_oidc_discovery_provider_types.go
  • pkg/controller/spire-oidc-discovery-provider/configmaps.go

Comment thread api/v1alpha1/spire_oidc_discovery_provider_types.go
@akram

akram commented Apr 30, 2026

Copy link
Copy Markdown

@bharath-b-rh @swghosh do you think you can review this PR and tell if it can be merged?
I have tested it for a case where I needed to enabled JWT federated tokens with keycloak and it works properly.

Without it, I had to set this config on the openshift-zero-trust-workload-identity-manager Subscription

spec:    
  config:
      env:
      - name: CREATE_ONLY_MODE
        value: "true"

@bharath-b-rh

Copy link
Copy Markdown

/cc @rausingh-rh @anirudhAgniRedhat for the reviews.

@bharath-b-rh

Copy link
Copy Markdown

/ok-to-test

@openshift-ci openshift-ci Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 30, 2026
@openshift-ci

openshift-ci Bot commented Apr 30, 2026

Copy link
Copy Markdown

@kabicin: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-operator f259947 link true /test e2e-operator

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose CR field to pass set_key_use to the OIDC Discovery Provider config

3 participants