This splits the validation code from parsing code, potentially allowing
us to either fully deferring it to the daemon, or to perform validation
separately.
For reference; daemon-side validation currently (docker 29.2.0) produces;
docker run --rm --mount type=bind,src=/var/run,target=/foo,bind-recursive=writable alpine
docker: Error response from daemon: mount options conflict: !ReadOnly && BindOptions.ReadOnlyNonRecursive
docker run --rm --mount type=bind,src=/var/run,target=/foo,bind-recursive=readonly alpine
docker: Error response from daemon: mount options conflict: !ReadOnly && BindOptions.ReadOnlyForceRecursive
Validation for BindOptions.Propagation is currently missing on the daemon;
docker run --rm --mount type=bind,src=/var/run,target=/foo,bind-recursive=readonly,readonly alpine
# no error
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The daemon already validates the target, so we don't have to validate
if a target is set. Instead, we can ignore empty targets, but produce
an error if a target option was set, but set to an empty value.
With this patch applied, omitting a target option is ignored by the CLI,
but still invalidated by the daemon if the given mount-type requires a
mount target;
docker run --rm --mount type=bind,src=/var/run/docker.sock alpine
docker: Error response from daemon: invalid mount config for type "bind": field Target must not be empty
docker run --rm --mount type=bind,src=/var/run/docker.sock,dst=../foo alpine
docker: Error response from daemon: invalid mount config for type "bind": invalid mount path: '../foo' mount path must be absolute
When passing a target option (`target`, `dst`, or `destination`), the
CLI produces an error if the value is empty;
docker run --rm --mount type=bind,src=/var/run/docker.sock,dst= alpine
invalid argument "type=bind,src=/var/run/docker.sock,dst=" for "--mount" flag: invalid value for 'dst': mount target must be a non-empty value
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this patch:
docker run --rm --mount "" busybox
invalid argument "" for "--mount" flag: EOF
With this patch:
docker run --rm --mount "" busybox
invalid argument "" for "--mount" flag: value is empty
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Setting the source and target paths is not tied to the mount-type,
so these tests where covering the same code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was introduced in 9c10a9c9ac, which added
use of the network.ParsePortRange.All method, which uses an iterator and
requires go1.23;
opts/swarmopts/port.go:172:18: cannot range over pr.All() (value of func type iter.Seq[network.Port]): requires go1.23 or later (-lang was set to go1.16; check go.mod)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
follow-up to f81816ef88
We could decide to not initialize a default, and do this only when
setting a value, but this may require more changes, so doing this
as a starting-point.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These were deprecated in 6f0c66c152 and are
no longer used.
This removes the deprecated:
- `NewNamedListOptsRef`
- `NewNamedMapOpts`
- `NamedListOpts`
- `NamedMapOpts`
- `NamedOption`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `NewNamedListOptsRef`, `NewNamedMapOpts` and related `NamedListOpts`,
`NamedMapOpts`, and `NamedOption` interface were added in [moby@677a6b3],
which added support for a `daemon.json` configuration file. That change
required a way to correlate command-line flags with their corresponding
fields in the `daemon.json` to detect conflicting options. At the time,
the CLI and daemon were produced from the same code, and shared packages
for command-line options, but when the CLI was moved to a separate
repository, these options were inherited.
[moby@677a6b3]: https://github.com/moby/moby/commit/677a6b3506107468ed8c00331991afd9176fa0b9
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `QuotedString` option was added in [moby@e4c1f07] and [moby@abe32de]
to work around a regression in Docker 1.13 that caused `docker-machine`
to fail. `docker-machine` produced instructions on how to set up a cli
to connect to the Machine it produced. These instructions used quotes
around the paths for TLS certificates, but with an `=` for the flag's
values instead of a space; due to this the shell would not handle
stripping quotes, so the CLI would now get the value including quotes.
Preserving quotes in such cases is expected (and standard behavior), but
versions of Docker before 1.13 used a custom "mflag" package for flag
parsing, and that package contained custom handling for quotes (added
in [moby@0e9c40e]).
For other flags, this problem could be solved by the user, but as these
instructions were produced by `docker-machine`'s `config` command, an
exception was made for the `--tls-xxx` flags. From [moby-29761]:
> The flag trimming behaviour is really unusual, and I would say unexpected.
> I think removing it is generally the right idea. Since we have one very
> common case where it's necessary for backwards compatibility we need to
> add a special case, but I don't think we should apply that case to every
> flag.
The `QuotedString` implementation has various limitations, as it doesn't
follow the same handling of quotes as a shell would.
Given that Docker Machine reached EOL a long time ago and other options,
such as `docker context`, have been added to configure the CLI to connect
to a specific host (with corresponding TLS configuration), we should remove
the special handling for these flags, as it's inconsitent with all other
flags, and not worth maintaining for a tool that no longer exists.
This patch deprecates the `QuotedString` option and removes its use. A
temporary, non-exported copy is added, but will be removed in the next
release.
[moby-29761]: https://github.com/moby/moby/issues/29761#issuecomment-270211265
[moby@e4c1f07]: https://github.com/moby/moby/commit/e4c1f0772923c3069ce14a82d445cd55af3382bc
[moby@abe32de]: https://github.com/moby/moby/commit/abe32de6b46825300f612864e6b4c98606a5bb0e
[moby@0e9c40e]: https://github.com/moby/moby/commit/0e9c40eb8243fa437bc6c3e93aaff64a10cb856e
[moby@c79a169]: https://github.com/moby/moby/commit/c79a169a35f8ee0ecb66cfcffab4b0bd2c77f996
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- deprecate sockets.GetProxyEnv, sockets.DialerFromEnvironment
- add support for unix sockets on Windows
- remove legacy CBC cipher suites from client config
- align client and server defaults to be the same.
- remove support for encrypted TLS private keys.
- nat: optimize ParsePortSpec
full diff: https://github.com/docker/go-connections/compare/v0.5.0...v0.6.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Use strong-typed switches for validating options
- Initialize defaults instead of setting them after
parsing the ports. Each option should be validated
as part of the parsing, so no invalid (or empty)
values should be set.
- Put variables closer to where they're used, and
pre-allocate slices.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- use consistent name for MountOpt vars
- cleanup some comments and make them a GoDoc
- remove import alias
- use subtests for tests that were prepared for it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `GetSlice()` function is part of cobra's [cobra.SliceValue] interface,
and duplicates the older `GetAll()` method. This patch deprecates the
`GetAll()` method in favor of `GetSlice()`.
[cobra.SliceValue]: https://pkg.go.dev/github.com/spf13/cobra@v1.9.1#SliceValue
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Cobra's shell completion has specific rules to decide whether a flag can
be accepted multiple times. If a flag does not meet that rule, it only
completes the flag name once; some of those rules depend on the "type"
of the option to end with "Array" or "Slice", which most of our options
don't.
Starting with Cobra 1.9, it also checks whether an option implements
the [cobra.SliceValue] interface (see [spf13/cobra 2210]).
This patch implements the [cobra.SliceValue] interface on ListOpts, so
that these options can be completed multiple times.
In a follow-up, we can update our code to replace our uses of `GetAll()`,
which is identical with the `GetSlice()` method, and potentially deprecate
the old method.
Before this patch, ListOpts would only be completed once when completing
flag names. For example, the following would show the `--label` flag the
first time, but omit it if a `--label` flag was already set;
docker run--l<TAB>
--label (Set meta data on a container) --link-local-ip (Container IPv4/IPv6 link-local addresses)
--label-file (Read in a line delimited file of labels) --log-driver (Logging driver for the container)
--link (Add link to another container) --log-opt (Log driver options)
docker run --label hello --l<TAB>
--label-file (Read in a line delimited file of labels) --link-local-ip (Container IPv4/IPv6 link-local addresses) --log-opt (Log driver options)
--link (Add link to another container) --log-driver (Logging driver for the container)
With this patch, the completion script correctly identifies the `--label`
flag to be accepted multiple times, and also completes it when already
set;
docker run --label hello --l<TAB>
--label (Set meta data on a container) --link-local-ip (Container IPv4/IPv6 link-local addresses)
--label-file (Read in a line delimited file of labels) --log-driver (Logging driver for the container)
--link (Add link to another container) --log-opt (Log driver options)
[cobra.SliceValue]: https://pkg.go.dev/github.com/spf13/cobra@v1.9.1#SliceValue
[spf13/cobra 2210]: https://github.com/spf13/cobra/pull/2210
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This prevents users of the CLI that don't implement swarm-related
features from depending on the swarm API types.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The errors.Wrap and errors.Wrapf functions gracefully handle nil-errors.
This allows them to be used unconditionally regardless if an error
was produced.
While this can be convenient, it can also be err-prone, as replacing
these with stdlib errors means they unconditionally produce an error.
This patch replaces code uses of errors.Wrap to be gated by a check
for nil-errors to future-proof our code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
opts/port.go:124:7: unused-receiver: method receiver 'p' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (p *PortOpt) Type() string {
^
opts/mount.go:218:7: unused-receiver: method receiver 'm' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (m *MountOpt) Type() string {
^
opts/quotedstring.go:16:7: unused-receiver: method receiver 's' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (s *QuotedString) Type() string {
^
opts/secret.go:82:7: unused-receiver: method receiver 'o' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (o *SecretOpt) Type() string {
^
opts/opts_test.go:235: line-length-limit: line is 283 characters, out of limit 200 (revive)
`foo.bar.baz.this.should.fail.on.long.name.because.it.is.longer.thanisshouldbethis.should.fail.on.long.name.because.it.is.longer.thanisshouldbethis.should.fail.on.long.name.because.it.is.longer.thanisshouldbethis.should.fail.on.long.name.because.it.is.longer.thanisshouldbe`,
opts/ulimit.go:61:7: unused-receiver: method receiver 'o' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (o *UlimitOpt) Type() string {
^
opts/weightdevice.go:82:7: unused-receiver: method receiver 'opt' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (opt *WeightdeviceOpt) Type() string {
^
opts/throttledevice.go:103:7: unused-receiver: method receiver 'opt' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (opt *ThrottledeviceOpt) Type() string {
^
opts/duration.go:49:7: unused-receiver: method receiver 'd' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (d *DurationOpt) Type() string {
^
opts/network.go:109:7: unused-receiver: method receiver 'n' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (n *NetworkOpt) Type() string {
^
opts/network.go:119:7: unused-receiver: method receiver 'n' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (n *NetworkOpt) String() string {
^
opts/opts.go:113:7: unused-receiver: method receiver 'opts' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (opts *ListOpts) Type() string {
^
opts/pull_behavior.go:13:7: unused-receiver: method receiver 'p' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (p *PullOpt) Type() string {
^
opts/config.go:83:7: unused-receiver: method receiver 'o' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (o *ConfigOpt) Type() string {
^
opts/gpus.go:95:7: unused-receiver: method receiver 'o' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (o *GpuOpts) Type() string {
^
opts/pull_behavior.go:23:7: unused-receiver: method receiver 'p' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (p *PullOpt) IsBoolFlag() bool {
^
opts/opts.go:183:7: unused-receiver: method receiver 'opts' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (opts *MapOpts) Type() string {
^
opts/opts.go:361:7: unused-receiver: method receiver 'o' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (o *FilterOpt) Type() string {
^
opts/opts.go:389:7: unused-receiver: method receiver 'c' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (c *NanoCPUs) Type() string {
^
opts/opts.go:466:7: unused-receiver: method receiver 'm' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (m *MemBytes) Type() string {
^
opts/opts.go:501:7: unused-receiver: method receiver 'm' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (m *MemSwapBytes) Type() string {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
