feat: add support for more values to be evaluated / templated as yaml

[treydock]

Description

Allow additional values to be templated when passed to the Helm chart.

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

[pierluigilenoci]

Review Notes

Thanks for the PR! A few observations and potential issues:

1. Inconsistent tpl context (. vs $)

In deployment.yaml the tpl call uses . (current context):

- --cookie-name={{ tpl .Values.config.cookieName . }}

While in _helpers.tpl and networkpolicy.yaml it uses $ (root context):

{{ tpl (toYaml .Values.customLabels) $ }}

Both work, but the inconsistency could cause confusion. Consider standardizing on $ everywhere for clarity.

2. envFrom documented in README but not implemented in templates

The diff adds envFrom to the README parameters table, but there's no corresponding change in any template file to actually use it. This looks like either a leftover from another change or premature documentation — could you clarify?

3. Semantic versioning: kind: fixed vs enhancement

The changelog entry uses kind: fixed, but this is more of a feature enhancement (supporting templated values in more places) rather than a bug fix. According to SemVer, a new feature should be a minor bump (10.2.0) rather than a patch (10.1.6). Worth reconsidering the version and changelog kind.

4. Security consideration with tpl

The tpl function executes arbitrary Go templates, including Sprig functions. If values are generated programmatically from untrusted input, this could be a vector for template injection. This is a common and accepted pattern in Helm charts, but it may be worth adding a note in the README about the security implications of templated values.

5. Missing CI test coverage for existingSecret and networkPolicy

The CI values in tpl-values.yaml cover cookieName and customLabels, but don't test the other newly templated values (config.existingSecret, networkPolicy.ingress, networkPolicy.egress). Adding test cases for these would improve confidence that the tpl wrapping works correctly in all modified paths.

[Copilot]

Pull request overview

This PR expands Helm templating support in the oauth2-proxy chart so additional user-supplied values can be rendered via tpl before being emitted into Kubernetes manifests.

Changes:

• Enable templating for networkPolicy.ingress / networkPolicy.egress, config.cookieName, config.existingSecret, and customLabels.
• Update CI templating values and documentation to reflect supported/configurable values.
• Bump chart version and append an Artifact Hub changelog entry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
helm/oauth2-proxy/templates/networkpolicy.yaml	Renders ingress/egress rules through tpl to allow templated network policy blocks.
helm/oauth2-proxy/templates/deployment.yaml	Allows config.cookieName to be templated when rendered into container args.
helm/oauth2-proxy/templates/_helpers.tpl	Allows templating for customLabels and config.existingSecret.
helm/oauth2-proxy/ci/tpl-values.yaml	Attempts to add CI coverage for newly templated values (needs adjustment).
helm/oauth2-proxy/README.md	Documents envFrom in the values table.
helm/oauth2-proxy/Chart.yaml	Version bump + changelog entry for the new templating capability.

---

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

[treydock]

Review Notes

Thanks for the PR! A few observations and potential issues:

1. Inconsistent tpl context (. vs $)

In deployment.yaml the tpl call uses . (current context):

- --cookie-name={{ tpl .Values.config.cookieName . }}

While in _helpers.tpl and networkpolicy.yaml it uses $ (root context):

{{ tpl (toYaml .Values.customLabels) $ }}

Both work, but the inconsistency could cause confusion. Consider standardizing on $ everywhere for clarity.

Updated my changes to standardize on $ but didn't want to change things I am not touching in the PR.

2. envFrom documented in README but not implemented in templates

The diff adds envFrom to the README parameters table, but there's no corresponding change in any template file to actually use it. This looks like either a leftover from another change or premature documentation — could you clarify?

I was updating the README for a missing item. The templating is already present:

manifests/helm/oauth2-proxy/values.yaml

Line 110 in 93b9b85

envFrom: []

3. Semantic versioning: kind: fixed vs enhancement

The changelog entry uses kind: fixed, but this is more of a feature enhancement (supporting templated values in more places) rather than a bug fix. According to SemVer, a new feature should be a minor bump (10.2.0) rather than a patch (10.1.6). Worth reconsidering the version and changelog kind.

Updated to be a minor version bump.

4. Security consideration with tpl

The tpl function executes arbitrary Go templates, including Sprig functions. If values are generated programmatically from untrusted input, this could be a vector for template injection. This is a common and accepted pattern in Helm charts, but it may be worth adding a note in the README about the security implications of templated values.

5. Missing CI test coverage for existingSecret and networkPolicy

The CI values in tpl-values.yaml cover cookieName and customLabels, but don't test the other newly templated values (config.existingSecret, networkPolicy.ingress, networkPolicy.egress). Adding test cases for these would improve confidence that the tpl wrapping works correctly in all modified paths.

Doing existingSecret would be a challenge due to needing to also generate a secret outside the Helm chart. Adding a testing network policy might also break tests due to Kubernetes now restricting access to the deployment. I can add if necessary but wanted to be cautious with potentially complex test additions.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 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]

The new tpl support for networkPolicy.ingress/egress isn’t exercised by any existing chart CI values files (no networkPolicy usage under helm/oauth2-proxy/ci/). Add a CI values case that sets networkPolicy.create: true and includes at least one templated field (e.g., a label value using .Release.Name) to prevent regressions.

[Copilot]

config.existingSecret is now rendered through tpl, but there’s no CI values coverage for this behavior (no existingSecret appears under helm/oauth2-proxy/ci/). Add a CI values test that sets config.existingSecret to a templated value and asserts the rendered secret references use the evaluated name.

[Copilot]

The Artifact Hub changelog entry is very broad (“Additional values can be templated”). Since this is user-facing behavior, please list the specific values/fields that gained tpl support (e.g., config.cookieName, config.existingSecret, customLabels, networkPolicy.ingress/egress) so users know what changed.

Suggested change

	description: Additional values can be templated
	description: Add tpl support for config.cookieName, config.existingSecret, customLabels, and networkPolicy.ingress/egress

[pierluigilenoci]

Nice work on this PR! A few thoughts on the remaining open items:

1. Changelog description

Agree with the suggestion to make it more specific. Users scanning the Artifact Hub changelog need to know exactly which fields gained tpl support without digging into the diff. Something like:

Add tpl support for config.cookieName, config.existingSecret, customLabels, and networkPolicy.ingress/egress

2. CI test for existingSecret

I understand the concern about complex test setups, but existingSecret should be straightforward to test — it only affects the rendered secret name, not whether the secret actually exists. A line like this in tpl-values.yaml should work without needing an external Secret:

config:
  existingSecret: '{{ .Release.Name }}-oauth2-proxy'

helm template (which is what CI runs) will just check that the template renders correctly, it won't try to resolve the Secret in a cluster. The networkPolicy case is more nuanced, so skipping that one seems reasonable.

3. Security note for tpl

This is a common and well-accepted pattern across the Helm ecosystem — not something that should block this PR. For reference, the Bitnami common library chart documents and encourages tpl rendering extensively: https://github.com/bitnami/charts/blob/main/bitnami/common/templates/_tplvalues.tpl

Virtually every major Helm chart (ingress-nginx, cert-manager, bitnami/*) uses tpl the same way. A README note could be a nice follow-up but shouldn't be a blocker here.

[treydock]

I've made the requested changes.

[treydock]

@pierluigilenoci I think the existing secret test is the problem:

Error: secret "oauth2-proxy-466sc1g5yi-oauth2-proxy" not found   

The ct install testing will install real deployment and the secret is expected to exist. I've removed that change so tests can pass.

[treydock]

@pierluigilenoci Let me know if any more changes are needed or if this is good for review.