Commit Graph

1614 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 9c42cd9a3e
cli/command: TestInitializeFromClientHangs fix unhandled error
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-22 16:44:09 +01:00
Sebastiaan van Stijn e36d5a0929
cli/command: DockerCli.CurrentContext: improve GoDoc
Also move the resolveContextName() function together with the
method for easier cross-referencing.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-22 14:27:44 +01:00
Sebastiaan van Stijn 793f09705d
cli/command: resolveContextName() move conflicting options check
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>
2022-11-22 13:54:28 +01:00
Sebastiaan van Stijn 2f5698511a
cli/command: resolveContextName() don't validate if context exists
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>
2022-11-22 13:53:10 +01:00
Sebastiaan van Stijn de52868abb
Merge pull request #3877 from thaJeztah/minor_context_cleanups
context: various minor cleanups and fixes
2022-11-22 13:34:35 +01:00
Sebastiaan van Stijn 592d90cafa
cli/command/context: minor cleanup in runList()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-22 12:57:57 +01:00
Sebastiaan van Stijn 0cc3f688d6
cli/command: use more descriptive const in test
On failures, the output wasn't clear which side was "expected" and
which side was "actual".

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-22 12:57:57 +01:00
Sebastiaan van Stijn acb934cc9d
cli/command: move default context description to context itself
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-22 12:57:54 +01:00
Sebastiaan van Stijn 3499669e18
cli/flags: merge CommonOptions into ClientOptions
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>
2022-11-22 12:32:18 +01:00
Sebastiaan van Stijn 6a734df1cc
cli/context/store: make sure we handle wrapped errors
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-22 12:20:32 +01:00
Sebastiaan van Stijn bf3f419b6e
cli/command/stack: TestConfigMergeInterpolation: various fixes
- Make the package-level configMergeTests local to the test itself.
- Rename fields to better describe intent
- Remove some redundant variables
- Reverse "expected" and "actual" fields for consistency
- Use assert.Check() to not fail early

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-17 13:17:51 +01:00
Sebastiaan van Stijn 4d2fb68b93
cli/compose/loader: fix error messages, and various tests
Various fixes:

- Don't capitalize error messages
- Rename variables that collided with imports or types
- Prefer assert.Check over assert.Assert to prevent tests covering multiple
  cases from failing early
- Fix inconsistent order of expected <--> actual, which made it difficult to
  check which output was the expected output.
- Fix formatting of some comments

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-17 13:17:51 +01:00
Sebastiaan van Stijn 0644aa3906
cli/compose/types: UlimitsConfig.MarshalYAML() fix recursion
When marshaling the type with `gopkg.in/yaml.v3`, unmarshaling would
recursively call the type's `MarshalYAML()` function, which ultimately
resulted in a crash:

    runtime: goroutine stack exceeds 1000000000-byte limit
    runtime: sp=0x140202e0430 stack=[0x140202e0000, 0x140402e0000]
    fatal error: stack overflow

This applies a similar fix as was implemented in e7788d6f9a
for the `MarshalJSON()` implementation. An alternative would be to use
a type alias (to remove the `MarshalYAML()`), but keeping it simple.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-17 13:17:51 +01:00
Sebastiaan van Stijn c18dd2719e
cli/compose/loader: TestMarshallConfig: fix duplicate version
The version was originally added in 570ee9cb54,
at the time the `expected` config did not have a `version:` field. A later
refactor in 0cf2e6353a updated the `expected`
config to have a `version:` included. However, the test was not updated,
which now resulted in the test using a compose file with a duplicate version
field:

    version: '3.10'
    version: "3.10"
      services:
        foo:
          build:

This issue was masked by `yaml.Unmarshal()` from `gopkg.in/yaml.v2` which
silently ignores the duplicate, taking the value of the last occurrence. When
upgrading to `gopkg.in/yaml.v3`, the duplicate value resulted in an error:

    yaml: unmarshal errors:
        line 2: mapping key "version" already defined at line 1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-17 13:17:50 +01:00
