-
Notifications
You must be signed in to change notification settings - Fork 176
feat: add config.secretKeys to selectively include secrets #384
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.
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
Merged
pierluigilenoci
merged 3 commits into
oauth2-proxy:main
from
pierluigilenoci:fix/exclude-client-secret-federated-auth
Jan 15, 2026
Merged
feat: add config.secretKeys to selectively include secrets #384
pierluigilenoci
merged 3 commits into
oauth2-proxy:main
from
pierluigilenoci:fix/exclude-client-secret-federated-auth
Jan 15, 2026
+28
−10
Conversation
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
tuunit
requested changes
Jan 7, 2026
pierluigilenoci
added a commit
to pierluigilenoci/manifests
that referenced
this pull request
Jan 7, 2026
Address maintainer feedback from PR review: - Renamed config.secretKeys to config.requiredSecretKeys for clarity - Moved default values from template to values.yaml as suggested - Removed the '| default' logic from templates Changes: - values.yaml: Define requiredSecretKeys with explicit defaults: [client-id, client-secret, cookie-secret] - _helpers.tpl: Use requiredSecretKeys directly without fallback - deployment.yaml: Use requiredSecretKeys directly without fallback This makes the configuration more explicit and easier to understand, following Helm best practices of defining defaults in values.yaml rather than in templates. Addresses: oauth2-proxy#384 (review comments) Signed-off-by: Pierluigi Lenoci <[email protected]>
Added config.secretKeys option to provide granular control over which
secrets are included in the Kubernetes secret and exposed as environment
variables. This addresses scenarios where certain authentication methods
don't require all secrets (e.g., Azure Entra ID federated token authentication
doesn't need client-secret).
Implementation:
- Added config.secretKeys list in values.yaml (defaults to empty list)
- When empty, defaults to all three secrets: [client-id, client-secret, cookie-secret]
- Modified _helpers.tpl to conditionally include secrets based on allowlist
- Modified deployment.yaml to conditionally set environment variables
Example usage to exclude client-secret:
config:
requiredSecretKeys:
- client-id
- cookie-secret
This provides explicit control and future extensibility for additional
secrets while maintaining full backward compatibility.
Fixes oauth2-proxy#376
Signed-off-by: Pierluigi Lenoci <[email protected]>
Address maintainer feedback from PR review: - Renamed config.secretKeys to config.requiredSecretKeys for clarity - Moved default values from template to values.yaml as suggested - Removed the '| default' logic from templates Changes: - values.yaml: Define requiredSecretKeys with explicit defaults: [client-id, client-secret, cookie-secret] - _helpers.tpl: Use requiredSecretKeys directly without fallback - deployment.yaml: Use requiredSecretKeys directly without fallback This makes the configuration more explicit and easier to understand, following Helm best practices of defining defaults in values.yaml rather than in templates. Addresses: oauth2-proxy#384 (review comments) Signed-off-by: Pierluigi Lenoci <[email protected]>
Update changelog annotation to use the correct field name 'requiredSecretKeys' instead of 'secretKeys' to match the implemented changes. Signed-off-by: Pierluigi Lenoci <[email protected]>
08bbf03 to
4c02042
Compare
tuunit
approved these changes
Jan 15, 2026
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.
Summary
This PR implements the
config.requiredSecretKeysoption to provide granular control over which secrets are included in the Kubernetes secret and exposed as environment variables.Problem
When using authentication methods like Azure Entra ID federated token authentication, the
client-secretis not required. However, the chart currently always includes and requires all three secrets (client-id,client-secret,cookie-secret), causing issues when usingproxyVarsAsSecrets: truewithexistingSecret.Solution
Added a
config.requiredSecretKeyslist that allows users to explicitly specify which secrets to include:Implementation Details
Following maintainer feedback, the implementation now uses:
config.requiredSecretKeyslist with explicit defaults:oauth2-proxy.secretstemplate to conditionally include secrets based on the listDefault Behavior (Backward Compatible)
The defaults are now explicitly defined in
values.yaml:This ensures full backward compatibility with existing deployments while making the configuration more explicit and easier to understand.
Usage Examples
Scenario 1: Default (all secrets)
→ Includes all three secrets (default behavior)
Scenario 2: Exclude client-secret (federated auth)
→ Includes only
client-idandcookie-secretScenario 3: Custom subset
→ Includes only
cookie-secretBenefits
Changes Based on Maintainer Feedback
secretKeystorequiredSecretKeysfor clarity| default) to explicit defaults invalues.yamlTesting
Tested with
helm templateto verify:Checklist
Fixes #376