Skip to content

Commit 31d6292

Browse files
cylithaJeztah
cyli
authored and
thaJeztah
committed
Propagate the provided external CA certificate to the external CA object
in swarm. Also, fix some CLI command confusions: 1. If the --external-ca flag is provided, require a --ca-cert flag as well, otherwise the external CA is set but the CA certificate is actually rotated to an internal cert 2. If a --ca-cert flag is provided, require a --ca-key or --external-ca flag be provided as well, otherwise either the server will say that the request is invalid, or if there was previously an external CA corresponding to the cert, it will succeed. While that works, it's better to require the user to explicitly set all the parameters of the new desired root CA. This also changes the `swarm update` function to set the external CA's CACert field, which while not strictly necessary, makes the CA list more explicit. Signed-off-by: Ying Li <[email protected]> (cherry picked from commit 4243440) Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent ebd85b1 commit 31d6292

5 files changed

Lines changed: 229 additions & 67 deletions

File tree

‎cli/command/swarm/ca.go‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,17 @@ func runCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error {
6868
return displayTrustRoot(dockerCli.Out(), swarmInspect)
6969
}
7070

71+
if flags.Changed(flagExternalCA) && len(opts.externalCA.Value()) > 0 && !flags.Changed(flagCACert) {
72+
return fmt.Errorf(
73+
"rotating to an external CA requires the `--%s` flag to specify the external CA's cert - "+
74+
"to add an external CA with the current root CA certificate, use the `update` command instead", flagCACert)
75+
}
76+
77+
if flags.Changed(flagCACert) && len(opts.externalCA.Value()) == 0 && !flags.Changed(flagCAKey) {
78+
return fmt.Errorf("the --%s flag requires that a --%s flag and/or --%s flag be provided as well",
79+
flagCACert, flagCAKey, flagExternalCA)
80+
}
81+
7182
updateSwarmSpec(&swarmInspect.Spec, flags, opts)
7283
if err := client.SwarmUpdate(ctx, swarmInspect.Version, swarmInspect.Spec, swarm.UpdateFlags{}); err != nil {
7384
return err
@@ -80,20 +91,15 @@ func runCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error {
8091
}
8192

8293
func updateSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, opts caOptions) {
83-
opts.mergeSwarmSpecCAFlags(spec, flags)
8494
caCert := opts.rootCACert.Contents()
8595
caKey := opts.rootCAKey.Contents()
96+
opts.mergeSwarmSpecCAFlags(spec, flags, caCert)
97+
98+
spec.CAConfig.SigningCACert = caCert
99+
spec.CAConfig.SigningCAKey = caKey
86100

87-
if caCert != "" {
88-
spec.CAConfig.SigningCACert = caCert
89-
}
90-
if caKey != "" {
91-
spec.CAConfig.SigningCAKey = caKey
92-
}
93101
if caKey == "" && caCert == "" {
94102
spec.CAConfig.ForceRotate++
95-
spec.CAConfig.SigningCACert = ""
96-
spec.CAConfig.SigningCAKey = ""
97103
}
98104
}
99105

‎cli/command/swarm/ca_test.go‎

Lines changed: 198 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,28 @@ import (
1313
is "gotest.tools/assert/cmp"
1414
)
1515

