This is a follow-up to 0e73168b7e
This repository is not yet a module (i.e., does not have a `go.mod`). This
is not problematic when building the code in GOPATH or "vendor" mode, but
when using the code as a module-dependency (in module-mode), different semantics
are applied since Go1.21, which switches Go _language versions_ on a per-module,
per-package, or even per-file base.
A condensed summary of that logic [is as follows][1]:
- For modules that have a go.mod containing a go version directive; that
version is considered a minimum _required_ version (starting with the
go1.19.13 and go1.20.8 patch releases: before those, it was only a
recommendation).
- For dependencies that don't have a go.mod (not a module), go language
version go1.16 is assumed.
- Likewise, for modules that have a go.mod, but the file does not have a
go version directive, go language version go1.16 is assumed.
- If a go.work file is present, but does not have a go version directive,
language version go1.17 is assumed.
When switching language versions, Go _downgrades_ the language version,
which means that language features (such as generics, and `any`) are not
available, and compilation fails. For example:
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
Note that these fallbacks are per-module, per-package, and can even be
per-file, so _(indirect) dependencies_ can still use modern language
features, as long as their respective go.mod has a version specified.
Unfortunately, these failures do not occur when building locally (using
vendor / GOPATH mode), but will affect consumers of the module.
Obviously, this situation is not ideal, and the ultimate solution is to
move to go modules (add a go.mod), but this comes with a non-insignificant
risk in other areas (due to our complex dependency tree).
We can revert to using go1.16 language features only, but this may be
limiting, and may still be problematic when (e.g.) matching signatures
of dependencies.
There is an escape hatch: adding a `//go:build` directive to files that
make use of go language features. From the [go toolchain docs][2]:
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a `go.mod` file with a go line such as `go 1.21` or `go 1.21.3`.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding `//go:build go1.22` to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This patch adds `//go:build` directives to those files using recent additions
to the language. It's currently using go1.19 as version to match the version
in our "vendor.mod", but we can consider being more permissive ("any" requires
go1.18 or up), or more "optimistic" (force go1.21, which is the version we
currently use to build).
For completeness sake, note that any file _without_ a `//go:build` directive
will continue to use go1.16 language version when used as a module.
[1]: 58c28ba286/src/cmd/go/internal/gover/version.go (L9-L56)
[2]; https://go.dev/doc/toolchain#:~:text=The%20go%20line%20for,file%20to%20Go%201.22
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The cli/command package defined two option-types with the same signature.
This patch creates a new type instead (CLIOption), and makes the existing
types an alias for this (deprecating their old names).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Both these functions took the whole DockerCLI as argument, but only needed
the ConfigFile. ResolveAuthConfig also had an unused context.Context as
argument.
This patch updates both functions to accept a ConfigFile, and removes the
unused context.Context.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
More things to be done after this, to allow passing a custom user-agent,
but let's start with just using this utility.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
commandconn: fix race on `Close()`
During normal operation, if a `Read()` or `Write()` call results
in an EOF, we call `onEOF()` to handle the terminating command,
and store it's exit value.
However, if a Read/Write call was blocked while `Close()` is called
the in/out pipes are immediately closed which causes an EOF to be
returned. Here, we shouldn't call `onEOF()`, since the reason why
we got an EOF is because we're already terminating the connection.
This also prevents a race between two calls to the commands `Wait()`,
in the `Close()` call and `onEOF()`
---
Add CLI init timeout to SSH connections
---
connhelper: add 30s ssh default dialer timeout
(same as non-ssh dialer)
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Add a const for the name of the environment-variable we accept, so
that we can document its purpose in code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This fixes the cli erroring out if the variable is set to an empty
value.
```
$ export DOCKER_BUILDKIT=
$ docker version
DOCKER_BUILDKIT environment variable expects boolean value: strconv.ParseBool: parsing "": invalid syntax
```
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The DockerCLI interface was repeating the Streams interface. Embed
the interface to make it more transparent that they're the same.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This allows commands that don't require a client connection (such as `context use`)
to be functional, but still produces an error when trying to run a command that
needs to connect with the API;
mkdir -p ~/.docker/ && echo '{"currentContext":"nosuchcontext"}' > ~/.docker/config.json
docker version
Failed to initialize: unable to resolve docker endpoint: load context "nosuchcontext": context does not exist: open /root/.docker/contexts/meta/8bfef2a74c7d06add4bf4c73b0af97d9f79c76fe151ae0e18b9d7e57104c149b/meta.json: no such file or directory
docker context use default
default
Current context is now "default"
docker version
Client:
Version: 22.06.0-dev
API version: 1.42
...
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This internalizes constructing the Client(), which allows us to provide
fallbacks when trying to determin the current API version.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also move the resolveContextName() function together with the
method for easier cross-referencing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There's no strict need to perform this validation inside this function;
validating flags should happen earlier, to allow faster detecting of
configuration issues (we may want to have a central config "validate"
function though).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
resolveContextName() is used to find which context to use, based on the
available configuration options. Once resolved, the context name is
used to load the actual context, which will fail if the context doesn't
exist, so there's no need to produce an error at this stage; only
check priority of the configuration options to pick the context
with the highest priority.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
CommonOptions was inherited from when the cli and daemon were in the same
repository, and some options would be shared between them. That's no longer
the case, and some options are even "incorrect" (for example, while the
daemon can be configured to run on multiple hosts, the CLI can only connect
with a single host / connection). This patch does not (yet) address that,
but merges the CommonOptions into the ClientOptions.
An alias is created for the old type, although it doesn't appear there's
any external consumers using the CommonOptions type (or its constructor).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The package defined various special errors; these errors existed for two reasons;
- being able to distinguish "not found" errors from other errors (as "not found"
errors can be ignored in various cases).
- to be able to update the context _name_ in the error message after the error
was created. This was needed in cases where the name was not available at the
location where the error was produced (e.g. only the "id" was present), and
the helpers to detect "not found" errors did not support wrapped errors (so
wrapping the error with a "name" could break logic); a `setContextName` interface
and corresponding `patchErrContextName()` utility was created for this (which
was a "creative", but not very standard approach).
This patch:
- Removes the special error-types, replacing them with errdefs definitions (which
is a more common approach in our code-base to detect error types / classes).
- Removes the internal utilities for error-handling, and deprecates the exported
utilities (to allow external consumers to adjust their code).
- Some errors have been enriched with detailed information (which may be useful
for debugging / problem solving).
- Note that in some cases, `patchErrContextName()` was called, but the code
producing the error would never return a `setContextName` error, so would
never update the error message.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`NewDockerCli` was configuring the standard streams using local code; this patch
instead uses the available `WithStandardStreams()` option to do the same.
There is slight difference in the order of events;
Previously, user-provided options would be applied first, after which NewDockerCli
would check if any of "in", "out", or "err" were nil, and if so set them to the
default stream (or writer) for that output.
The new code unconditionally sets the defaults _before_ applying user-provided
options. In practive, howver, this makes no difference; the fields set are not
exported, and the only functions updating them are `WithStandardStreams`,
`WithInputStream`, and `WithCombinedStream`, neither of which checks the old
value (so always overrides).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Note that this does not fully fix the referenced issue, but
at least makes sure that API clients don't hang forever on
the initialization step.
See: https://github.com/docker/cli/issues/3652
Signed-off-by: Nick Santos <nick.santos@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's slightly more verbose, but helps finding the purpose of each
of the environment variables. In tests, I kept the fixed strings.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The information in this struct was basically fixed (there's
some discrepancy around the "DefaultVersion" which, probably,
should never vary, and always be set to the Default (maximum)
API version supported by the client.
Experimental is now always enabled, so this information did
not require any dynamic info as well.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Just `config` as name for the package should work; this also revealed that one
file was importing the same package twice.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
> Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since
> it does not authenticate the ciphertext, it is vulnerable to padding oracle
> attacks that can let an attacker recover the plaintext
From https://go-review.googlesource.com/c/go/+/264159
> It's unfortunate that we don't implement PKCS#8 encryption so we can't
> recommend an alternative but PEM encryption is so broken that it's worth
> deprecating outright.
This feature allowed using an encrypted private key with a supplied password,
but did not provide additional security as the encryption is known to be broken,
and the key is sitting next to the password in the filesystem. Users are recommended
to decrypt the private key, and store it un-encrypted to continue using it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
> Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since
> it does not authenticate the ciphertext, it is vulnerable to padding oracle
> attacks that can let an attacker recover the plaintext
From https://go-review.googlesource.com/c/go/+/264159
> It's unfortunate that we don't implement PKCS#8 encryption so we can't
> recommend an alternative but PEM encryption is so broken that it's worth
> deprecating outright.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The CLI disabled experimental features by default, requiring users
to set a configuration option to enable them.
Disabling experimental features was a request from Enterprise users
that did not want experimental features to be accessible.
We are changing this policy, and now enable experimental features
by default. Experimental features may still change and/or removed,
and will be highlighted in the documentation and "usage" output.
For example, the `docker manifest inspect --help` output now shows:
EXPERIMENTAL:
docker manifest inspect is an experimental feature.
Experimental features provide early access to product functionality. These features
may change between releases without warning or can be removed entirely from a future
release. Learn more about experimental features: https://docs.docker.com/go/experimental/
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When initializing the API client, the User-Agent was added to any custom
HTTPHeaders that were configured. However, because the map was not properly
dereferenced, the original map was modified, causing the User-Agent to also
be saved to config.json after `docker login` and `docker logout`:
Before this change;
$ cat ~/.docker/config.json
cat: can't open '/root/.docker/config.json': No such file or directory
$ docker login -u myusername
Password:
...
Login Succeeded
$ cat ~/.docker/config.json
{
"auths": {
"https://index.docker.io/v1/": {
"auth": "<base64 auth>"
}
},
"HttpHeaders": {
"User-Agent": "Docker-Client/19.03.12 (linux)"
}
}
$ docker logout
{
"auths": {},
"HttpHeaders": {
"User-Agent": "Docker-Client/19.03.12 (linux)"
}
}
After this change:
$ cat ~/.docker/config.json
cat: can't open '/root/.docker/config.json': No such file or directory
$ docker login -u myusername
Password:
...
Login Succeeded
$ cat ~/.docker/config.json
{
"auths": {
"https://index.docker.io/v1/": {
"auth": "<base64 auth>"
}
}
}
$ docker logout
Removing login credentials for https://index.docker.io/v1/
$ cat ~/.docker/config.json
{
"auths": {}
}
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These packages are now living in their own repository. Updating
docker/docker to replace the dependencies.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Perform feature detection when actually needed, instead of during
initializing
- Version negotiation is performed either when making an API request,
or when (e.g.) running `docker help` (to hide unsupported features)
- Use a 2 second timeout when 'pinging' the daemon; this should be
sufficient for most cases, and when feature detection failed, the
daemon will still perform validation (and produce an error if needed)
- context.WithTimeout doesn't currently work with ssh connections (connhelper),
so we're only applying this timeout for tcp:// connections, otherwise
keep the old behavior.
Before this change:
time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker help &> /dev/null'
real 0m32.919s
user 0m0.370s
sys 0m0.227s
time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker context ls &> /dev/null'
real 0m32.072s
user 0m0.029s
sys 0m0.023s
After this change:
time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker help &> /dev/null'
real 0m 2.28s
user 0m 0.03s
sys 0m 0.03s
time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker context ls &> /dev/null'
real 0m 0.13s
user 0m 0.02s
sys 0m 0.02s
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn
Brian Goff
Sebastiaan van Stijn
Laura Brehm
Paweł Gronowski
Nick Santos
Nick Santos
CrazyMax
Nicolas De Loof
Daniil Nikolenko
Silvin Lubecki