@tonistiigi @tiborvass @AkihiroSuda PTAL if the changes for secrets and git require documentation changes and if local changes are needed to expose the functionality

Ah, guess this answers some of my questions;

#14 0.375 Building statically linked build/docker-linux-amd64
#14 26.34 # github.com/docker/cli/cli/command/image
#14 26.34 cli/command/image/build_buildkit.go:427:15: undefined: secretsprovider.FileSource
#14 26.34 cli/command/image/build_buildkit.go:435:16: undefined: secretsprovider.NewFileStore
#14 26.34 cli/command/image/build_buildkit.go:442:34: undefined: secretsprovider.FileSource
#14 26.34 cli/command/image/build_buildkit.go:449:8: undefined: secretsprovider.FileSource
#14 ERROR: executor failed running [/bin/sh -c ./scripts/build/binary]: runc did not terminate sucessfully

Codecov Report

Merging #2656 into master will increase coverage by 0.01%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master    #2656      +/-   ##
==========================================
+ Coverage   58.54%   58.55%   +0.01%     
==========================================
  Files         296      296              
  Lines       21286    21293       +7     
==========================================
+ Hits        12462    12469       +7     
  Misses       7915     7915              
  Partials      909      909              

Spotted possible issue, fixed in moby/buildkit#1613 . Don't need to be vendored but just same fix can be done in cli.

I'm a bit confused by the UX. Looking at it, it doesn't seem consistent.

The --secret flag currently supports these keys: type, id, source, src, env

• id is a required option
• type is full ignored, unless an invalid type is specified (anything other than "file" or "env" (the code doesn't follow different paths if file or env is specified)
• both source, src and env can be specified at the same time.
  • this is ambiguous if no explicit "type" is set
• if type is set to "env", source / src acts as an alias for env (if unset)
• if only id is set (and no source / src or env), and an environment variable is found that matches that name, the default is now switched to "take value from the environment variable"
  • This is a breaking change: previously id would be used as filename

I think

• if no type is set, and no source / src / env, use the current default and use id as source (current behavior)
• if no type is set, but source / src is set; implicitly assume type=file (could be seen as same as above, as file is default)
• if no type is set, but env is set; implicitly assume type=env
• if type=file
  • only look at source / src (ignore env)
  • fall back to id if not set
  • produce error if env is set? (invalid option for type=file ?
• if type=env
  • look at env to use as "source"
  • fall back to source / src if not set
  • fall back to id if not set
  • produce error if both env and source / src are set?

However, from the above; does env= provide much value, given that we already have type=env, and that source/src and id already provide the functionality?
Effectively if would only be useful to allow switching between file and env without having to explicitly set type=.

One "feature" could be to explicitly allow env as a "fallback", so;

--secret id=foo-token,src=~/my-token.txt,env=FOO_TOKEN

Which would either pick the value from $FOO_TOKEN or from ~/my-token.txt (not sure in which order though)

If we would remove env=, the combinations would be reduced quite a bit;

• if type=file or if no type is set (default)
  • look at source / src
  • fall back to id if not set
  • produce error if the file was not found
• if type=env
  • look at source / src
  • fall back to id if not set
  • produce error if no environment variable with the given name was found? (TBD)