opts: deprecate QuotedString #6274
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]: moby/moby#29761 (comment) [moby@e4c1f07]: moby/moby@e4c1f07 [moby@abe32de]: moby/moby@abe32de [moby@0e9c40e]: moby/moby@0e9c40e [moby@c79a169]: moby/moby@c79a169 Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
added
impact/deprecation
status/2-code-review
kind/refactor
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
austinvazquez
approved these changes
Aug 15, 2025
This was referenced Aug 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
relates to:
opts: deprecate QuotedString
The
QuotedStringoption was added in moby@e4c1f07 and moby@abe32de to work around a regression in Docker 1.13 that causeddocker-machineto fail.docker-machineproduced 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'sconfigcommand, an exception was made for the--tls-xxxflags. From moby-29761:The
QuotedStringimplementation 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
QuotedStringoption and removes its use. A temporary, non-exported copy is added, but will be removed in the next release.- What I did
- How I did it
- How to verify it
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)