feat(config): implement commented .stackctl init and profile discovery#18
Conversation
- 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
wax911
left a comment
There was a problem hiding this comment.
Review notes against #3:
-
Profile ambiguity/default-profile behavior is not implemented.
resolveConfig()only reads--profile/STACKCTL_PROFILE; it does not useproject.defaultProfile, and it does not detect multiple.stackctl.*files to prompt interactively or fail in non-interactive mode as required. -
stackctl init --detectis too shallow for the local-stack use case. It only scans first-level files incwd, derives stack names from service names, and does not inspect nested service compose files,x-stack, existing stack files, sibling swarm fragments, SOPS config, example env files, or encrypted env files as required. -
--write-gitignoreis parsed by the CLI but is not passed intoinitConfig()or implemented. That path is currently dead. -
Profile generation writes the same full base template to
.stackctl.<profile>. A profile overlay should be shorter, profile-specific, and include aprofile: <name>field or equivalent intent. Copying the full base config increases drift risk and does not match the planned overlay model. -
The generated template is not parsed/validated before writing. The issue explicitly requires generated commented config to be reparsed before save so broken templates cannot be emitted.
-
The template schema does not match the agreed plan. It uses
project: ""andstack:whereas the planned shape used structured sections for project, stacks, generation, render, docker, env, secrets, overrides, and commands. Either align the schema or explicitly update the issue/spec before merging.
This needs another pass before it can close #3.
- Add defaultProfile fallback when --profile not specified - Detect multiple .stackctl.* files and fail on ambiguity - Deepen --detect to scan subdirectories, fragments, SOPS, env files - Move --write-gitignore into initConfig function - Generate shorter profile-specific overlays instead of full templates - Validate generated template before writing - Expand template schema with secrets, overrides, commands sections
wax911
left a comment
There was a problem hiding this comment.
Follow-up review after the push:
Fixed/improved:
--write-gitignorenow exists inInitOptionsand is passed from the CLI.- Generated config is parsed before writing.
- Profile generation now uses a smaller overlay template with an explicit
profilefield. defaultProfile/profile fallback logic was added.- Profile ambiguity is now detected and reported.
Remaining issues:
-
The PR is currently not mergeable. Resolve the conflict/base issue before it is considered ready.
-
Detection is still not aligned with local-stack. It scans only root plus one directory deep. local-stack can contain nested service directories, so this should use configured discovery globs / recursive walk with skip directories.
-
Detection still confuses stack names and services. The local-stack convention is a top-level compose extension
x-stack: <stackName>in each service compose file. The current detection mostly derives names from service names unless it finds service-levelx-stack, which is not the same contract. -
Existing
stacks/*.ymlfiles are scanned but added tofragments; they should be used to infer stack names from filenames. -
The config schema still diverges from the earlier agreed shape (
project,stack,render) instead of the richerproject.name,stacks,generation,docker,overrides, etc. That may be acceptable if the implementation intentionally simplified the schema, but then update the issue/spec/docs so every downstream PR uses the same contract.
This is materially better, but still not ready to close #3 for local-stack compatibility.
Closes #3