diff --git a/cli/command/swarm/ca.go b/cli/command/swarm/ca.go index 1aa619ecf357..961e7e89441b 100644 --- a/cli/command/swarm/ca.go +++ b/cli/command/swarm/ca.go @@ -68,6 +68,17 @@ func runCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error { return displayTrustRoot(dockerCli.Out(), swarmInspect) } + if flags.Changed(flagExternalCA) && len(opts.externalCA.Value()) > 0 && !flags.Changed(flagCACert) { + return fmt.Errorf( + "rotating to an external CA requires the `--%s` flag to specify the external CA's cert - "+ + "to add an external CA with the current root CA certificate, use the `update` command instead", flagCACert) + } + + if flags.Changed(flagCACert) && len(opts.externalCA.Value()) == 0 && !flags.Changed(flagCAKey) { + return fmt.Errorf("the --%s flag requires that a --%s flag and/or --%s flag be provided as well", + flagCACert, flagCAKey, flagExternalCA) + } + updateSwarmSpec(&swarmInspect.Spec, flags, opts) if err := client.SwarmUpdate(ctx, swarmInspect.Version, swarmInspect.Spec, swarm.UpdateFlags{}); err != nil { return err @@ -80,20 +91,15 @@ func runCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error { } func updateSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, opts caOptions) { - opts.mergeSwarmSpecCAFlags(spec, flags) caCert := opts.rootCACert.Contents() caKey := opts.rootCAKey.Contents() + opts.mergeSwarmSpecCAFlags(spec, flags, caCert) + + spec.CAConfig.SigningCACert = caCert + spec.CAConfig.SigningCAKey = caKey - if caCert != "" { - spec.CAConfig.SigningCACert = caCert - } - if caKey != "" { - spec.CAConfig.SigningCAKey = caKey - } if caKey == "" && caCert == "" { spec.CAConfig.ForceRotate++ - spec.CAConfig.SigningCACert = "" - spec.CAConfig.SigningCAKey = "" } } diff --git a/cli/command/swarm/ca_test.go b/cli/command/swarm/ca_test.go index 499090477e4b..d0eff1164708 100644 --- a/cli/command/swarm/ca_test.go +++ b/cli/command/swarm/ca_test.go @@ -13,6 +13,28 @@ import ( is "gotest.tools/assert/cmp" ) +const ( + cert = ` +-----BEGIN CERTIFICATE----- +MIIBuDCCAV4CCQDOqUYOWdqMdjAKBggqhkjOPQQDAzBjMQswCQYDVQQGEwJVUzEL +MAkGA1UECAwCQ0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRv +Y2tlcjEPMA0GA1UECwwGRG9ja2VyMQ0wCwYDVQQDDARUZXN0MCAXDTE4MDcwMjIx +MjkxOFoYDzMwMTcxMTAyMjEyOTE4WjBjMQswCQYDVQQGEwJVUzELMAkGA1UECAwC +Q0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRvY2tlcjEPMA0G +A1UECwwGRG9ja2VyMQ0wCwYDVQQDDARUZXN0MFkwEwYHKoZIzj0CAQYIKoZIzj0D +AQcDQgAEgvvZl5Vqpr1e+g5IhoU6TZHgRau+BZETVFTmqyWYajA/mooRQ1MZTozu +s9ZZZA8tzUhIqS36gsFuyIZ4YiAlyjAKBggqhkjOPQQDAwNIADBFAiBQ7pCPQrj8 +8zaItMf0pk8j1NU5XrFqFEZICzvjzUJQBAIhAKq2gFwoTn8KH+cAAXZpAGJPmOsT +zsBT8gBAOHhNA6/2 +-----END CERTIFICATE-----` + key = ` +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEICyheZpw70pbgO4hEuwhZTETWyTpNJmJ3TyFaWT6WTRkoAoGCCqGSM49 +AwEHoUQDQgAEgvvZl5Vqpr1e+g5IhoU6TZHgRau+BZETVFTmqyWYajA/mooRQ1MZ +Tozus9ZZZA8tzUhIqS36gsFuyIZ4YiAlyg== +-----END EC PRIVATE KEY-----` +) + func swarmSpecWithFullCAConfig() *swarm.Spec { return &swarm.Spec{ CAConfig: swarm.CAConfig{ @@ -37,51 +59,79 @@ func TestDisplayTrustRootNoRoot(t *testing.T) { assert.Error(t, err, "No CA information available") } +type invalidCATestCases struct { + args []string + errorMsg string +} + +func writeFile(data string) (string, error) { + tmpfile, err := ioutil.TempFile("", "testfile") + if err != nil { + return "", err + } + _, err = tmpfile.Write([]byte(data)) + if err != nil { + return "", err + } + tmpfile.Close() + return tmpfile.Name(), nil +} + func TestDisplayTrustRootInvalidFlags(t *testing.T) { // we need an actual PEMfile to test - tmpfile, err := ioutil.TempFile("", "pemfile") + tmpfile, err := writeFile(cert) assert.NilError(t, err) - defer os.Remove(tmpfile.Name()) - tmpfile.Write([]byte(` ------BEGIN CERTIFICATE----- -MIIBajCCARCgAwIBAgIUe0+jYWhxN8fFOByC7yveIYgvx1kwCgYIKoZIzj0EAwIw -EzERMA8GA1UEAxMIc3dhcm0tY2EwHhcNMTcwNjI3MTUxNDAwWhcNMzcwNjIyMTUx -NDAwWjATMREwDwYDVQQDEwhzd2FybS1jYTBZMBMGByqGSM49AgEGCCqGSM49AwEH -A0IABGgbOZLd7b4b262+6m4ignIecbAZKim6djNiIS1Kl5IHciXYn7gnSpsayjn7 -GQABpgkdPeM9TEQowmtR1qSnORujQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMB -Af8EBTADAQH/MB0GA1UdDgQWBBQ6Rtcn823/fxRZyheRDFpDzuBMpTAKBggqhkjO -PQQDAgNIADBFAiEAqD3Kb2rgsy6NoTk+zEgcUi/aGBCsvQDG3vML1PXN8j0CIBjj -4nDj+GmHXcnKa8wXx70Z8OZEpRQIiKDDLmcXuslp ------END CERTIFICATE----- -`)) - tmpfile.Close() + defer os.Remove(tmpfile) - errorTestCases := [][]string{ + errorTestCases := []invalidCATestCases{ { - "--ca-cert=" + tmpfile.Name(), + args: []string{"--ca-cert=" + tmpfile}, + errorMsg: "flag requires the `--rotate` flag to update the CA", }, { - "--ca-key=" + tmpfile.Name(), + args: []string{"--ca-key=" + tmpfile}, + errorMsg: "flag requires the `--rotate` flag to update the CA", }, { // to make sure we're not erroring because we didn't provide a CA key along with the CA cert - - "--ca-cert=" + tmpfile.Name(), - "--ca-key=" + tmpfile.Name(), + args: []string{ + "--ca-cert=" + tmpfile, + "--ca-key=" + tmpfile, + }, + errorMsg: "flag requires the `--rotate` flag to update the CA", }, { - "--cert-expiry=2160h0m0s", + args: []string{"--cert-expiry=2160h0m0s"}, + errorMsg: "flag requires the `--rotate` flag to update the CA", }, { - "--external-ca=protocol=cfssl,url=https://some.com/https/url", + args: []string{"--external-ca=protocol=cfssl,url=https://some.com/https/url"}, + errorMsg: "flag requires the `--rotate` flag to update the CA", }, { // to make sure we're not erroring because we didn't provide a CA cert and external CA - - "--ca-cert=" + tmpfile.Name(), - "--external-ca=protocol=cfssl,url=https://some.com/https/url", + args: []string{ + "--ca-cert=" + tmpfile, + "--external-ca=protocol=cfssl,url=https://some.com/https/url", + }, + errorMsg: "flag requires the `--rotate` flag to update the CA", + }, + { + args: []string{ + "--rotate", + "--external-ca=protocol=cfssl,url=https://some.com/https/url", + }, + errorMsg: "rotating to an external CA requires the `--ca-cert` flag to specify the external CA's cert - " + + "to add an external CA with the current root CA certificate, use the `update` command instead", + }, + { + args: []string{ + "--rotate", + "--ca-cert=" + tmpfile, + }, + errorMsg: "the --ca-cert flag requires that a --ca-key flag and/or --external-ca flag be provided as well", }, } - for _, args := range errorTestCases { + for _, testCase := range errorTestCases { cmd := newCACommand( test.NewFakeCli(&fakeClient{ swarmInspectFunc: func() (swarm.Swarm, error) { @@ -94,9 +144,9 @@ PQQDAgNIADBFAiEAqD3Kb2rgsy6NoTk+zEgcUi/aGBCsvQDG3vML1PXN8j0CIBjj }, nil }, })) - assert.Check(t, cmd.Flags().Parse(args)) + assert.Check(t, cmd.Flags().Parse(testCase.args)) cmd.SetOutput(ioutil.Discard) - assert.ErrorContains(t, cmd.Execute(), "flag requires the `--rotate` flag to update the CA") + assert.ErrorContains(t, cmd.Execute(), testCase.errorMsg) } } @@ -112,43 +162,139 @@ func TestDisplayTrustRoot(t *testing.T) { assert.Check(t, is.Equal(trustRoot+"\n", buffer.String())) } +type swarmUpdateRecorder struct { + spec swarm.Spec +} + +func (s *swarmUpdateRecorder) swarmUpdate(sp swarm.Spec, _ swarm.UpdateFlags) error { + s.spec = sp + return nil +} + +func swarmInspectFuncWithFullCAConfig() (swarm.Swarm, error) { + return swarm.Swarm{ + ClusterInfo: swarm.ClusterInfo{ + Spec: *swarmSpecWithFullCAConfig(), + }, + }, nil +} + func TestUpdateSwarmSpecDefaultRotate(t *testing.T) { - spec := swarmSpecWithFullCAConfig() - flags := newCACommand(nil).Flags() - updateSwarmSpec(spec, flags, caOptions{}) + s := &swarmUpdateRecorder{} + cli := test.NewFakeCli(&fakeClient{ + swarmInspectFunc: swarmInspectFuncWithFullCAConfig, + swarmUpdateFunc: s.swarmUpdate, + }) + cmd := newCACommand(cli) + cmd.SetArgs([]string{"--rotate", "--detach"}) + cmd.SetOutput(cli.OutBuffer()) + assert.NilError(t, cmd.Execute()) expected := swarmSpecWithFullCAConfig() expected.CAConfig.ForceRotate = 2 expected.CAConfig.SigningCACert = "" expected.CAConfig.SigningCAKey = "" - assert.Check(t, is.DeepEqual(expected, spec)) + assert.Check(t, is.DeepEqual(*expected, s.spec)) } -func TestUpdateSwarmSpecPartial(t *testing.T) { - spec := swarmSpecWithFullCAConfig() - flags := newCACommand(nil).Flags() - updateSwarmSpec(spec, flags, caOptions{ - rootCACert: PEMFile{contents: "cacert"}, +func TestUpdateSwarmSpecCertAndKey(t *testing.T) { + certfile, err := writeFile(cert) + assert.NilError(t, err) + defer os.Remove(certfile) + + keyfile, err := writeFile(key) + assert.NilError(t, err) + defer os.Remove(keyfile) + + s := &swarmUpdateRecorder{} + cli := test.NewFakeCli(&fakeClient{ + swarmInspectFunc: swarmInspectFuncWithFullCAConfig, + swarmUpdateFunc: s.swarmUpdate, }) + cmd := newCACommand(cli) + cmd.SetArgs([]string{ + "--rotate", + "--detach", + "--ca-cert=" + certfile, + "--ca-key=" + keyfile, + "--cert-expiry=3m"}) + cmd.SetOutput(cli.OutBuffer()) + assert.NilError(t, cmd.Execute()) expected := swarmSpecWithFullCAConfig() - expected.CAConfig.SigningCACert = "cacert" - assert.Check(t, is.DeepEqual(expected, spec)) + expected.CAConfig.SigningCACert = cert + expected.CAConfig.SigningCAKey = key + expected.CAConfig.NodeCertExpiry = 3 * time.Minute + assert.Check(t, is.DeepEqual(*expected, s.spec)) } -func TestUpdateSwarmSpecFullFlags(t *testing.T) { - flags := newCACommand(nil).Flags() - flags.Lookup(flagCertExpiry).Changed = true - spec := swarmSpecWithFullCAConfig() - updateSwarmSpec(spec, flags, caOptions{ - rootCACert: PEMFile{contents: "cacert"}, - rootCAKey: PEMFile{contents: "cakey"}, - swarmCAOptions: swarmCAOptions{nodeCertExpiry: 3 * time.Minute}, +func TestUpdateSwarmSpecCertAndExternalCA(t *testing.T) { + certfile, err := writeFile(cert) + assert.NilError(t, err) + defer os.Remove(certfile) + + s := &swarmUpdateRecorder{} + cli := test.NewFakeCli(&fakeClient{ + swarmInspectFunc: swarmInspectFuncWithFullCAConfig, + swarmUpdateFunc: s.swarmUpdate, }) + cmd := newCACommand(cli) + cmd.SetArgs([]string{ + "--rotate", + "--detach", + "--ca-cert=" + certfile, + "--external-ca=protocol=cfssl,url=https://some.external.ca"}) + cmd.SetOutput(cli.OutBuffer()) + assert.NilError(t, cmd.Execute()) expected := swarmSpecWithFullCAConfig() - expected.CAConfig.SigningCACert = "cacert" - expected.CAConfig.SigningCAKey = "cakey" - expected.CAConfig.NodeCertExpiry = 3 * time.Minute - assert.Check(t, is.DeepEqual(expected, spec)) + expected.CAConfig.SigningCACert = cert + expected.CAConfig.SigningCAKey = "" + expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{ + { + Protocol: swarm.ExternalCAProtocolCFSSL, + URL: "https://some.external.ca", + CACert: cert, + Options: make(map[string]string), + }, + } + assert.Check(t, is.DeepEqual(*expected, s.spec)) +} + +func TestUpdateSwarmSpecCertAndKeyAndExternalCA(t *testing.T) { + certfile, err := writeFile(cert) + assert.NilError(t, err) + defer os.Remove(certfile) + + keyfile, err := writeFile(key) + assert.NilError(t, err) + defer os.Remove(keyfile) + + s := &swarmUpdateRecorder{} + cli := test.NewFakeCli(&fakeClient{ + swarmInspectFunc: swarmInspectFuncWithFullCAConfig, + swarmUpdateFunc: s.swarmUpdate, + }) + cmd := newCACommand(cli) + cmd.SetArgs([]string{ + "--rotate", + "--detach", + "--ca-cert=" + certfile, + "--ca-key=" + keyfile, + "--external-ca=protocol=cfssl,url=https://some.external.ca"}) + cmd.SetOutput(cli.OutBuffer()) + assert.NilError(t, cmd.Execute()) + + expected := swarmSpecWithFullCAConfig() + expected.CAConfig.SigningCACert = cert + expected.CAConfig.SigningCAKey = key + expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{ + { + Protocol: swarm.ExternalCAProtocolCFSSL, + URL: "https://some.external.ca", + CACert: cert, + Options: make(map[string]string), + }, + } + assert.Check(t, is.DeepEqual(*expected, s.spec)) } diff --git a/cli/command/swarm/opts.go b/cli/command/swarm/opts.go index b3baba273ea6..b2a4de1f692e 100644 --- a/cli/command/swarm/opts.go +++ b/cli/command/swarm/opts.go @@ -230,7 +230,7 @@ func addSwarmFlags(flags *pflag.FlagSet, opts *swarmOptions) { addSwarmCAFlags(flags, &opts.swarmCAOptions) } -func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet) { +func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) { if flags.Changed(flagTaskHistoryLimit) { spec.Orchestration.TaskHistoryRetentionLimit = &opts.taskHistoryLimit } @@ -246,7 +246,7 @@ func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet) if flags.Changed(flagAutolock) { spec.EncryptionConfig.AutoLockManagers = opts.autolock } - opts.mergeSwarmSpecCAFlags(spec, flags) + opts.mergeSwarmSpecCAFlags(spec, flags, caCert) } type swarmCAOptions struct { @@ -254,17 +254,20 @@ type swarmCAOptions struct { externalCA ExternalCAOption } -func (opts *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet) { +func (opts *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) { if flags.Changed(flagCertExpiry) { spec.CAConfig.NodeCertExpiry = opts.nodeCertExpiry } if flags.Changed(flagExternalCA) { spec.CAConfig.ExternalCAs = opts.externalCA.Value() + for _, ca := range spec.CAConfig.ExternalCAs { + ca.CACert = caCert + } } } func (opts *swarmOptions) ToSpec(flags *pflag.FlagSet) swarm.Spec { var spec swarm.Spec - opts.mergeSwarmSpec(&spec, flags) + opts.mergeSwarmSpec(&spec, flags, "") return spec } diff --git a/cli/command/swarm/update.go b/cli/command/swarm/update.go index 52dc335fdc54..6b9ad728f6cf 100644 --- a/cli/command/swarm/update.go +++ b/cli/command/swarm/update.go @@ -48,7 +48,7 @@ func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, opts swarmOptions) e prevAutoLock := swarmInspect.Spec.EncryptionConfig.AutoLockManagers - opts.mergeSwarmSpec(&swarmInspect.Spec, flags) + opts.mergeSwarmSpec(&swarmInspect.Spec, flags, swarmInspect.ClusterInfo.TLSInfo.TrustRoot) curAutoLock := swarmInspect.Spec.EncryptionConfig.AutoLockManagers diff --git a/cli/command/swarm/update_test.go b/cli/command/swarm/update_test.go index 256c9de6d0db..20a5624a897e 100644 --- a/cli/command/swarm/update_test.go +++ b/cli/command/swarm/update_test.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "github.com/pkg/errors" + // Import builders to get the builder function as package function . "github.com/docker/cli/internal/test/builders" "gotest.tools/assert" @@ -82,6 +83,9 @@ func TestSwarmUpdateErrors(t *testing.T) { } func TestSwarmUpdate(t *testing.T) { + swarmInfo := Swarm() + swarmInfo.ClusterInfo.TLSInfo.TrustRoot = "trustroot" + testCases := []struct { name string args []string @@ -105,6 +109,9 @@ func TestSwarmUpdate(t *testing.T) { flagAutolock: "true", flagQuiet: "true", }, + swarmInspectFunc: func() (swarm.Swarm, error) { + return *swarmInfo, nil + }, swarmUpdateFunc: func(swarm swarm.Spec, flags swarm.UpdateFlags) error { if *swarm.Orchestration.TaskHistoryRetentionLimit != 10 { return errors.Errorf("historyLimit not correctly set") @@ -123,7 +130,7 @@ func TestSwarmUpdate(t *testing.T) { if swarm.CAConfig.NodeCertExpiry != certExpiryDuration { return errors.Errorf("certExpiry not correctly set") } - if len(swarm.CAConfig.ExternalCAs) != 1 { + if len(swarm.CAConfig.ExternalCAs) != 1 || swarm.CAConfig.ExternalCAs[0].CACert != "trustroot" { return errors.Errorf("externalCA not correctly set") } if *swarm.Raft.KeepOldSnapshots != 10 {