- Return error when user refuses at confirmation prompt
- Avoid sending space freed msg if user cancelled
- Fixed unit tests
Signed-off-by: Christopher Petito <chrisjpetito@gmail.com>
The `docker images` top-level subcommand predates the `docker <object> <verb>`
convention (e.g. `docker image ls`), but accepts a positional argument to
search/filter images by name (globbing). It's common for users to accidentally
mistake these commands, and to use (e.g.) `docker images ls`, expecting
to see all images, but ending up with an empty list because no image named
"ls" was found.
Disallowing these search-terms would be a breaking change, but we can print
and informational message to help the users correct their mistake.
Before this patch:
docker images ls
REPOSITORY TAG IMAGE ID CREATED SIZE
With this patch applied:
docker images ls
REPOSITORY TAG IMAGE ID CREATED SIZE
No images found matching "ls": did you mean "docker image ls"?
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use the `XXXVar` equivalent for flags that don't have a shorthand flag
instead of passing an empty string for the shorthand flag.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- rename confusing `target` argument, and use `containerID` in all places;
also make the variable more clearly local-scoped.
- rename `dockerCli` to be correctly camel-case, and to be consistent in
all places in this file.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Docker Engine 1.13 (API v1.25) added an option to set a custom default seccomp
profile on the daemon (see [moby/moby@b237189]). A warning was added on the
client-side if a non-default profile was set.
Docker Engine 23.0 (API v1.42) added warnings about non-default seccomp
profiles to the "info" response ([moby/moby@04f932a]), and the client was
updated to skip generating client-side warnings for API v1.42 and up in
[docker/cli@8964595].
These warnings are purely informational, and given that Docker Engine versions
before 23.0 have reached EOL, and any current version of the Engine now returns
the Warnings, it should be safe to remove the client-side fall back logic.
This patch removes the client-side fall back code for warnings that was
added in 8964595692.
[moby/moby@b237189]: b237189e6c
[moby/moby@04f932a]: 04f932ac86
[docker/cli@8964595]: 8964595692
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was deprecated in 298bddcc23 for
v25.0, and unused. This patch removes the function.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These types were deprecated in 7af509c7f1 (v25.0),
in favor of CLIOption, and are no longer used.
This patch removes the deprecated type-aliases, and while updating, also improves
the documentation for the CLIOption type.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Docker Engine 18.09 (API v1.39) introduced a Warnings field in the into response.
This enhancement was not gated by API version (see [moby/moby@a3d4238]), and
will be returned by Docker Engine 18.09 and up, regardless of the API version
chosen.
Likewise, the client-side code was written to prefer warnings returned by
the daemon, but to fall back on client-side detection of missing features
based on information in the Info response (see [docker/cli@3c27ce2]).
Thse warnings are purely informational, and given that Docker Engine versions
before 18.09 have reached EOL 6 Years ago, and any current version of the
Engine now returns the Warnings, it should be safe to remove the client-side
fall back logic.
This patch removes the client-side fall back code for warnings that was
added in 3c27ce21c9.
[moby/moby@a3d4238]: a3d4238b9c
[docker/cli@3c27ce2]: 3c27ce21c9
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Docker v23.0 and up allow the daemon to be configured to have seccomp disabled
by default (using the "unconfined" profile as default), and introduced a new
"builtin" profile-name for the default (see [moby@f8795ed364586acd][1] and
[mnoby@ac449d6b5ad29a50][2]).
However, the CLI had no special handling for the "builtin" profile, which
resulted in it trying to load it as a file, which would fail;
docker run -it --rm --security-opt seccomp=builtin busybox
docker: opening seccomp profile (builtin) failed: open builtin: no such file or directory.
See 'docker run --help'.
This patch adds a special case for the "builtin" profile, to allow using the
default profile on daemons with seccomp disabled (unconfined) by default.
[1]: f8795ed364
[2]: ac449d6b5a
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit 9e1f8d646e changed this to a
"nolint" comment due to a regression in GoSec. That regression was
fixed, so we can go back to use the more fine-grained "nosec" comment.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The filter option is not currently exposed on the command-line,
but can be added as a flag in future. It will be used by compose
to filter the list of containers to include based on compose
labels.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When running `docker stats` without a list of containers, `runStats` collects
an initial list of containers. If that API call fails, the error is sent to the
`closeChan`, however, `closeChan` is non-buffered, and nothing is reading the
channel until we received the initial list and start collecting stats.
This patch rewrites the code that gets the initial list of containers to
return the error if the API call fails. The `getContainerList` closure is
also removed and inlined to make the logic somewhat easier to read.
Before this patch, the command would hang without producing output;
docker stats
# hangs; no output
With this patch, the error is printed, and the CLI exits:
docker stats
Error response from daemon: some error occurred
echo $?
1
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The monitorContainerEvents and getContainerList closures where only
used when collecting "all" containers, so let's define them in that
branch of the code.
Also move some of the other variables closer to where they're used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We were unconditionally registering event-handlers for these events, but
the handler itself would ignore the event depending on the "all" option.
This patch skips registering the event handlers, so that we're not handling
them (saving some resources).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- memoize the API-client in a local variable.
- use struct-literals in some places.
- rename some variables for clarity and to prevent colliding with imports.
- make use of the event-constants (events.ContainerEventType).
- fix some grammar
- fix some minor linting warnings
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
This driver has been deprecated and removed because the service
is no longer operational. Remove it from the tests to better reflect
reality.
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>
fix some nolintlint false positives
For some reason, nolintlint doesn't consider these used, but they seem to be
legitimate cases where deprecated fields are used.
templates/templates.go:27:29: directive `//nolint:staticcheck // strings.Title is deprecated, but we only use it for ASCII, so replacing with golang.org/x/text is out of scope` is unused for linter "staticcheck" (nolintlint)
"title": strings.Title, //nolint:staticcheck // strings.Title is deprecated, but we only use it for ASCII, so replacing with golang.org/x/text is out of scope
^
cli/command/formatter/image_test.go:75:31: directive `//nolint:staticcheck // ignore SA1019: field is deprecated, but still set on API < v1.44.` is unused for linter "staticcheck" (nolintlint)
call: ctx.VirtualSize, //nolint:staticcheck // ignore SA1019: field is deprecated, but still set on API < v1.44.
^
cli/command/registry/formatter_search.go💯39: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
return c.formatBool(c.s.IsAutomated) //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
^
cli/command/registry/formatter_search_test.go:50:55: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
s: registrytypes.SearchResult{IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
^
cli/command/registry/formatter_search_test.go:53:31: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
call: ctx.IsAutomated, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
^
cli/command/registry/formatter_search_test.go:59:27: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
call: ctx.IsAutomated, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
^
cli/command/registry/formatter_search_test.go:202:84: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
{Name: "result2", Description: "Not official", StarCount: 5, IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/compose/types/types.go:568:17: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return []byte(fmt.Sprintf("%v", e.External)), nil
^
cli/command/formatter/buildcache.go:174:9: fmt.Sprintf can be replaced with faster strconv.Itoa (perfsprint)
return fmt.Sprintf("%d", c.v.UsageCount)
^
cli/command/formatter/buildcache.go:178:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%t", c.v.InUse)
^
cli/command/formatter/buildcache.go:182:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%t", c.v.Shared)
^
cli/command/formatter/image.go:259:9: fmt.Sprintf can be replaced with faster strconv.FormatInt (perfsprint)
return fmt.Sprintf("%d", c.i.Containers)
^
cli/command/formatter/tabwriter/tabwriter_test.go:698:9: fmt.Sprintf can be replaced with faster strconv.Itoa (perfsprint)
b.Run(fmt.Sprintf("%d", x), func(b *testing.B) {
^
cli/command/formatter/tabwriter/tabwriter_test.go:720:9: fmt.Sprintf can be replaced with faster strconv.Itoa (perfsprint)
b.Run(fmt.Sprintf("%d", h), func(b *testing.B) {
^
cli/command/image/prune.go:62:31: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
pruneFilters.Add("dangling", fmt.Sprintf("%v", !options.all))
^
cli/command/network/formatter.go:92:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%v", c.n.EnableIPv6)
^
cli/command/network/formatter.go:96:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%v", c.n.Internal)
^
cli/command/service/formatter.go:745:9: fmt.Sprintf can be replaced with faster strconv.FormatUint (perfsprint)
pub = fmt.Sprintf("%d", pr.pStart)
^
cli/command/service/formatter.go:750:9: fmt.Sprintf can be replaced with faster strconv.FormatUint (perfsprint)
tgt = fmt.Sprintf("%d", pr.tStart)
^
cli/command/service/opts.go:49:10: fmt.Sprintf can be replaced with faster strconv.FormatUint (perfsprint)
return fmt.Sprintf("%v", *i.value)
^
cli/compose/loader/loader.go:720:36: fmt.Sprint can be replaced with faster strconv.Itoa (perfsprint)
v, err := toServicePortConfigs(fmt.Sprint(value))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/idresolver/idresolver.go:33:4: error is not nil (line 31) but it returns nil (nilerr)
return id, nil
^
cli/command/idresolver/idresolver.go:45:4: error is not nil (line 43) but it returns nil (nilerr)
return id, nil
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/utils.go:190:35: param new has same name as predeclared identifier (predeclared)
func StringSliceReplaceAt(s, old, new []string, requireIndex int) ([]string, bool) {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/system/info.go:470:1: deprecatedComment: use `Deprecated: ` (note the casing) instead of `DEPRECATED: ` (gocritic)
// DEPRECATED: warnings are now generated by the daemon, and returned in
^
cli/command/system/info.go:492:1: deprecatedComment: use `Deprecated: ` (note the casing) instead of `DEPRECATED: ` (gocritic)
// DEPRECATED: warnings are now generated by the daemon, and returned in
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/trust/inspect.go:74:33: appendAssign: append result not assigned to the same slice (gocritic)
signatureRows[idx].Signers = append(sig.Signers, releasedRoleName)
^
cli/command/task/print.go:92:7: appendAssign: append result not assigned to the same slice (gocritic)
t := append(tasks[:0:0], tasks...)
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- document accepted values
- add test-coverage for the function's behavior (including whitespace handling),
and use sub-tests.
- improve error-message to use uppercase for "IP", and to use a common prefix.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Christopher Petito
Sebastiaan van Stijn
Paweł Gronowski
Sebastiaan van Stijn
Nicolas De Loof
Brian Goff
David Karlsson