Skip to content

Added possibility to specify custom secret name to be passed#213

Merged
mskwierczynski982 merged 3 commits into
masterfrom
netChartEnvFromSecret
Jun 9, 2026
Merged

Added possibility to specify custom secret name to be passed#213
mskwierczynski982 merged 3 commits into
masterfrom
netChartEnvFromSecret

Conversation

@mskwierczynski982

Copy link
Copy Markdown
Contributor

Description

Right now there is no way to specify secret generated outside of the chart to be passed as environment variables making custom non-microservices dotnet deployments hard to pass secrets.
It adds such possibility without breaking current flow - fully backward compatible.

Chart

Select the chart that you are modifying:

  • core
  • dotnet-core
  • cron-job
  • job
  • app-reverse-proxy
  • pact-broker
  • ado-build-agents
  • ado-agent-cleaner
  • event-worker

Checklist

  • Description provided
  • Linked issue
  • Chart version bumped
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py

@mskwierczynski982 mskwierczynski982 requested a review from a team as a code owner June 4, 2026 11:19
@MgrothEcovadis

Copy link
Copy Markdown
Collaborator

▎ The change is simple, correct, and fully backward compatible. A few notes:

▎ Redundant inner condition — the inner {{- if .Values.global.extraEnvFrom }} check is redundant because the outer or condition already guards the envFrom block. An empty list [] is falsy in Go templates, so both
▎ conditions behave identically. Consider removing the inner guard:
▎ {{- if .Values.global.extraEnvFrom }}
▎ {{- toYaml .Values.global.extraEnvFrom | nindent 12 }}
▎ {{- end }}
▎ can stay as-is if you prefer the explicit guard for readability, but it's not needed for correctness.

▎ Missing Helm unit test — the PR checklist mentions template tests. There's no test covering the extraEnvFrom path (non-empty list should render envFrom entries correctly). Given the chart likely has a tests/ directory,
▎ this is straightforward to add.

▎ Checklist — all items are unchecked including the description box, even though a description is present. Worth ticking the boxes before merge.

▎ Overall: approve pending the test addition.

@mskwierczynski982 mskwierczynski982 merged commit 6ba9ad5 into master Jun 9, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants