fix(chart): use hasKey for boolean enabled to avoid Go template default trap#639
Open
chaodu-agent wants to merge 1 commit intomainfrom
Open
fix(chart): use hasKey for boolean enabled to avoid Go template default trap#639chaodu-agent wants to merge 1 commit intomainfrom
chaodu-agent wants to merge 1 commit intomainfrom
Conversation
…e default trap
Go template's `default` treats `false` as a zero value, so
`{{ false | default true }}` returns `true`. This made it impossible
to disable cronjobs or reactions via `enabled: false`.
Fix both occurrences:
- reactions.enabled (L104)
- cron.jobs[].enabled (L142)
Also clarify the enabled field comment in values.yaml to reflect that
disabled jobs are still validated at startup.
Discovered during four-monk review of PR #638.
Co-authored-by: 覺渡法師 <[email protected]>
|
All PRs must reference a prior Discord discussion to ensure community alignment before implementation. Please edit the PR description to include a link like: This PR will be automatically closed in 3 days if the link is not added. |
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
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.
What problem does this solve?
PR #638 introduced
enabledfields for cronjobs and the existingreactions.enabledalready had the same pattern:enabled = {{ .enabled | default true }}In Go templates,
defaulttreatsfalseas a zero value, so{{ false | default true }}returnstrue. This means users cannot disable cronjobs or reactions by settingenabled: false— the value is silently overridden totrue.Discovered during four-monk review of PR #638 by 覺渡法師 (Gemini).
Changes
charts/openab/templates/configmap.yaml{{ .enabled | default true }}with{{ if hasKey . "enabled" }}{{ .enabled }}{{ else }}true{{ end }}for bothreactions.enabledandcron.jobs[].enabledcharts/openab/values.yamlenabledcomment:false = skip scheduling (job config is still validated at startup)Testing
Related
validate_cronjobs()insrc/cron.rsstill validates disabled jobs at startup. This is a separate runtime concern — suggest opening a follow-up issue if the team wantsenabled: falseto also skip validation.