Brian Goff 4011187b78
Merge pull request #3305 from crazy-max/rm-stream
Cleanup leftovers for --stream build flag
2022-11-09 11:57:38 -08:00
Sebastiaan van Stijn 8a19043cc7
Merge pull request #3798 from thaJeztah/gofumpt_linting
Use gofumpt if available, and enable gofumpt linter
2022-11-04 19:04:51 +01:00
CrazyMax 8dec4bcce8
cleanup leftovers for --stream build flag
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2022-11-04 08:16:04 +01:00
Sam Thibault 3dfef7691e
Merge pull request #3662 from dperny/cluster-volumes-compose
Add compose support for cluster volumes
2022-11-02 18:57:08 +01:00
Drew Erny 02e7826923 Add compose support for cluster volumes
Signed-off-by: Drew Erny <derny@mirantis.com>
2022-11-02 11:27:06 -05:00
Sebastiaan van Stijn 6fe31557cb
vendor: docker v20.10.3-0.20221006185438-87d9d96ab0b6 (v22.06-dev)
full diff: e143eed8bc...87d9d96ab0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-21 15:15:11 +02:00
Sebastiaan van Stijn 616124525e
format go with gofumpt (with -lang=1.19)
Looks like the linter uses an explicit -lang, which (for go1.19)
results in some additional formatting for octal values.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 19:14:36 +02:00
Sebastiaan van Stijn 1da95ff6aa
format code with gofumpt
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 11:59:11 +02:00
Sebastiaan van Stijn fed93645f1
Merge pull request #3796 from thaJeztah/add_is_plugincommand_utility
cli-plugins/manager: add IsPluginCommand(() utility
2022-09-30 11:58:54 +02:00
Sebastiaan van Stijn cd7c493ea2
cli/context/store: rename removeAllContextData(), removeAllEndpointData()
The existing `remove()` was unused, and using that as name makes it more
consistent with the metadata-store. Also renaming `removeAllEndpointData`
to just `removeEndpoint`, as it's part of the TLS-store, which should already
make it clear it's about (TLS)data.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 11:19:26 +02:00
Sebastiaan van Stijn 09c94c1c21
cli/context/store: List(): don't interrupt listing for not-found errors
There's no reason to stop listing contexts if a context does not exist
while iterating over the directories,

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 11:19:26 +02:00
Sebastiaan van Stijn 951bb481c0
cli/context/store: New(): return concrete type
Go conventions are for interfaces to be defined on the receiver side,
and for producers to return concrete types. This patch changes the
constructor to return a concrete type.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 11:19:25 +02:00
Sebastiaan van Stijn de6020a240
cli/context/store: simplify error handling, and make it more idiomatic
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>
2022-09-30 11:19:25 +02:00
Sebastiaan van Stijn 38f54e7926
cli/context/store: remove filePath(), make contextDir() accept name
removing the extra abstraction, and simplify use of contextDir()

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 11:18:52 +02:00
Sebastiaan van Stijn 9720d5b451
cli/context/store: listContextData(): accept name instead of ID
This allows callers to just pass the name, and handle the conversion to ID and
path internally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 10:35:06 +02:00
Sebastiaan van Stijn 712cc9a1c7
cli/context/store: remove(): accept name instead of ID
This allows callers to just pass the name, and handle the conversion to ID and
path internally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 10:35:06 +02:00
Sebastiaan van Stijn 0bcdff2571
cli/context/store: getData(): accept name instead of ID
This allows callers to just pass the name, and handle the conversion to ID and
path internally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 10:35:06 +02:00
Sebastiaan van Stijn 3b7f13a5e5
cli/context/store: createOrUpdate(): accept name instead of ID
This allows callers to just pass the name, and handle the conversion to ID and
path internally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 10:35:06 +02:00
Sebastiaan van Stijn c3eb116f9c
cli/context/store: removeAllEndpointData(): accept name instead of ID
This allows callers to just pass the name, and handle the conversion to ID and
path internally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 10:35:06 +02:00
Sebastiaan van Stijn 42e275eaf6
cli/context/store: TestTlsCreateUpdateGetRemove(): use correct ID
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>
2022-09-30 10:35:05 +02:00
Sebastiaan van Stijn d0398c423f
cli/context/store: removeAllContextData(): accept name instead of ID
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>
2022-09-30 10:35:05 +02:00
Sebastiaan van Stijn f843c42c05
cli/context/store: listRecursivelyMetadataDirs(): use filepath.Join()
Looks like the intent here is to get the path of a subdirectory.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 10:35:05 +02:00
Sebastiaan van Stijn c6dfff131f
cli/context/store: metadataStore.get(), .remove(): accept name instead of ID
This allows callers to just pass the name, and handle the conversion to ID and
path internally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 10:35:05 +02:00
Sebastiaan van Stijn 90f1238fb2
cli-plugins/manager: add IsPluginCommand(() utility
This makes it more convenient to check if a command is a plugin-stub

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 02:24:23 +02:00
Sebastiaan van Stijn a9ccb6552d
cli/compose: remove redundant reflection from tests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-29 15:35:31 +02:00
Sebastiaan van Stijn 9a493b1bf7
docker context rm: allow --force to ignore non-existing contexts
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>
2022-09-29 10:57:51 +02:00
Sebastiaan van Stijn 28b0aa9f1a
replace uses of deprecated env.Patch()
Also removing redundant defer for env.PatchAll(), which is now automatically
handled in t.Cleanup()

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-22 17:28:07 +02:00
Jason Hall 90db6b82a3
cli: use N/A as placeholder for old CREATED dates
Signed-off-by: Jason Hall <jason@chainguard.dev>
2022-09-19 12:08:10 -04:00
Jason Hall 0253634f59
cli: hide old CREATED times, instead of "52 years ago"
Signed-off-by: Jason Hall <jason@chainguard.dev>
2022-09-16 10:52:12 -04:00
Sebastiaan van Stijn 48da8a4b3b
Merge pull request #3764 from thaJeztah/cli_remove_intermediate_var
cli: createContainer() remove intermediate variable
2022-09-08 16:14:20 +02:00
Sebastiaan van Stijn e97d5d54c3
Merge pull request #3761 from thaJeztah/newcli_dedup
cli/command: NewDockerCli(): use WithStandardStreams()
2022-09-06 16:48:33 +02:00
Sebastiaan van Stijn f57200535b
Merge pull request #3766 from thaJeztah/bump_go_units
vendor: github.com/docker/go-units v0.5.0
2022-09-06 16:46:46 +02:00
Sebastiaan van Stijn 3dfdaa6458
linting: assigned to src, but reassigned without using the value (wastedassign)
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>
2022-09-03 21:25:45 +02:00
Sebastiaan van Stijn b58731fa46
linting: ST1005: error strings should not be capitalized (stylecheck)
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>
2022-09-03 21:25:42 +02:00
Sebastiaan van Stijn 7cec7101c4
linting: tabwriter: ignore stylecheck
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>
2022-09-03 21:25:39 +02:00
Sebastiaan van Stijn cef858170d
linting: os.Setenv() can be replaced by `t.Setenv()` (tenv)
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>
2022-09-03 21:25:37 +02:00