Commit Graph

9897 Commits

Author SHA1 Message Date
Sebastiaan van Stijn eae75092a0
cmd/docker: split handling exit-code to a separate utility
This allows dockerMain() to return an error "as usual", and puts the
responsibility for turning that into an appropriate exit-code in
main() (which also sets the exit-code when terminating).

We could consider putting this utility in the cli package and exporting
it if would be useful for doing a similar handling in plugins.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 11:02:22 +02:00
Sebastiaan van Stijn b7695d6c79
cli-plugins: RunPlugin(): rename error-variable that's possibly shadowed
The logic in this function is confusing; let's start make it obvious where
the error that is returned is produced,

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 11:01:41 +02:00
Sebastiaan van Stijn 350a0b68a9
cli-plugins: Run(): don't discard cli.StatusError errors without message
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 10:59:27 +02:00
Sebastiaan van Stijn 3dd6fc365d
cmd/docker: don't discard cli.StatusError errors without custom message
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 10:59:27 +02:00
Sebastiaan van Stijn 2f83064ec4
e2e/cli-plugins: check for exit-errors in tests
Verify that we get the expected exit-code, not just the message.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 10:59:26 +02:00
Sebastiaan van Stijn baf35da401
e2e/cli-plugins: use cmd.CombinedOutput() instead of custom buffer
Also remove a debug-log, as the output would already be shown if
the test would fail.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 10:59:26 +02:00
Sebastiaan van Stijn c6b40640cc
e2e/cli-plugins: use identifiable output for test
This confused me fore a bit, because I thought the test was checking for
an actual `context.Canceled` error (which is spelled "context canceled"
with a single "l". But then I found that this was a string that's printed
as part of a test-utility, just looking very similar but with the British
spelling ("cancelled").

Let's change this to a message that's unique for the test, also to make it
more grep'able.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 10:59:26 +02:00
Sebastiaan van Stijn e9f32edac5
e2e/cli-plugins: explicitly ignore fmt.Printxx errors
To keep some linters happier, and my IDE to be less noisy.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 10:59:26 +02:00
Sebastiaan van Stijn 5e7948ec83
e2e/cli-plugins: rename var that shadowed import
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-05 10:59:13 +02:00
Sebastiaan van Stijn cad08ff3b1
Merge pull request #5231 from thaJeztah/prettier_exit_status
cli: make cli.StatusError slightly prettier
2024-07-05 10:50:46 +02:00
Paweł Gronowski be6a415f86
Merge pull request #5230 from thaJeztah/clean_skip
cli/command/container: TestSplitCpArg: cleaner skip
2024-07-05 10:41:36 +02:00
Sebastiaan van Stijn bca2090061
cli: make cli.StatusError slightly prettier
This error didn't do a great job at formatting. If a StatusError was
produced without a Status message, it would print a very non-informative
error, with information missing.

Let's update the output:

- If a status-message is provided; print just that (after all the
  status code is something that can be found from the shell, e.g.
  through `echo $?` in Bash).
- If no status-message is provided: print a message more similar to
  Go's `exec.ExecError`, which uses `os.rocessState.String()` (see [1]).

Before this patch, an error without custom status would print:

    Status: , Code: 2

After this patch:

    exit status 2

In situations where a custom error-message is provided, the error-message
is print as-is, whereas before this patch, the message got combined with
the `Status:` and `Code:`, which resulted in some odd output.

Before this patch:

    docker volume --no-such-flag
    Status: unknown flag: --no-such-flag
    See 'docker volume --help'.

    Usage:  docker volume COMMAND

    Manage volumes

    Commands:
      create      Create a volume
      inspect     Display detailed information on one or more volumes
      ls          List volumes
      prune       Remove unused local volumes
      rm          Remove one or more volumes
      update      Update a volume (cluster volumes only)

    Run 'docker volume COMMAND --help' for more information on a command.
    , Code: 125

With this patch, the error is shown as-is;

    docker volume --no-such-flag
    unknown flag: --no-such-flag
    See 'docker volume --help'.

    Usage:  docker volume COMMAND

    Manage volumes

    Commands:
      create      Create a volume
      inspect     Display detailed information on one or more volumes
      ls          List volumes
      prune       Remove unused local volumes
      rm          Remove one or more volumes
      update      Update a volume (cluster volumes only)

    Run 'docker volume COMMAND --help' for more information on a command.

While the exit-code is no longer printed, it's still properly handled;

    echo $?
    125

[1]: 82c14346d8/src/os/exec_posix.go (L107-L135)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 22:08:18 +02:00
Sebastiaan van Stijn 88896eeaab
cli/command/container: TestSplitCpArg: cleaner skip
Trying to make the logic slightly clearer, and adding a custom
message for the skip,

Before this:

    === RUN   TestSplitCpArg/absolute_path_with_drive
        cp_test.go:184: tc.os == "windows" && runtime.GOOS != "windows" || tc.os == "linux" && runtime.GOOS == "windows"

After this:

    === RUN   TestSplitCpArg/absolute_path_with_drive
	cp_test.go:184: skipping windows test on non-windows platform

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 19:57:43 +02:00
Sebastiaan van Stijn 5aae44baaa
Merge pull request #5226 from thaJeztah/bump_engine_temp
vendor: github.com/docker/docker 508cc7c61226 (master)
2024-07-04 15:36:03 +02:00
Sebastiaan van Stijn b194274beb
replace uses of deprecated API types
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 15:22:18 +02:00
Sebastiaan van Stijn 4cac8efb56
vendor: github.com/docker/docker 508cc7c61226 (master)
full diff: https://github.com/docker/docker/v27.0.3..508cc7c6122651c4dfeeec2e626568704cfaf0f9

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 15:20:27 +02:00
Laura Brehm f5ce584ce0
Merge pull request #5223 from fredden/feature/completion/images
Enable completion for some 'image' sub commands
2024-07-04 11:15:23 +01:00
Sebastiaan van Stijn e99dfcd13e
Merge pull request #5224 from thaJeztah/test_spring_cleaning
test spring-cleaning
2024-07-04 12:14:00 +02:00
Paweł Gronowski 6abed4e3c4
Merge pull request #5225 from thaJeztah/network_cleanups
cli/command/network: some cleanup and pass smaller interfaces
2024-07-04 12:05:46 +02:00
Paweł Gronowski 404bf267f9
Merge pull request #5228 from thaJeztah/bump_bk_deps
vendor: update various dependencies
2024-07-04 11:10:05 +02:00
Sebastiaan van Stijn a0c4e56dea
vendor: golang.org/x/net v0.25.0
full diff: https://github.com/golang/net/compare/v0.24.0...v0.25.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 09:39:45 +02:00
Sebastiaan van Stijn 723130d7fe
vendor: golang.org/x/crypto v0.23.0
no changes in vendored code

full diff: https://github.com/golang/crypto/compare/v0.22.0...v0.23.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 09:38:44 +02:00
Sebastiaan van Stijn d33ef57dcb
vendor: golang.org/x/text v0.15.0
no changes in vendored files

full diff: https://github.com/golang/text/compare/v0.14.0...v0.15.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 09:38:04 +02:00
Sebastiaan van Stijn 21dbedd419
vendor: golang.org/x/sys v0.21.0
full diff: https://github.com/golang/sys/compare/v0.19.0...v0.21.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 09:32:57 +02:00
Sebastiaan van Stijn f8e7c0a0d6
vendor: github.com/klauspost/compress v1.17.9
full diff: https://github.com/klauspost/compress/compare/v1.17.4...v1.17.9

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 09:32:06 +02:00
David Karlsson 1dee86b4ba
Merge pull request #5002 from dvdksn/buildx_build_canonical
docs: make buildx build the canonical reference doc
2024-07-04 09:06:57 +02:00
Sebastiaan van Stijn f30d1ecc91
Merge pull request #5216 from Benehiko/force-lf
feat: force lf line endings by default
2024-07-04 02:09:34 +02:00
Sebastiaan van Stijn 10a015f871
cli/command/network: NewPruneCommand: explicitly ignore error
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 01:39:28 +02:00
Sebastiaan van Stijn b3d8809f42
cli/command/network: rewrite consolidateIpam to take an option-struct
Introduce a (non-exported) ipamOptions that collects all options for
creating a network.IPAM, so that this utility is more atomic (potentially
even could be moved to a separate package and exported).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 01:39:27 +02:00
Sebastiaan van Stijn ab230240ad
test spring-cleaning
This makes a quick pass through our tests;

Discard output/err
----------------------------------------------

Many tests were testing for error-conditions, but didn't discard output.
This produced a lot of noise when running the tests, and made it hard
to discover if there were actual failures, or if the output was expected.
For example:

    === RUN   TestConfigCreateErrors
    Error: "create" requires exactly 2 arguments.
    See 'create --help'.

    Usage:  create [OPTIONS] CONFIG file|- [flags]

    Create a config from a file or STDIN
    Error: "create" requires exactly 2 arguments.
    See 'create --help'.

    Usage:  create [OPTIONS] CONFIG file|- [flags]

    Create a config from a file or STDIN
    Error: error creating config
    --- PASS: TestConfigCreateErrors (0.00s)

And after discarding output:

    === RUN   TestConfigCreateErrors
    --- PASS: TestConfigCreateErrors (0.00s)

Use sub-tests where possible
----------------------------------------------

Some tests were already set-up to use test-tables, and even had a usable
name (or in some cases "error" to check for). Change them to actual sub-
tests. Same test as above, but now with sub-tests and output discarded:

    === RUN   TestConfigCreateErrors
    === RUN   TestConfigCreateErrors/requires_exactly_2_arguments
    === RUN   TestConfigCreateErrors/requires_exactly_2_arguments#01
    === RUN   TestConfigCreateErrors/error_creating_config
    --- PASS: TestConfigCreateErrors (0.00s)
        --- PASS: TestConfigCreateErrors/requires_exactly_2_arguments (0.00s)
        --- PASS: TestConfigCreateErrors/requires_exactly_2_arguments#01 (0.00s)
        --- PASS: TestConfigCreateErrors/error_creating_config (0.00s)
    PASS

It's not perfect in all cases (in the above, there's duplicate "expected"
errors, but Go conveniently adds "#01" for the duplicate). There's probably
also various tests I missed that could still use the same changes applied;
we can improve these in follow-ups.