16+
const (
17+
cert = `
18+
-----BEGIN CERTIFICATE-----
19+
MIIBuDCCAV4CCQDOqUYOWdqMdjAKBggqhkjOPQQDAzBjMQswCQYDVQQGEwJVUzEL
20+
MAkGA1UECAwCQ0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRv
21+
Y2tlcjEPMA0GA1UECwwGRG9ja2VyMQ0wCwYDVQQDDARUZXN0MCAXDTE4MDcwMjIx
22+
MjkxOFoYDzMwMTcxMTAyMjEyOTE4WjBjMQswCQYDVQQGEwJVUzELMAkGA1UECAwC
23+
Q0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRvY2tlcjEPMA0G
24+
A1UECwwGRG9ja2VyMQ0wCwYDVQQDDARUZXN0MFkwEwYHKoZIzj0CAQYIKoZIzj0D
25+
AQcDQgAEgvvZl5Vqpr1e+g5IhoU6TZHgRau+BZETVFTmqyWYajA/mooRQ1MZTozu
26+
s9ZZZA8tzUhIqS36gsFuyIZ4YiAlyjAKBggqhkjOPQQDAwNIADBFAiBQ7pCPQrj8
27+
8zaItMf0pk8j1NU5XrFqFEZICzvjzUJQBAIhAKq2gFwoTn8KH+cAAXZpAGJPmOsT
28+
zsBT8gBAOHhNA6/2
29+
-----END CERTIFICATE-----`
30+
key = `
31+
-----BEGIN EC PRIVATE KEY-----
32+
MHcCAQEEICyheZpw70pbgO4hEuwhZTETWyTpNJmJ3TyFaWT6WTRkoAoGCCqGSM49
33+
AwEHoUQDQgAEgvvZl5Vqpr1e+g5IhoU6TZHgRau+BZETVFTmqyWYajA/mooRQ1MZ
34+
Tozus9ZZZA8tzUhIqS36gsFuyIZ4YiAlyg==
35+
-----END EC PRIVATE KEY-----`
36+
)
37+
1638
func swarmSpecWithFullCAConfig() *swarm.Spec {
1739
return &swarm.Spec{
1840
CAConfig: swarm.CAConfig{
@@ -37,51 +59,79 @@ func TestDisplayTrustRootNoRoot(t *testing.T) {
3759
assert.Error(t, err, "No CA information available")
3860
}
3961

62+
type invalidCATestCases struct {
63+
args []string
64+
errorMsg string
65+
}
66+
67+
func writeFile(data string) (string, error) {
68+
tmpfile, err := ioutil.TempFile("", "testfile")
69+
if err != nil {
70+
return "", err
71+
}
72+
_, err = tmpfile.Write([]byte(data))
73+
if err != nil {
74+
return "", err
75+
}
76+
tmpfile.Close()
77+
return tmpfile.Name(), nil
78+
}
79+
4080
func TestDisplayTrustRootInvalidFlags(t *testing.T) {
4181
// we need an actual PEMfile to test
42-
tmpfile, err := ioutil.TempFile("", "pemfile")
82+
tmpfile, err := writeFile(cert)
4383
assert.NilError(t, err)
44-
defer os.Remove(tmpfile.Name())
45-
tmpfile.Write([]byte(`
46-
-----BEGIN CERTIFICATE-----
47-
MIIBajCCARCgAwIBAgIUe0+jYWhxN8fFOByC7yveIYgvx1kwCgYIKoZIzj0EAwIw
48-
EzERMA8GA1UEAxMIc3dhcm0tY2EwHhcNMTcwNjI3MTUxNDAwWhcNMzcwNjIyMTUx
49-
NDAwWjATMREwDwYDVQQDEwhzd2FybS1jYTBZMBMGByqGSM49AgEGCCqGSM49AwEH
50-
A0IABGgbOZLd7b4b262+6m4ignIecbAZKim6djNiIS1Kl5IHciXYn7gnSpsayjn7
51-
GQABpgkdPeM9TEQowmtR1qSnORujQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMB
52-
Af8EBTADAQH/MB0GA1UdDgQWBBQ6Rtcn823/fxRZyheRDFpDzuBMpTAKBggqhkjO
53-
PQQDAgNIADBFAiEAqD3Kb2rgsy6NoTk+zEgcUi/aGBCsvQDG3vML1PXN8j0CIBjj
54-
4nDj+GmHXcnKa8wXx70Z8OZEpRQIiKDDLmcXuslp
55-
-----END CERTIFICATE-----
56-
`))
57-
tmpfile.Close()
84+
defer os.Remove(tmpfile)
5885

59-
errorTestCases := [][]string{
86+
errorTestCases := []invalidCATestCases{
6087
{
61-
"--ca-cert=" + tmpfile.Name(),
88+
args: []string{"--ca-cert=" + tmpfile},
89+
errorMsg: "flag requires the `--rotate` flag to update the CA",
6290
},
6391
{
64-
"--ca-key=" + tmpfile.Name(),
92+
args: []string{"--ca-key=" + tmpfile},
93+
errorMsg: "flag requires the `--rotate` flag to update the CA",
6594
},
6695
{ // to make sure we're not erroring because we didn't provide a CA key along with the CA cert
67-
68-
"--ca-cert=" + tmpfile.Name(),
69-
"--ca-key=" + tmpfile.Name(),
96+
args: []string{
97+
"--ca-cert=" + tmpfile,
98+
"--ca-key=" + tmpfile,
99+
},
100+
errorMsg: "flag requires the `--rotate` flag to update the CA",
70101
},
71102
{
72-
"--cert-expiry=2160h0m0s",
103+
args: []string{"--cert-expiry=2160h0m0s"},
104+
errorMsg: "flag requires the `--rotate` flag to update the CA",
73105
},
74106
{
75-
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
107+
args: []string{"--external-ca=protocol=cfssl,url=https://some.com/https/url"},
108+
errorMsg: "flag requires the `--rotate` flag to update the CA",
76109
},
77110
{ // to make sure we're not erroring because we didn't provide a CA cert and external CA
78-
79-
"--ca-cert=" + tmpfile.Name(),
80-
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
111+
args: []string{
112+
"--ca-cert=" + tmpfile,
113+
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
114+
},
115+
errorMsg: "flag requires the `--rotate` flag to update the CA",
116+
},
117+
{
118+
args: []string{
119+
"--rotate",
120+
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
121+
},
122+
errorMsg: "rotating to an external CA requires the `--ca-cert` flag to specify the external CA's cert - " +
123+
"to add an external CA with the current root CA certificate, use the `update` command instead",
124+
},
125+
{
126+
args: []string{
127+
"--rotate",
128+
"--ca-cert=" + tmpfile,
129+
},
130+
errorMsg: "the --ca-cert flag requires that a --ca-key flag and/or --external-ca flag be provided as well",
81131
},
82132
}
83133

84-
for _, args := range errorTestCases {
134+
for _, testCase := range errorTestCases {
85135
cmd := newCACommand(
86136
test.NewFakeCli(&fakeClient{
87137
swarmInspectFunc: func() (swarm.Swarm, error) {
@@ -94,9 +144,9 @@ PQQDAgNIADBFAiEAqD3Kb2rgsy6NoTk+zEgcUi/aGBCsvQDG3vML1PXN8j0CIBjj
94144
}, nil
95145
},
96146
}))
97-
assert.Check(t, cmd.Flags().Parse(args))
147+
assert.Check(t, cmd.Flags().Parse(testCase.args))
98148
cmd.SetOutput(ioutil.Discard)
99-
assert.ErrorContains(t, cmd.Execute(), "flag requires the `--rotate` flag to update the CA")
149+
assert.ErrorContains(t, cmd.Execute(), testCase.errorMsg)
100150
}
101151
}
102152

@@ -112,43 +162,139 @@ func TestDisplayTrustRoot(t *testing.T) {
112162
assert.Check(t, is.Equal(trustRoot+"\n", buffer.String()))
113163
}
114164

165+
type swarmUpdateRecorder struct {
166+
spec swarm.Spec
167+
}
168+
169+
func (s *swarmUpdateRecorder) swarmUpdate(sp swarm.Spec, _ swarm.UpdateFlags) error {
170+
s.spec = sp
171+
return nil
172+
}
173+
174+
func swarmInspectFuncWithFullCAConfig() (swarm.Swarm, error) {
175+
return swarm.Swarm{
176+
ClusterInfo: swarm.ClusterInfo{
177+
Spec: *swarmSpecWithFullCAConfig(),
178+
},
179+
}, nil
180+
}
181+
115182
func TestUpdateSwarmSpecDefaultRotate(t *testing.T) {
116-
spec := swarmSpecWithFullCAConfig()
117-
flags := newCACommand(nil).Flags()
118-
updateSwarmSpec(spec, flags, caOptions{})
183+
s := &swarmUpdateRecorder{}
184+
cli := test.NewFakeCli(&fakeClient{
185+
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
186+
swarmUpdateFunc: s.swarmUpdate,
187+
})
188+
cmd := newCACommand(cli)
189+
cmd.SetArgs([]string{"--rotate", "--detach"})
190+
cmd.SetOutput(cli.OutBuffer())
191+
assert.NilError(t, cmd.Execute())
119192

120193
expected := swarmSpecWithFullCAConfig()
121194
expected.CAConfig.ForceRotate = 2
122195
expected.CAConfig.SigningCACert = ""
123196
expected.CAConfig.SigningCAKey = ""
124-
assert.Check(t, is.DeepEqual(expected, spec))
197+
assert.Check(t, is.DeepEqual(*expected, s.spec))
125198
}
126199

127-
func TestUpdateSwarmSpecPartial(t *testing.T) {
128-
spec := swarmSpecWithFullCAConfig()
129-
flags := newCACommand(nil).Flags()
130-
updateSwarmSpec(spec, flags, caOptions{
131-
rootCACert: PEMFile{contents: "cacert"},
200+
func TestUpdateSwarmSpecCertAndKey(t *testing.T) {
201+
certfile, err := writeFile(cert)
202+
assert.NilError(t, err)
203+
defer os.Remove(certfile)
204+
205+
keyfile, err := writeFile(key)
206+
assert.NilError(t, err)
207+
defer os.Remove(keyfile)
208+
209+
s := &swarmUpdateRecorder{}
210+
cli := test.NewFakeCli(&fakeClient{
211+
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
212+
swarmUpdateFunc: s.swarmUpdate,
132213
})
214+
cmd := newCACommand(cli)
215+
cmd.SetArgs([]string{
216+
"--rotate",
217+
"--detach",
218+
"--ca-cert=" + certfile,
219+
"--ca-key=" + keyfile,
220+
"--cert-expiry=3m"})
221+
cmd.SetOutput(cli.OutBuffer())
222+
assert.NilError(t, cmd.Execute())
133223

134224
expected := swarmSpecWithFullCAConfig()
135-
expected.CAConfig.SigningCACert = "cacert"
136-
assert.Check(t, is.DeepEqual(expected, spec))
225+
expected.CAConfig.SigningCACert = cert
226+
expected.CAConfig.SigningCAKey = key
227+
expected.CAConfig.NodeCertExpiry = 3 * time.Minute
228+
assert.Check(t, is.DeepEqual(*expected, s.spec))
137229
}
138230

139-
func TestUpdateSwarmSpecFullFlags(t *testing.T) {
140-
flags := newCACommand(nil).Flags()
141-
flags.Lookup(flagCertExpiry).Changed = true
142-
spec := swarmSpecWithFullCAConfig()
143-
updateSwarmSpec(spec, flags, caOptions{
144-
rootCACert: PEMFile{contents: "cacert"},
145-
rootCAKey: PEMFile{contents: "cakey"},
146-
swarmCAOptions: swarmCAOptions{nodeCertExpiry: 3 * time.Minute},
231+
func TestUpdateSwarmSpecCertAndExternalCA(t *testing.T) {
232+
certfile, err := writeFile(cert)
233+
assert.NilError(t, err)
234+
defer os.Remove(certfile)
235+
236+
s := &swarmUpdateRecorder{}
237+
cli := test.NewFakeCli(&fakeClient{
238+
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
239+
swarmUpdateFunc: s.swarmUpdate,
147240
})
241+
cmd := newCACommand(cli)
242+
cmd.SetArgs([]string{
243+
"--rotate",
244+
"--detach",
245+
"--ca-cert=" + certfile,
246+
"--external-ca=protocol=cfssl,url=https://some.external.ca"})
247+
cmd.SetOutput(cli.OutBuffer())
248+
assert.NilError(t, cmd.Execute())
148249

149250
expected := swarmSpecWithFullCAConfig()
150-
expected.CAConfig.SigningCACert = "cacert"
151-
expected.CAConfig.SigningCAKey = "cakey"
152-
expected.CAConfig.NodeCertExpiry = 3 * time.Minute
153-
assert.Check(t, is.DeepEqual(expected, spec))
251+
expected.CAConfig.SigningCACert = cert
252+
expected.CAConfig.SigningCAKey = ""
253+
expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{
254+
{
255+
Protocol: swarm.ExternalCAProtocolCFSSL,
256+
URL: "https://some.external.ca",
257+
CACert: cert,
258+
Options: make(map[string]string),
259+
},
260+
}
261+
assert.Check(t, is.DeepEqual(*expected, s.spec))
262+
}
263+
264+
func TestUpdateSwarmSpecCertAndKeyAndExternalCA(t *testing.T) {
265+
certfile, err := writeFile(cert)
266+
assert.NilError(t, err)
267+
defer os.Remove(certfile)
268+
269+
keyfile, err := writeFile(key)
270+
assert.NilError(t, err)
271+
defer os.Remove(keyfile)
272+
273+
s := &swarmUpdateRecorder{}
274+
cli := test.NewFakeCli(&fakeClient{
275+
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
276+
swarmUpdateFunc: s.swarmUpdate,
277+
})
278+
cmd := newCACommand(cli)
279+
cmd.SetArgs([]string{
280+
"--rotate",
281+
"--detach",
282+
"--ca-cert=" + certfile,
283+
"--ca-key=" + keyfile,
284+
"--external-ca=protocol=cfssl,url=https://some.external.ca"})
285+
cmd.SetOutput(cli.OutBuffer())
286+
assert.NilError(t, cmd.Execute())
287+
288+
expected := swarmSpecWithFullCAConfig()
289+
expected.CAConfig.SigningCACert = cert
290+
expected.CAConfig.SigningCAKey = key
291+
expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{
292+
{
293+
Protocol: swarm.ExternalCAProtocolCFSSL,
294+
URL: "https://some.external.ca",
295+
CACert: cert,
296+
Options: make(map[string]string),
297+
},
298+
}
299+
assert.Check(t, is.DeepEqual(*expected, s.spec))
154300
}

‎cli/command/swarm/opts.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func addSwarmFlags(flags *pflag.FlagSet, opts *swarmOptions) {
230230
addSwarmCAFlags(flags, &opts.swarmCAOptions)
231231
}
232232

233-
func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet) {
233+
func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) {
234234
if flags.Changed(flagTaskHistoryLimit) {
235235
spec.Orchestration.TaskHistoryRetentionLimit = &opts.taskHistoryLimit
236236
}
@@ -246,25 +246,28 @@ func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet)
246246
if flags.Changed(flagAutolock) {
247247
spec.EncryptionConfig.AutoLockManagers = opts.autolock
248248
}
249-
opts.mergeSwarmSpecCAFlags(spec, flags)
249+
opts.mergeSwarmSpecCAFlags(spec, flags, caCert)
250250
}
251251

252252
type swarmCAOptions struct {
253253
nodeCertExpiry time.Duration
254254
externalCA ExternalCAOption
255255
}
256256

257-
func (opts *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet) {
257+
func (opts *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) {
258258
if flags.Changed(flagCertExpiry) {
259259
spec.CAConfig.NodeCertExpiry = opts.nodeCertExpiry
260260
}
261261
if flags.Changed(flagExternalCA) {
262262
spec.CAConfig.ExternalCAs = opts.externalCA.Value()
263+
for _, ca := range spec.CAConfig.ExternalCAs {
264+
ca.CACert = caCert
265+
}
263266
}
264267
}
265268

266269
func (opts *swarmOptions) ToSpec(flags *pflag.FlagSet) swarm.Spec {
267270
var spec swarm.Spec
268-
opts.mergeSwarmSpec(&spec, flags)
271+
opts.mergeSwarmSpec(&spec, flags, "")
269272
return spec
270273
}

0 commit comments

Comments
 (0)