fix: honour legacy 'source' config key as alias for 'container-engine'#691
Open
ricardbejarano wants to merge 1 commit into
Open
Conversation
Before the CLI refactor introduced in wagoodman#587 (v0.14), dive read the container engine from a viper key named 'source'. Config files with 'source: podman' therefore worked correctly in v0.13.x. After the refactor the struct field was wired up with mapstructure tag 'container-engine', so the canonical key in config became 'container-engine'. The old 'source' key is now silently ignored, breaking existing configs. Fix: read the legacy 'source' key via an unexported struct field and, when 'container-engine' is still at its default value, promote the legacy value with a deprecation warning. If both keys are present 'container-engine' takes precedence, matching the documented behaviour. Tests: add unit tests covering the current key, the legacy key, the precedence rule, and the scheme-prefix override. Fixes wagoodman#643 Signed-off-by: Tommy <tommy@bejarano.io>
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.
Summary
Closes #643.
Before the CLI refactor in #587 (v0.14), dive read the container engine from a viper key named
source. Config files withsource: podmanworked correctly in v0.13.x.After the refactor the struct field was wired up with the mapstructure tag
container-engine, so the canonical config key becamecontainer-engine. The legacysourcekey is now silently ignored, breaking existing configs for users who upgrade from v0.13.x.Root cause
In the old viper-based CLI, the
--sourceflag was bound to the viper keysourceand the config file consumed that same key:After the refactor, the
--sourceflag still exists (for backward CLI compatibility) but it is now bound via fangs to the struct fieldContainerEnginewhich carries the mapstructure tagcontainer-engine. Config files using the oldsource:key are therefore silently ignored.Fix
Add an unexported
LegacySourcefield withmapstructure:"source"so that viper still reads the old key. InPostLoad(), whencontainer-engineis still at its default value andsourcewas explicitly set, promote the legacy value and emit a single deprecation warning. If both keys are present,container-enginetakes precedence.This mirrors the pattern already used by
CIRulesfor its legacy camelCase keys.Testing
Added unit tests in
cmd/dive/cli/internal/options/analysis_test.gocovering:container-engine: podman(current key) resolves correctlycontainer-engine: docker) resolves correctlydocker://...) overrides the config valuesource: podmanresolves to podman engine with a deprecation warningcontainer-enginewins