Set cmd.Args to prevent test-failures
----------------------------------------------

When running tests from my IDE, it compiles the tests before running,
then executes the compiled binary to run the tests. Cobra doesn't like
that, because in that situation `os.Args` is taken as argument for the
command that's executed. The command that's tested now sees the test-
flags as arguments (`-test.v -test.run ..`), which causes various tests
to fail ("Command XYZ does not accept arguments").

    # compile the tests:
    go test -c -o foo.test

    # execute the test:
    ./foo.test -test.v -test.run TestFoo
    === RUN   TestFoo
    Error: "foo" accepts no arguments.

The Cobra maintainers ran into the same situation, and for their own
use have added a special case to ignore `os.Args` in these cases;
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L1078-L1083

    args := c.args

    // Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155
    if c.args == nil && filepath.Base(os.Args[0]) != "cobra.test" {
        args = os.Args[1:]
    }

Unfortunately, that exception is too specific (only checks for `cobra.test`),
so doesn't automatically fix the issue for other test-binaries. They did
provide a `cmd.SetArgs()` utility for this purpose
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L276-L280

    // SetArgs sets arguments for the command. It is set to os.Args[1:] by default, if desired, can be overridden
    // particularly useful when testing.
    func (c *Command) SetArgs(a []string) {
        c.args = a
    }

