feat(overrides): add profile and explicit override merge support#20
Conversation
wax911
left a comment
wax911
left a comment
There was a problem hiding this comment.
Review notes against #8:
-
The PR body says
--override <file>is supported ongenerateandsync. That contradicts the plan and issue non-goals. Overrides belong in the runtime path: base generated stack -> override merge -> render -> deploy. Canonicalgenerateshould emit base stack artifacts, and canonicalsyncshould validate only base generated stacks unless a separate explicit override-validation command is added. -
syncmust remain focused on committed canonical stack drift. Applying overrides during sync risks making local/machine-specific state look canonical, which was explicitly rejected in the local-stack planning issue. -
If an override flag exists on
generate, it needs to be removed or renamed to a distinct non-canonical operation. Otherwise it creates exactly the class of generated-stack mutation we were trying to avoid. -
Keep explicit overrides for
render,up,reload, andplanfirst. Add a futurevalidate-overridescommand only if there is a strong need.
Please correct the command surface before this closes #8.
wax911
changed the base branch from
main
to
feat/4-port-stack-generation-to-deno
- Remove --override from generate command (base artifacts only) - Remove --override from sync command (canonical comparison only) - Keep --override on render, up, reload, plan only
wax911
left a comment
wax911
left a comment
There was a problem hiding this comment.
Follow-up review after the push:
The PR is now stacked on #19, which is better structurally.
Remaining blocker: --override is still wired into generate, and the PR body still says override support exists on generate and sync. That remains contrary to the accepted model.
Canonical generation must stay clean:
compose sources -> generated base stack files
Runtime override application belongs here:
base generated stack -> override merge -> render -> up/reload/plan
Please remove --override from generate and keep override application to render, up, reload, and plan. If override validation is needed later, add a separate explicit command such as stackctl validate-overrides or stackctl plan render --override ...; do not mutate the canonical generation path.
wax911
force-pushed
the
feat/4-port-stack-generation-to-deno
branch
from
1d6cee2 to
0c01b0f
Compare
- Deno 2.x project structure with deno.json and task definitions - JSR dependencies: @cliffy/command, @std/assert, @std/testing, @std/yaml, @std/dotenv, @std/fs, @std/path - Full CLI command tree with stubs for all 15 issues - Shared interfaces (ProcessRunner, config types, ExitCode) for parallel work - FakeProcessRunner with recording, pre-programmed responses, and dry-run support - CI pipeline: fmt, lint, typecheck, test, coverage, and cross-platform build - .gitignore for generated and environment-specific files
- Default config values with sensible defaults - Deep merge for 5-layer config resolution (defaults -> base -> profile -> local -> local-profile) - Filesystem discovery (.stackctl, .stackctl.<profile>, .stackctl.local, .stackctl.local.<profile>) - Post-merge validation returning all errors at once - Template generation with inline comments, --detect, --preset, --profile, --force, --dry-run - STACKCTL_PROFILE env var support - 43 config tests + existing 15 = 58 passing - CLI init command wired to real implementation
Port of tools/generate_stacks.py from AniTrend/local-stack to idiomatic Deno TypeScript: - File discovery: walks repo root, finds docker-compose.yml/yaml files with x-stack metadata - Fragment loading: optional swarm.fragment.yml deep-merge per service - Compose deep merge (dict recursive, array replacement, scalar override) - Service transforms: strip compose-only keys (container_name, restart, build), inject logging defaults, rewrite env_file and bind-mount paths to repo-root relative - Named volume collection (external: true), default traefik-public overlay network - YAML output with header comment, --dry-run support - CLI generate command wired to real implementation - 60 compose tests + 58 existing = 118 passing
- composeOverrideMerge: scalars replace, maps merge, sequences append (distinct from fragment merge which replaces arrays) - loadOverrideFile: load YAML override from relative/absolute path - applyOverrides: load and apply chain of override files to base compose - Override integration in generateStacks via GenerateOptions.overrides - 26 tests covering all merge rules, file loading, edge cases - CLI generate command accepts --override flag
- Variable interpolation: ${VAR}, ${VAR-default}, ${VAR:-default}, $VAR, $$
- Variable scope resolution: shell env -> env_file(s) -> service.environment
- Deep interpolation through all string values in compose structures
- Path absolutization for env_file and bind-mount paths
- Strict mode (fail on unresolved) and non-strict mode (leave as-is with warnings)
- CLI pipeline: resolveConfig -> generateStacks -> renderStack -> output
- 49 comprehensive tests covering all interpolation forms and edge cases
- Remove --override from generate command (base artifacts only) - Remove --override from sync command (canonical comparison only) - Keep --override on render, up, reload, plan only
wax911
force-pushed
the
feat/8-add-override-merge-support
branch
from
a3b6c26 to
37145ff
Compare
1 participant
Closes #8