Skip to content

fix(update,ssl,deploy): rollback record error, update-plan source codes, per-user certbot config, hasura digest order (D-0541, D-0560, D-0595, D-1154) - #956

Merged
acamarata merged 6 commits into
mainfrom
dw2-cli-3
Oct 26, 2026

Conversation

@acamarata

Copy link
Copy Markdown
Collaborator

Fixes four open debt rows; D-0215 is left open (reason below).

Per row

D-0541 (deploy rollback ignored the record save error)
rollbackRemoteV15 (now in cmd/commands/deploy_lifecycle_rollback.go) returns an error after a successful rollback when .nself/state/deploy.json cannot be saved. The message says the hosts were rolled back but the record was not saved. New test TestRollbackReportsUnsavedRecord makes the state dir read-only; it fails on main (exit nil) and passes now (skipped when run as root).

D-0560 (update --plan/--yes generic E400, status read from text)

  • fetchReleaseFromURL returns a typed *releaseStatusError{Status} (message text unchanged).
  • sourceFailure and the 404 check in gatherPlan read the status with errors.As. Text parsing remains only as a fallback for the plugin registry fetch, which still words its status as text (internal/plugin/registry.go, not changed here).
  • New codes in internal/errs/codes_lock.go (E456/E457 sit in the existing P7-LIVE-09 range): E456 "Update source unavailable", E457 "Update source rate limited" (HTTP 429). Rows added to .github/wiki/error-codes.md with go run ./tools/wikigen.
  • Helpers moved to cmd/commands/update_source_failure.go to keep update_plan.go under the 300-line cap.
  • Tests: TestUpdatePlanSourceFailureCodes (typed, wrapped, wording that mentions 429 but is a 403, no status); the two existing 429 tests now expect E457.

D-0595 (certbot hook scan skipped the per-user config)
certbotHooks also scans $XDG_CONFIG_HOME/letsencrypt/cli.ini, ~/.config/letsencrypt/cli.ini of the caller, and the same file under root's home. Absence is tolerated; so is a permission denial on root's home when the caller is not root (they cannot have written there); any other read error still refuses. certbotUserConfigs is a variable; the test init stubs it so tests never read the real home. New test TestCertbotHooksPerUserConfig fails when the per-user scan is removed.

D-1154 (hasura pin order trusted tag before digest)
hasuraPinOrder now checks the digest against the lock entries first (the digest is what the daemon runs); the tag decides only when the digest is absent or unknown. Added three table cases to TestUpdatePlanHasuraRow; the two mismatched tag@digest cases fail on main.

D-0215 (scaffold Dockerfile base images outside the lock): NOT fixed
Rendering golang, node, python, rust, nginx, distroless and alpine bases from the lock needs new lock entries. Each entry gets a docker.io/nself/<name> mirror (registry publishing), changes which base versions every scaffolded service pins (node 20 and Go 1.23 are today's literals), and feeds the project lock pins and the bump policy. Those are product decisions plus a registry write, so this is left for the owner. Extending the scan alone would need an allowlist of about ten lines, against the ratchet (maxAllowlist = 1).

Commands run

  • go build ./..., go vet ./cmd/... ./internal/errs/..., gofmt -l cmd internal tools (clean)
  • go test ./cmd/commands ./cmd/nself ./internal/errs ./tools/wikigen ./internal/deploy ./internal/compose all ok (full cmd/commands run was before the pure-move refactor commit)
  • after the refactor: go test ./cmd/commands -run 'Update|Certbot|Rollback|Hasura|Retire|ACME|Acme', go test ./internal/repoqa ./cmd/nself ./internal/errs all ok
  • Each new test was run against the unfixed code and failed there.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed in 9d701de: a permission-denied stat of root's per-user cli.ini is now returned from the certbot hook scan, so refuseCertbotHooks refuses with E151 instead of skipping a file that may hold the root-scheduler copy hook. Added TestCertbotHooksRootConfigDenied; it fails on the previous code (nil error) and passes now.

@acamarata
acamarata merged commit 49afa12 into main Oct 26, 2026
35 of 36 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.

1 participant