And the fix is to explicitly set the command's args to an empty slice to
prevent Cobra from falling back to using `os.Args[1:]` as arguments.

    cmd := newSomeThingCommand()
    cmd.SetArgs([]string{})

Some tests already take this issue into account, and I updated some tests
for this, but there's likely many other ones that can use the same treatment.

Perhaps the Cobra maintainers would accept a contribution to make their
condition less specific and to look for binaries ending with a `.test`
suffix (which is what compiled binaries usually are named as).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-04 01:35:12 +02:00
Dan Wallis c7d46aa7a1
Enable completion for 'image' sub commands
Signed-off-by: Dan Wallis <dan@wallis.nz>
2024-07-03 17:00:40 +01:00
Sebastiaan van Stijn 2eb61318b5
cli/command/network: some cleanup and pass smaller interfaces
Pass the appropriate API-client where possible instead of all of
DockerCLI, and some cleaning up.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 17:40:35 +02:00
Sebastiaan van Stijn 3837aa62d8
Merge pull request #5222 from thaJeztah/cleanup_for_engine_update
assorted minor changes in preparation of updating docker/docker dependency
2024-07-03 17:38:18 +02:00
Sebastiaan van Stijn b711372cab
cli/command/container: TestNewAttachCommandErrors: use struct-literals
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 17:09:41 +02:00
Sebastiaan van Stijn 229616e173
cli/command/image: fakeClient.ImagesPrune: fix unhandled err-return
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 17:09:41 +02:00
Sebastiaan van Stijn 42ba29395b
rename vars to prevent colliding with imports
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 17:09:41 +02:00
Sebastiaan van Stijn 3a77fdd91f
cli/command/trust: unconvert
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 17:09:40 +02:00
Sebastiaan van Stijn 26223f7017
cli/command/formatter: don't use unkeyed structs
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 17:09:40 +02:00
Paweł Gronowski bab48ebcc8
Merge pull request #5168 from Benehiko/fix-cli-login
fix: ctx cancellation on login prompt
2024-07-03 15:20:12 +02:00
Alano Terblanche c56f4a1ef7
workflow: remove git `autocrlf=false` setup on windows
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
2024-07-03 13:43:07 +02:00
Laura Brehm e2361a5ca8
Merge pull request #5218 from vvoland/update-go
update to go1.21.12
2024-07-03 11:03:40 +01:00
Paweł Gronowski d73d7d4ed3
update to go1.21.12
- https://github.com/golang/go/issues?q=milestone%3AGo1.21.12+label%3ACherryPickApproved
- full diff: https://github.com/golang/go/compare/go1.21.11...go1.21.12

