fix: configmap generation with `no-config` mode for external secret management by tuunit · Pull Request #402 · oauth2-proxy/manifests

tuunit · 2026-03-29T07:03:21Z

Description

Add back support for running without generated/mounted ConfigMap when alphaConfig.enabled=false and forceLegacyConfig=false. This enables users to manage oauth2-proxy configuration entirely via external secrets or other means as was previously possible as mentioned by #385 (comment)

Add no-config legacy-config mode
Skip ConfigMap generation and volume mounts in no-config mode
Add mode label to generated ConfigMaps for debugging
Update values.yaml documentation

Checklist:

I have bumped the version in the Chart.yaml according to Semantic Versioning.
I have updated the documentation/CHANGELOG at the bottom of the Chart.yaml
I have signed off all my commits.
(Optional) I have updated the Chart.lock for dependency updates
(Optional) I have implemented helm tests for new feature flags

Copilot

Pull request overview

Reintroduces support for running the chart without a generated/mounted legacy ConfigMap when alphaConfig.enabled=false and config.forceLegacyConfig=false, enabling externally-managed configuration.

Changes:

Add no-config as a new legacy-config mode and use it to skip legacy ConfigMap generation.
Update Deployment template to conditionally skip legacy config volume/volumeMount in no-config mode.
Add a mode label to generated legacy ConfigMaps and update values documentation; bump chart version + changelog entry.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`helm/oauth2-proxy/values.yaml`	Documents the new `no-config` behavior for legacy config handling.
`helm/oauth2-proxy/templates/deployment.yaml`	Adds mode-aware conditional mounting of the legacy config volume/volumeMount.
`helm/oauth2-proxy/templates/configmap.yaml`	Skips rendering the legacy ConfigMap in `existing-configmap` and `no-config` modes; adds a `mode` label.
`helm/oauth2-proxy/templates/_helpers.tpl`	Introduces `no-config` as a possible value for `legacy-config.mode`.
`helm/oauth2-proxy/Chart.yaml`	Bumps chart version and updates Artifact Hub changelog annotation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

helm/oauth2-proxy/templates/deployment.yaml

helm/oauth2-proxy/templates/_helpers.tpl

helm/oauth2-proxy/values.yaml

helm/oauth2-proxy/templates/deployment.yaml

…anagement Signed-off-by: Jan Larwig <jan@larwig.com>

tuunit · 2026-03-29T07:32:15Z

@pierluigilenoci this whole config handling is getting quite messy... We need to find a better solution

Signed-off-by: Jan Larwig <jan@larwig.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-29T08:11:27Z

helm/oauth2-proxy/values.yaml

+  # when the chart generates the alpha config itself.
  configFile: ""
-  # Use an existing config map (see secret-alpha.yaml for required fields)
+  # Use an existing config map (see secret-alpha.yaml for required fields).


The comment for alphaConfig.existingConfig points to secret-alpha.yaml for required fields, but that template defines a Secret (base64-encoded data:), not a ConfigMap. This is misleading for users providing an external ConfigMap; consider documenting the required key directly (e.g., data.oauth2_proxy.yml) or referencing a ConfigMap-specific example/template instead.

Suggested change

# Use an existing config map (see secret-alpha.yaml for required fields).

# Use an existing ConfigMap containing the alpha config file (for example,

# with a key like `oauth2_proxy.yml` under `.data`).

Copilot · 2026-03-29T08:11:28Z

helm/oauth2-proxy/README.md

 Keep the following in mind:

 - The chart always mounts `/etc/oauth2_proxy/oauth2_proxy.cfg`. (Legacy toml config)
+  Unless both `alphaConfig.enabled` and `forceLegacyConfig` are set to `false`


This line references forceLegacyConfig without the config. prefix. Since the value is config.forceLegacyConfig, the current wording is ambiguous and could confuse readers; please change it to config.forceLegacyConfig for consistency with the rest of the document.

Suggested change

Unless both `alphaConfig.enabled` and `forceLegacyConfig` are set to `false`

Unless both `alphaConfig.enabled` and `config.forceLegacyConfig` are set to `false`

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-29T14:44:19Z

helm/oauth2-proxy/templates/configmap.yaml

+{{- $legacySource := include "oauth2-proxy.legacy-config.source" . }}
+{{- if not (has $legacySource (list "existing-configmap" "no-config")) }}
 apiVersion: v1
 kind: ConfigMap


