fix: add missing Woodpecker secrets to pillar + validation gate #141
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo_admin/pal-e-platform!141
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "140-feat-secrets-pillar-validation-gate"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
woodpecker_db_passwordandwoodpecker_encryption_keyto Salt pillar (GPG-encrypted from running k8s cluster)make tofu-validate-secretsMakefile target that catches pillar/variable drift before plan or applytofu-planandtofu-applythrough the new validation gateChanges
salt/pillar/secrets/platform.sls— added GPG-encryptedwoodpecker_db_passwordandwoodpecker_encryption_keyentries, sourced from running k8s secretsMakefile— newtofu-validate-secretstarget that comparesTF_SECRET_VARSagainst renderedsecrets.auto.tfvars;tofu-planandtofu-applynow chain through it instead oftofu-secretsdirectlyContext
Discovered during
tofu applyfor PR #130 (Keycloak theme):make tofu-planfailed because two required Terraform variables existed invariables.tfand.woodpecker.yamlbut were never encrypted into the Salt pillar. CI worked because Woodpecker has its own secret store. Local apply was broken.Salt pillar now renders all 17 platform secrets. The validation gate ensures this never drifts again.
tofu plan output
Single change: Woodpecker Helm release picks up the two new secret values. Already applied successfully.
Test Plan
make tofu-secretsrenders 17 vars (was 15)make tofu-validate-secretspasses with all vars presentmake tofu-validate-secretsfails with clear error when a var is missingtofu applycompleted successfully — 0 added, 1 changed, 0 destroyedReview Checklist
tofu fmt— no formatting changes neededtofu validate— passesRelated
plan-pal-e-platform— Platform HardeningGenerated with Claude Code
PR #141 Review
DOMAIN REVIEW
Tech stack: Makefile, Salt pillar (GPG-encrypted secrets), Woodpecker CI YAML, OpenTofu/Terraform.
Salt pillar (GPG secrets)
woodpecker_db_passwordandwoodpecker_encryption_keyare properly GPG-encrypted PGP message blocks. No plaintext secrets.secrets:platformpillar key is consistent with existing entries (e.g.,harbor_admin_passwordimmediately after).TF_SECRET_VARSlist in the Makefile and thevariabledeclarations interraform/variables.tf(lines 139, 151).Makefile validation gate
tofu-validate-secretsiterates over$(TF_SECRET_VARS)and greps for^$$varin the renderedsecrets.auto.tfvars. The anchor^and trailing space ensure exact key match -- good.tofu-plan -> tofu-validate-secrets -> tofu-secretsandtofu-apply -> tofu-validate-secrets -> tofu-secretscorrectly gates both commands.wc -wcount in the success message uses a herestring (<<<), which requires bash. This is fine for local dev Makefiles but worth noting -- ifSHELLis ever changed to/bin/sh, this would break. Current defaultSHELL = /bin/shin Make could be an issue on systems where/bin/shis not bash. Consider settingSHELL := /bin/bashat the top of the Makefile, or usingecho '$(TF_SECRET_VARS)' | wc -winstead..PHONYupdated to includetofu-validate-secrets. Good.Woodpecker CI YAML -- scope concern
The diff includes significant
.woodpecker.yamlchanges that are NOT described in the PR body's Changes section:GODEBUG: netdns=cgoadded to validate, plan, and apply stepstofu init(sleep 3 + retry) andapk add(triple retry with sleep 10)-lock=falseadded to bothtofu applyinvocations in the apply stepThese are CI resiliency improvements that appear related to issues #121/#133 (clone auth, CI pipeline broken). They are valuable changes, but they are undocumented in this PR body and unrelated to issue #140's scope (secrets pillar validation gate). This is scope creep.
BLOCKERS
1. Scope creep in
.woodpecker.yaml(undocumented changes)The PR title and body describe two changes: (a) add missing Woodpecker secrets to pillar, (b) add
tofu-validate-secretsMakefile target. The.woodpecker.yamlchanges introduce a custom clone step, DNS debugging flags, retry logic, and lock-free apply -- none of which are mentioned in the PR body's Changes section or related to issue #140.Per SOP, these should either:
This is a process blocker, not a code quality blocker. The changes themselves look correct. But merging undocumented CI behavior changes silently violates traceability.
Resolution options:
.woodpecker.yamlchanges and reference the issues they address (#121, #133, etc.), OR.woodpecker.yamlchanges into a separate PR linked to the appropriate issue(s)NITS
Makefile line 91 herestring portability:
wc -w <<< '$(TF_SECRET_VARS)'assumes bash. Considerecho '$(TF_SECRET_VARS)' | wc -wor addSHELL := /bin/bashto the Makefile header. Low risk since this is a local-dev-only target, but defensive.apk addtriple retry (.woodpecker.yamlplan step): The patterncmd || (sleep 10 && cmd) || (sleep 10 && cmd)works but is noisy. A small loop would be cleaner, though this is a CI file so pragmatism wins.-lock=falseon apply deserves a comment in the YAML explaining why it is safe (e.g., "Single CI runner, no concurrent applies"). Without context, a future reader might question whether this risks state corruption.SOP COMPLIANCE
140-feat-secrets-pillar-validation-gate-> issue #140)plan-pal-e-platform(Platform Hardening)tofu fmtandtofu validateconfirmed passingtofu planoutput included (0 add, 1 change, 0 destroy).woodpecker.yamlchanges are undocumented scope creep (see BLOCKERS)PROCESS OBSERVATIONS
DORA impact: This PR directly improves Change Failure Rate (CFR) -- the validation gate prevents the exact class of failure that broke
tofu applyduring PR #130. The pillar-to-tfvars drift detection is a solid guardrail.Deployment frequency: The CI resiliency changes (clone fix, retries, GODEBUG) will improve pipeline reliability, which unblocks deployment frequency. However, bundling them without documentation creates traceability debt.
Documentation gap: The
.woodpecker.yamlchanges solve real problems (DNS resolution failures, clone auth, state lock contention) but lack issue-level traceability. If any of these cause a regression, there is no issue trail to trace back to.VERDICT: NOT APPROVED
The pillar and Makefile changes are clean and well-tested. The blocker is purely process: the
.woodpecker.yamlcontains significant undocumented CI behavior changes that should be either documented in the PR body with issue references, or split into their own tracked PR. Update the PR body to acknowledge these changes and this will pass.bf9e1b392017008b738dPR #141 Review
DOMAIN REVIEW
Tech stack: Makefile (shell), Salt pillar (GPG-encrypted YAML), OpenTofu/Terraform (variable definitions, Helm values).
Scope verification: The previous review flagged
.woodpecker.yamlscope creep. This rebase correctly removes it. The diff now contains exactly 2 files:Makefileandsalt/pillar/secrets/platform.sls. Scope creep is resolved.Pillar entries: Two new GPG-encrypted blocks added for
woodpecker_db_passwordandwoodpecker_encryption_key. Both are properly wrapped in-----BEGIN PGP MESSAGE-----/-----END PGP MESSAGE-----armor. No plaintext secrets present. The entries follow the same YAML indentation pattern (4-space key, 6-space PGP body) as existing entries.TF_SECRET_VARS alignment: Cross-referenced the 17 entries in
TF_SECRET_VARSagainst the 17variableblocks without defaults interraform/variables.tf. All match. The pillar file (on this branch) should contain all 17 keys after this PR merges.Validation gate logic: The
tofu-validate-secretstarget iteratesTF_SECRET_VARS, greps for^$$varinsecrets.auto.tfvars, and exits 1 on any miss. The dependency chain is correct:tofu-planandtofu-applyboth depend ontofu-validate-secrets, which depends ontofu-secrets. This means every plan/apply now runs the render + validate pipeline automatically.Shell correctness: The
grep -q "^$$var "pattern uses a trailing space to avoid prefix-matching (e.g.,woodpecker_db_passwordwould not false-matchwoodpecker_db_password_extra). The2>/dev/nullsuppresses errors when the file doesn't exist. Thewc -w <<< '$(TF_SECRET_VARS)'heredoc works in bash but requires the shell to be bash (Make defaults to/bin/sh). This is not a blocker because the system runs Arch Linux where/bin/shis typically bash-compatible, but worth noting for portability.Secrets handling: All secret material is GPG-encrypted. The
secrets.auto.tfvarsfile is rendered at runtime bytofu-secretsand should be in.gitignore.BLOCKERS
None. This PR adds infrastructure validation tooling (Makefile target) and encrypted secret entries. No application code, no user input paths, no auth logic. The blocker criteria do not apply:
NITS
k3s.tfvars.exampledrift: The example file haswoodpecker_db_passwordbut is missingwoodpecker_encryption_keyandwoodpecker_agent_secret. Not in scope for this PR, but the example file is now 6 entries behind the full secret set (17 vars vs 11 example entries). Consider a follow-up to sync it.Shell portability of
wc -w <<< '...': The<<<heredoc syntax is a bash-ism. If Make ever uses/bin/sh(dash, etc.), this line would fail. A portable alternative:echo '$(TF_SECRET_VARS)' | wc -w. This is cosmetic -- the validation logic itself uses POSIX-compatible constructs..PHONYline length: The.PHONYdeclaration is getting long (8 targets). Consider splitting into groups (salt targets, tofu targets) for readability. Minor.SOP COMPLIANCE
140-feat-secrets-pillar-validation-gatereferences issue #140plan-pal-e-platform-- Platform Hardeningtofu planoutput included (0 add, 1 change, 0 destroy)tofu fmtandtofu validateconfirmed passingPROCESS OBSERVATIONS
tofu applywould fail silently when secrets drifted between the pillar and variables.tf. The validation gate catches this before plan, reducing failed deployments.tofu-validate-secretsin the usage block (lines 6-15). Minor, but the usage section could be updated in a follow-up.VERDICT: APPROVED