fix: add no-config legacy mode for external config management#404
fix: add no-config legacy mode for external config management#404pierluigilenoci wants to merge 7 commits intooauth2-proxy:mainfrom
Conversation
When alphaConfig.enabled=false and forceLegacyConfig=false, the chart now skips ConfigMap generation, volume mount, and --config flag entirely. This restores support for users managing oauth2-proxy configuration via external means (e.g., CSI SecretStore Driver) that was broken by oauth2-proxy#385. Closes oauth2-proxy#385 (discussion_r3001082837) Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
pierluigilenoci
force-pushed
the
fix/no-config-mode-external-secrets
branch
from
ff8095d to
37b40ad
Compare
There was a problem hiding this comment.
Pull request overview
Adds a “no-config” legacy mode to the Helm chart so users can fully manage oauth2-proxy configuration externally (e.g., CSI SecretStore), by preventing the chart from generating/mounting a legacy ConfigMap and omitting the --config flag.
Changes:
- Introduces
no-configas a new legacy-config mode and updates templates to skip legacy ConfigMap generation/mounting and the--configarg in that mode. - Adds a CI values file to exercise the no-config scenario.
- Updates
values.yamldocumentation to describe the no-config behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
helm/oauth2-proxy/values.yaml |
Documents the new no-config behavior and how it relates to alphaConfig.enabled / forceLegacyConfig. |
helm/oauth2-proxy/templates/deployment.yaml |
Conditionally omits --config and the legacy config volume/mount when in no-config mode; adjusts checksum behavior accordingly. |
helm/oauth2-proxy/templates/configmap.yaml |
Skips legacy ConfigMap rendering when in no-config mode (or using an existing ConfigMap). |
helm/oauth2-proxy/templates/_helpers.tpl |
Adds no-config to the legacy-config mode helper logic. |
helm/oauth2-proxy/ci/no-config-values.yaml |
Adds CI coverage for the no-config values scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Bump chart version to 10.4.3 with changelog entry - Fix helper doc comment to accurately describe all legacy-config modes - Clarify that no-config mode only applies when neither configFile nor existingConfig are set (those always take precedence) Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Re: Copilot comment on Fixed in 77b50d6: updated the doc comment to accurately reflect that |
The doc comment now accurately reflects that config.existingConfig and config.configFile are ignored when alphaConfig.enabled=true and forceLegacyConfig=false (first branch takes precedence). Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
This is a new feature (no-config mode), not a bug fix, so it warrants a minor version bump per semver. Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Without a config file, oauth2-proxy needs email-domain and upstream passed via extraArgs to start successfully. Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The flag now also has an effect when alphaConfig.enabled=false: setting forceLegacyConfig=false disables config generation entirely. Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
no-configlegacy-config mode: whenalphaConfig.enabled=falseandforceLegacyConfig=false, skip ConfigMap generation, volume mount, and--configflagFixes the issue reported in #385 (comment)
Test plan
helm templatewithno-config-values.yamlproduces no ConfigMap, no--configflag, no config volume mounthelm templatewithdefault-values.yamlstill generates ConfigMap and--configflag (no regression)helm lintpasses for all CI value files (24/24 pass)config.forceLegacyConfig: false