These minor releases include 1 security fixes following the security policy:

net/http: denial of service due to improper 100-continue handling

The net/http HTTP/1.1 client mishandled the case where a server responds to a request with an "Expect: 100-continue" header with a non-informational (200 or higher) status. This mishandling could leave a client connection in an invalid state, where the next request sent on the connection will fail.

An attacker sending a request to a net/http/httputil.ReverseProxy proxy can exploit this mishandling to cause a denial of service by sending "Expect: 100-continue" requests which elicit a non-informational response from the backend. Each such request leaves the proxy with an invalid connection, and causes one subsequent request using that connection to fail.

Thanks to Geoff Franks for reporting this issue.

This is CVE-2024-24791 and Go issue https://go.dev/issue/67555.
View the release notes for more information:
https://go.dev/doc/devel/release#go1.21.12

**- Description for the changelog**

```markdown changelog
Update Go runtime to 1.21.12
```

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-07-03 10:59:37 +02:00
David Karlsson e91f0ded9c docs: make buildx build the canonical reference doc
Move common flag descriptions to the buildx build reference, and make
that page the canonical page in docs. Also rewrite some content in
image_build to make clear that this page is only for the legacy builder.

Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
2024-07-03 08:55:16 +02:00
Alano Terblanche c15ade0c64
fix: ctx cancellation on login prompt
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
2024-07-02 12:07:16 +02:00
Alano Terblanche 2bd4e95bf1
feat: force lf line endings by default
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
2024-07-02 11:53:42 +02:00
Paweł Gronowski 03409219af
Merge pull request #5213 from thaJeztah/vendor_sortorder_v1.1.0
vendor: github.com/fvbommel/sortorder v1.1.0
2024-07-01 17:20:24 +02:00
Sebastiaan van Stijn cd982b111a
Merge pull request #5212 from thaJeztah/fix_stack_sorting
cli/command/stack: fix faulty sort for sorting stacks
2024-07-01 15:08:04 +02:00
Paweł Gronowski ab2373a18e
Merge pull request #5207 from thaJeztah/bump_engine_27.0.3
vendor: github.com/docker/docker v27.0.3
2024-07-01 13:41:34 +02:00
Sebastiaan van Stijn be14edca2a
cli/command/stack: fix faulty sort for sorting stacks
This code was updated in 7b9580df51, which
removed support for using kubernetes as orchestrator, but in doing so
made this `sort.Slice` (probably) not do what it was expected to do ':)

    index 412cc2e5ee86..861ae1be2fb9 100644
    @@ -75,8 +54,7 @@ func format(dockerCli command.Cli, opts options.List, orchestrator command.Orche
        }
        sort.Slice(stacks, func(i, j int) bool {
            return sortorder.NaturalLess(stacks[i].Name, stacks[j].Name) ||
    -            !sortorder.NaturalLess(stacks[j].Name, stacks[i].Name) &&
    -            sortorder.NaturalLess(stacks[j].Namespace, stacks[i].Namespace)
    +            !sortorder.NaturalLess(stacks[j].Name, stacks[i].Name)
        })
        return formatter.StackWrite(stackCtx, stacks)
     }

The extra condition was added in 84241cc393
to support multiple namespaces. This patch removes it, bringing it back to
the state it was before that commit.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-01 13:19:37 +02:00
Sebastiaan van Stijn 0f6370c94b
vendor: github.com/fvbommel/sortorder v1.1.0
- Add a case-folding version of Natural sort order
  This can be used to perform case-insensitive comparisons and sorting.
  It's been placed in a separate sub-package because it requires the Unicode
  tables in the standard library, which can add significantly to binary size.

full diff: https://github.com/fvbommel/sortorder/compare/v1.0.2...v1.1.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-01 13:10:44 +02:00