Skip to content

Commit 187a942

Browse files
committed
opts: deprecate QuotedString
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]>
1 parent 53b6fdd commit 187a942

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

cli/flags/options.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ func (o *ClientOptions) InstallFlags(flags *pflag.FlagSet) {
9090
KeyFile: filepath.Join(dockerCertPath, DefaultKeyFile),
9191
}
9292
tlsOptions := o.TLSOptions
93-
flags.Var(opts.NewQuotedString(&tlsOptions.CAFile), "tlscacert", "Trust certs signed only by this CA")
94-
flags.Var(opts.NewQuotedString(&tlsOptions.CertFile), "tlscert", "Path to TLS certificate file")
95-
flags.Var(opts.NewQuotedString(&tlsOptions.KeyFile), "tlskey", "Path to TLS key file")
93+
flags.Var(&quotedString{&tlsOptions.CAFile}, "tlscacert", "Trust certs signed only by this CA")
94+
flags.Var(&quotedString{&tlsOptions.CertFile}, "tlscert", "Path to TLS certificate file")
95+
flags.Var(&quotedString{&tlsOptions.KeyFile}, "tlskey", "Path to TLS key file")
9696

9797
// opts.ValidateHost is not used here, so as to allow connection helpers
9898
hostOpt := opts.NewNamedListOptsRef("hosts", &o.Hosts, nil)
@@ -146,3 +146,33 @@ func SetLogLevel(logLevel string) {
146146
logrus.SetLevel(logrus.InfoLevel)
147147
}
148148
}
149+
150+
type quotedString struct {
151+
value *string
152+
}
153+
154+
func (s *quotedString) Set(val string) error {
155+
*s.value = trimQuotes(val)
156+
return nil
157+
}
158+
159+
func (*quotedString) Type() string {
160+
return "string"
161+
}
162+
163+
func (s *quotedString) String() string {
164+
return *s.value
165+
}
166+
167+
func trimQuotes(value string) string {
168+
if len(value) < 2 {
169+
return value
170+
}
171+
lastIndex := len(value) - 1
172+
for _, char := range []byte{'\'', '"'} {
173+
if value[0] == char && value[lastIndex] == char {
174+
return value[1:lastIndex]
175+
}
176+
}
177+
return value
178+
}

opts/quotedstring.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package opts
22

33
// QuotedString is a string that may have extra quotes around the value. The
44
// quotes are stripped from the value.
5+
//
6+
// Deprecated: This option type is no longer used and will be removed in the next release.
57
type QuotedString struct {
68
value *string
79
}
@@ -35,6 +37,8 @@ func trimQuotes(value string) string {
3537
}
3638

3739
// NewQuotedString returns a new quoted string option
40+
//
41+
// Deprecated: This option type is no longer used and will be removed in the next release.
3842
func NewQuotedString(value *string) *QuotedString {
3943
return &QuotedString{value: value}
4044
}

0 commit comments

Comments
 (0)