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>
This test was depending on the fact that contextDir's are a string,
and for the test is was using the context _name_ as a pseudo-ID.
This patch updates the test to be more explicit where ID's and where
names are used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This allows callers to just pass the name, and handle the conversion to ID and
path internally. This also fixes a test which incorrectly used "names" as
pseudo-IDs.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this change, running `docker context rm --force` would fail if the context
did not exist. This behavior was different from other commands, which allowed
ignoring non-existing objects.
For example; when trying to remove a non-existing volume, the command would
fail without "force":
```bash
docker volume rm nosuchvolume
Error: No such volume: nosuchvolume
echo $?
1
```
But using the `-f` / `--force` option would make the command complete successfully
(the error itself is still printed for informational purposes);
```bash
docker volume rm -f nosuchvolume
nosuchvolume
echo $?
0
```
With this patch, `docker context rm` behaves the same:
```bash
docker context rm nosuchcontext
context "nosuchcontext" does not exist
echo $?
1
```
```bash
docker context rm -f nosuchcontext
nosuchcontext
echo $?
0
```
This patch also simplifies how we check if the context exists; previously we
would try to read the context's metadata; this could fail if a context was
corrupted, or if an empty directory was present. This patch now only checks
if the directory exists, without first validating the context's data.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also removing redundant defer for env.PatchAll(), which is now automatically
handled in t.Cleanup()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/container/opts.go:928:2: assigned to src, but reassigned without using the value (wastedassign)
src := ""
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
While fixing, also updated errors without placeholders to `errors.New()`, and
updated some code to use pkg/errors if it was already in use in the file.
cli/command/config/inspect.go:59:10: ST1005: error strings should not be capitalized (stylecheck)
return fmt.Errorf("Cannot supply extra formatting options to the pretty template")
^
cli/command/node/inspect.go:61:10: ST1005: error strings should not be capitalized (stylecheck)
return fmt.Errorf("Cannot supply extra formatting options to the pretty template")
^
cli/command/secret/inspect.go:57:10: ST1005: error strings should not be capitalized (stylecheck)
return fmt.Errorf("Cannot supply extra formatting options to the pretty template")
^
cli/command/trust/common.go:77:74: ST1005: error strings should not be capitalized (stylecheck)
return []trustTagRow{}, []client.RoleWithSignatures{}, []data.Role{}, fmt.Errorf("No signatures or cannot access %s", remote)
^
cli/command/trust/common.go:85:73: ST1005: error strings should not be capitalized (stylecheck)
return []trustTagRow{}, []client.RoleWithSignatures{}, []data.Role{}, fmt.Errorf("No signers for %s", remote)
^
cli/command/trust/sign.go:137:10: ST1005: error strings should not be capitalized (stylecheck)
return fmt.Errorf("No tag specified for %s", imgRefAndAuth.Name())
^
cli/command/trust/sign.go:151:19: ST1005: error strings should not be capitalized (stylecheck)
return *target, fmt.Errorf("No tag specified")
^
cli/command/trust/signer_add.go:77:10: ST1005: error strings should not be capitalized (stylecheck)
return fmt.Errorf("Failed to add signer to: %s", strings.Join(errRepos, ", "))
^
cli/command/trust/signer_remove.go:52:10: ST1005: error strings should not be capitalized (stylecheck)
return fmt.Errorf("Error removing signer from: %s", strings.Join(errRepos, ", "))
^
cli/command/trust/signer_remove.go:67:17: ST1005: error strings should not be capitalized (stylecheck)
return false, fmt.Errorf("All signed tags are currently revoked, use docker trust sign to fix")
^
cli/command/trust/signer_remove.go:108:17: ST1005: error strings should not be capitalized (stylecheck)
return false, fmt.Errorf("No signer %s for repository %s", signerName, repoName)
^
opts/hosts.go:89:14: ST1005: error strings should not be capitalized (stylecheck)
return "", fmt.Errorf("Invalid bind address format: %s", addr)
^
opts/hosts.go💯14: ST1005: error strings should not be capitalized (stylecheck)
return "", fmt.Errorf("Invalid proto, expected %s: %s", proto, addr)
^
opts/hosts.go:119:14: ST1005: error strings should not be capitalized (stylecheck)
return "", fmt.Errorf("Invalid proto, expected tcp: %s", tryAddr)
^
opts/hosts.go:144:14: ST1005: error strings should not be capitalized (stylecheck)
return "", fmt.Errorf("Invalid bind address format: %s", tryAddr)
^
opts/hosts.go:155:14: ST1005: error strings should not be capitalized (stylecheck)
return "", fmt.Errorf("Invalid bind address format: %s", tryAddr)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We try to keep this package close to upstream golang's code, so suppress the
linter warning.
cli/command/formatter/tabwriter/tabwriter.go:200:1: ST1020: comment on exported method Init should be of the form "Init ..." (stylecheck)
// A Writer must be initialized with a call to Init. The first parameter (output)
^
cli/command/formatter/tabwriter/tabwriter.go:425:1: ST1022: comment on exported const Escape should be of the form "Escape ..." (stylecheck)
// To escape a text segment, bracket it with Escape characters.
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/cli_options_test.go:29:2: os.Setenv() can be replaced by `t.Setenv()` in TestWithContentTrustFromEnv (tenv)
os.Setenv(envvar, "true")
^
cli/command/cli_options_test.go:31:2: os.Setenv() can be replaced by `t.Setenv()` in TestWithContentTrustFromEnv (tenv)
os.Setenv(envvar, "false")
^
cli/command/cli_options_test.go:33:2: os.Setenv() can be replaced by `t.Setenv()` in TestWithContentTrustFromEnv (tenv)
os.Setenv(envvar, "invalid")
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/manifest/inspect_test.go:9:2: ST1019: package "github.com/docker/cli/cli/manifest/types" is being imported more than once (stylecheck)
"github.com/docker/cli/cli/manifest/types"
^
cli/command/manifest/inspect_test.go:10:2: ST1019(related information): other import of "github.com/docker/cli/cli/manifest/types" (stylecheck)
manifesttypes "github.com/docker/cli/cli/manifest/types"
^
cli/command/stack/swarm/deploy_composefile.go:14:2: ST1019: package "github.com/docker/docker/client" is being imported more than once (stylecheck)
apiclient "github.com/docker/docker/client"
^
cli/command/stack/swarm/deploy_composefile.go:15:2: ST1019(related information): other import of "github.com/docker/docker/client" (stylecheck)
dockerclient "github.com/docker/docker/client"
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/image/build/context.go:238:23: "400" can be replaced by http.StatusBadRequest (usestdlibvars)
if resp.StatusCode < 400 {
^
cli/trust/trust.go:139:30: "GET" can be replaced by http.MethodGet (usestdlibvars)
req, err := http.NewRequest("GET", endpointStr, nil)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Having the intermediate variable made it difficult to see if it was
possibly mutated and/or something special done with it, so just use
the cli's accessors to get its Err().
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
As it's just an alias for filepath.IsAbs. Also added a normalize step in
TrimBuildFilesFromExcludes, so that callers are not _required_ to first
normalize the path.
We are considering deprecating and/or removing this function in the archive
package, so removing it in the cli code helps transitioning if we decide to
deprecate and/or remove it.
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>
Migrating these functions to allow them being shared between moby, docker/cli,
and containerd, and to allow using them without importing all of sys / system,
which (in containerd) also depends on hcsshim and more.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- moby: a60b458179...d2590dc3cd
- swarmkit: 6068d1894d...48dd89375d
The .Parent field for buildcache entries was deprecated, and replaced with a
.Parents (plural) field. This patch updates the code accordingly. Unlike the
change in buildx
9c3be32bc9
we continue to fall back to the old field (which will be set on older API
versions).
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>
Avoid updating the config-file if nothing changed. This also prevents creating
the file and config-directory if the default is used and no config-file existed
yet.
`config.Save()` performs various steps (creating the directory, updating
or copying permissions, etc etc), which are not needed if the defaults are
used; a445d97c25/cli/config/configfile/file.go (L135-L176)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This code was handling validation and parsing, only to discard the
results if it was the default context.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Older versions of Go do not format these comments, so we can already
reformat them ahead of time to prevent gofmt linting failing once
we update to Go 1.19 or up.
Result of:
gofmt -s -w $(find . -type f -name '*.go' | grep -v "/vendor/")
With some manual adjusting.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Cobra allows for aliases to be defined for a command, but only allows these
to be defined at the same level (for example, `docker image ls` as alias for
`docker image list`). Our CLI has some commands that are available both as a
top-level shorthand as well as `docker <object> <verb>` subcommands. For example,
`docker ps` is a shorthand for `docker container ps` / `docker container ls`.
This patch introduces a custom "aliases" annotation that can be used to print
all available aliases for a command. While this requires these aliases to be
defined manually, in practice the list of aliases rarely changes, so maintenance
should be minimal.
As a convention, we could consider the first command in this list to be the
canonical command, so that we can use this information to add redirects in
our documentation in future.
Before this patch:
docker images --help
Usage: docker images [OPTIONS] [REPOSITORY[:TAG]]
List images
Options:
-a, --all Show all images (default hides intermediate images)
...
With this patch:
docker images --help
Usage: docker images [OPTIONS] [REPOSITORY[:TAG]]
List images
Aliases:
docker image ls, docker image list, docker images
Options:
-a, --all Show all images (default hides intermediate images)
...
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this change, specifying the `--pull` flag without a value, could
result in the flag after it, or the positional argument to be used as
value.
This patch makes sure that the value is an expected value;
docker create --pull --rm hello-world
docker: invalid pull option: '--rm': must be one of "always", "missing" or "never".
docker run --pull --rm hello-world
docker: invalid pull option: '--rm': must be one of "always", "missing" or "never".
docker run --pull hello-world
docker: invalid pull option: 'hello-world': must be one of "always", "missing" or "never".
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The default output for Cobra aliases only shows the subcommand as alias, which
is not very intuitive. This patch changes the output to print the full command
as it would be called by the user.
Note that there's still some improvements to be made; due to how aliases must be
set-up in Cobra, aliases at different "levels" are still not shown. So for example,
`docker ps --help` will not show `docker container ps` as alias, and vice-versa.
This will require additional changes, and can possibly be resolved using custom
metadata/annotations.
Before this patch:
docker container ls --help
Usage: docker container ls [OPTIONS]
List containers
Aliases:
ls, ps, list
After this patch:
docker container ls --help
Usage: docker container ls [OPTIONS]
List containers
Aliases:
docker container ls, docker container ps, docker container list
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn
Jason Hall
Sebastiaan van Stijn
Nick Santos
Nick Santos