bake: use singular keys for --set overrides - #4089
Conversation
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
83a84ec to
b45b71e
Compare
tonistiigi
left a comment
There was a problem hiding this comment.
Agent suggestions:
Follow-ups (not blocking this PR)
All of these already exist on master. They turned up while reviewing, but they don't belong in this change.
1. Mixing = and += for the same key drops the replacement
Whichever --set comes last decides whether the whole list replaces the target value or appends to it (bake.go#L715, #L721).
target "app" {
tags = ["orig/tag"]
}--set |
Result |
|---|---|
app.tags=a app.tags+=b |
[orig/tag a b]: = is ignored |
app.tags+=a app.tags=b |
[a b] |
With this PR, the same happens when mixing aliases (app.tags=a app.tag+=b).
One possible rule: values repeated for a key still combine, but they append only if every one used +=. Otherwise they replace. That is a behavior change, so it needs a compatibility and release decision. The docs would also need to say that annotation, entitlement and attest always append whichever operator is used.
2. Wrong error text for secret and ssh parse errors
Both wrap the error as invalid value for outputs (#L1312, #L1325). It should name secret and ssh.
3. Broken --set docs example
buildx_bake.md#L435 shows --set foo*.no-cache, which fails:
invalid override foo*.no-cache, expected target.name=value
A value has always been required for every key except args (true since 6634f1e), so the example should be foo*.no-cache=true.
4. Unknown keys are ignored for targets that aren't built
After this PR, newOverrides rejects bad subkeys for every matched target, but unknown keys are still only checked in AddOverrides for targets that are actually built:
$ docker buildx bake app --set other.tag.foo=1 # error: tag does not support subkeys
$ docker buildx bake app --set other.bogus=1 # silently ignorednewOverrides now lists every valid key, so its default: branch (#L766) could reject unknown keys right away. The check could also run once before expandTargets instead of once per matched target.
5. One alias table for newOverrides and AddOverrides
AddOverrides is exported and accepts both singular and plural keys (#L1245 and others). A direct Go caller that passes both tag and tags gets a result that depends on map order. --set is not affected, because newOverrides normalizes the keys first.
A shared map[string]string of aliases, applied in both functions, would remove the duplicated alias lists and make the direct-call behavior deterministic.
|
@tonistiigi On item 5 is that a shared alias map removes the duplicated mappings, but normalization alone doesn't make I also found one related pre-existing case: |
needs bake: fix override value parsing #4088--setand HCL for target platform(s) #2870--setoverrides #3754Singular names are now canonical for
--setoverride selectors, while existing plural spellings remain supported as compatibility aliases. This makesplatformandplatformsinterchangeable and applies the same convention to other collection and map selectors. The documentation distinguishes override selectors from Bake definition attributes and explains exceptions such ascontextversuscontexts.<name>andsecretversussecret.<id>.