PR description mentions adding a mode label to generated ConfigMaps for debugging, but this template currently only changes the generation condition and does not add any label reflecting $legacySource. Consider adding a label (e.g., legacy-config/source or mode) under metadata.labels when the ConfigMap is rendered so users can identify which legacy config source was used.

Copilot · 2026-03-29T14:44:19Z

helm/oauth2-proxy/templates/_helpers.tpl

+{{- else -}}
 generated-alpha-compatible
+{{- end -}}
+{{- else if not .Values.config.forceLegacyConfig -}}
+no-config
+{{- else if .Values.config.existingConfig -}}


The new no-config legacy-config source (when alphaConfig.enabled=false and config.forceLegacyConfig=false) changes rendered manifests by removing the legacy ConfigMap, --config arg, and volume mounts, but there is no chart-testing install case in helm/oauth2-proxy/ci/ covering this mode. Add a CI values file exercising no-config to prevent regressions (e.g., ensure configmap.yaml is skipped and the Deployment no longer references configmain).

pierluigilenoci · 2026-03-29T15:21:41Z

Hi! I've opened #404 as a minimal, surgical fix for the same issue reported by @MattiasGees.

The key difference is that #404 only adds a no-config branch to the existing legacy-config.mode helper (2 lines in _helpers.tpl) and gates the ConfigMap generation, --config flag, volume mount, and volume definition on it — with zero renames, no new helpers, and no behavioral changes for existing configurations.

I think the structural improvements in this PR (alpha-config helpers, deprecation guards, better naming) are valuable, but they'd be better suited as a separate follow-up refactor PR after the core fix lands, to keep the blast radius small and review straightforward.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-29T15:26:51Z

helm/oauth2-proxy/templates/_helpers.tpl

 {{/*
 If `config.forceLegacyConfig=false`, the chart ignores both the `config.configFile` and `config.existingConfig` overrides and only generates a minimal necessary legacy config.
 If `config.existingConfig` is set and `config.forceLegacyConfig=true`, the external ConfigMap is mounted into the mounted file.
 If `config.configFile` is set and `config.forceLegacyConfig=true`, the chart renders that inline content into the mounted file.
+If `config.forceLegacyConfig=false` and `alphaConfig.enabled=false`, the chart renders no config map and does not mount a file.
 */}}


The helper doc comment says config.forceLegacyConfig=false “only generates a minimal necessary legacy config”, but the new no-config path means no legacy config is rendered/mounted when alphaConfig.enabled=false and config.forceLegacyConfig=false. Update the comment to reflect the two different behaviors (alphaConfig-enabled vs alphaConfig-disabled).

Copilot · 2026-03-29T15:26:51Z

helm/oauth2-proxy/templates/deployment.yaml

+        {{- if $legacyConfigEnabled }}
          - --config=/etc/oauth2_proxy/oauth2_proxy.cfg
+        {{- end }}


New no-config mode changes runtime behavior (skips legacy ConfigMap generation/mount and omits the --config arg), but there is no chart-testing install scenario covering alphaConfig.enabled=false + config.forceLegacyConfig=false. Add a helm/oauth2-proxy/ci/*-values.yaml case to ensure templates render and install correctly for this mode.

Copilot · 2026-03-29T15:26:51Z

helm/oauth2-proxy/values.yaml

  # 2. When alphaConfig.enabled=true and forceLegacyConfig=false,
  #    both configFile and existingConfig are ignored and the chart
  #    generates a minimal legacy config from emailDomains only.
  # 3. If configFile is empty/not set, the config is auto-generated
  #    from emailDomains and, when alphaConfig is disabled, upstreams.
+  # 4. When alphaConfig.enabled=false and forceLegacyConfig=false
+  #    no ConfigMap is generated and mounted


The newly documented alphaConfig.enabled=false + config.forceLegacyConfig=false “no ConfigMap” behavior means forceLegacyConfig now also has an effect when alphaConfig is disabled. Later in this section the comment says “This flag only has an effect when alphaConfig.enabled is true…”, which is now inconsistent—please update that wording to include the no-config mode case.

pierluigilenoci · 2026-03-29T16:10:56Z

Following up on my previous comment — I've now split the work across two PRs:

fix: add no-config legacy mode for external config management #404 — Minimal bug fix: adds no-config legacy mode so users managing config externally (CSI SecretStore Driver, etc.) are no longer broken by feat: add forceLegacyConfig flag for alphaConfig compatibility #385. Surgical change, no renames, no breaking changes.
feat: add alpha-config helpers and deprecation guards #405 — Structural improvements cherry-picked from this PR: alpha-config.source and alpha-config.name helpers, deprecation guards for invalid existingConfig/existingSecret combinations, simplified secret-alpha.yaml and deployment.yaml volume logic, CI test for alphaConfig.existingSecret.

Intentionally left out from #405:

Renaming legacy-config.mode → legacy-config.source (breaking for custom templates)
Always-on debug annotations (legacy/source, alpha/source)
legacy-config.enabled helper (thin wrapper, not needed)

This way the bug fix can land quickly (#404) and the refactor improvements can be reviewed independently (#405). Thanks for the original work on this PR — it was a great starting point!

tuunit · 2026-03-29T19:44:19Z

@pierluigilenoci I have to disagree with all three points that have been left out in the split. I don't understand why the split into two more PRs is necessary. We could just work in this PR to figure out a final solution.

I think renaming to source will not breaking anything. The AI isn't aware of when the release was done and in my opinion no one will have built custom templating based on the new helpers in the last 3 days...

Furthermore, why wouldn't we want the annotations for the type of config used? It a quick insight and doesn't hurt anyone.

One other possible route we could take, is revert the original PR and take more time to fully rework this mess to have a clean state even if it means introducing a breaking change

tuunit mentioned this pull request Mar 29, 2026

feat: add forceLegacyConfig flag for alphaConfig compatibility #385

Merged

6 tasks

tuunit force-pushed the fix/configmap-generation branch from 5a2b3b6 to ad9a277 Compare March 29, 2026 07:07

tuunit requested review from Copilot and pierluigilenoci March 29, 2026 07:10

Copilot started reviewing on behalf of tuunit March 29, 2026 07:10 View session

Copilot AI reviewed Mar 29, 2026

View reviewed changes

tuunit force-pushed the fix/configmap-generation branch 2 times, most recently from 71b2bf9 to 4d0756a Compare March 29, 2026 07:18

fix: configmap generation with no-config mode for external secret m…

39eabb4

…anagement Signed-off-by: Jan Larwig <jan@larwig.com>

tuunit force-pushed the fix/configmap-generation branch from 4d0756a to 39eabb4 Compare March 29, 2026 07:31

tuunit force-pushed the fix/configmap-generation branch from 4c8e399 to afddb2a Compare March 29, 2026 08:02

tuunit requested a review from Copilot March 29, 2026 08:02

Copilot started reviewing on behalf of tuunit March 29, 2026 08:07 View session

rework

8ece5f7

Signed-off-by: Jan Larwig <jan@larwig.com>

tuunit force-pushed the fix/configmap-generation branch from afddb2a to 8ece5f7 Compare March 29, 2026 08:09

Copilot AI reviewed Mar 29, 2026

View reviewed changes

Merge branch 'main' into fix/configmap-generation

294a381

pierluigilenoci requested a review from Copilot March 29, 2026 14:40

Copilot started reviewing on behalf of pierluigilenoci March 29, 2026 14:40 View session

Copilot AI reviewed Mar 29, 2026

View reviewed changes

pierluigilenoci requested a review from Copilot March 29, 2026 15:21

Copilot started reviewing on behalf of pierluigilenoci March 29, 2026 15:22 View session

Copilot AI reviewed Mar 29, 2026

View reviewed changes

pierluigilenoci mentioned this pull request Mar 29, 2026

feat: add alpha-config helpers and deprecation guards #405

Open

4 tasks

	# Use an existing config map (see secret-alpha.yaml for required fields).
	# Use an existing ConfigMap containing the alpha config file (for example,
	# with a key like `oauth2_proxy.yml` under `.data`).

	Unless both `alphaConfig.enabled` and `forceLegacyConfig` are set to `false`
	Unless both `alphaConfig.enabled` and `config.forceLegacyConfig` are set to `false`

Conversation

tuunit commented Mar 29, 2026

Description

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tuunit commented Mar 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

pierluigilenoci commented Mar 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

pierluigilenoci commented Mar 29, 2026

Uh oh!

tuunit commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants