From 81e9837859b3c624fbd0e87f51e29e01211e3c81 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 20 Jun 2017 14:41:40 -0400 Subject: [PATCH] Refactor caCommand Split out a swarmCAOptions struct for options that are shared between the ca and update commands. Change the 'no trust root' message to an error. Add some unit tests. Signed-off-by: Daniel Nephin --- cli/command/swarm/ca.go | 80 ++++++++++++++++---------------- cli/command/swarm/ca_test.go | 88 ++++++++++++++++++++++++++++++++++++ cli/command/swarm/cmd.go | 2 +- cli/command/swarm/opts.go | 28 ++++++++---- 4 files changed, 148 insertions(+), 50 deletions(-) create mode 100644 cli/command/swarm/ca_test.go diff --git a/cli/command/swarm/ca.go b/cli/command/swarm/ca.go index 2f01ab4da4..83ac02e776 100644 --- a/cli/command/swarm/ca.go +++ b/cli/command/swarm/ca.go @@ -3,23 +3,22 @@ package swarm import ( "fmt" "io" - "strings" - - "golang.org/x/net/context" - "io/ioutil" + "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/swarm/progress" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/pkg/jsonmessage" + "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" + "golang.org/x/net/context" ) type caOptions struct { - swarmOptions + swarmCAOptions rootCACert PEMFile rootCAKey PEMFile rotate bool @@ -27,21 +26,21 @@ type caOptions struct { quiet bool } -func newRotateCACommand(dockerCli command.Cli) *cobra.Command { +func newCACommand(dockerCli command.Cli) *cobra.Command { opts := caOptions{} cmd := &cobra.Command{ Use: "ca [OPTIONS]", - Short: "Manage root CA", + Short: "Display and rotate the root CA", Args: cli.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return runRotateCA(dockerCli, cmd.Flags(), opts) + return runCA(dockerCli, cmd.Flags(), opts) }, Tags: map[string]string{"version": "1.30"}, } flags := cmd.Flags() - addSwarmCAFlags(flags, &opts.swarmOptions) + addSwarmCAFlags(flags, &opts.swarmCAOptions) flags.BoolVar(&opts.rotate, flagRotate, false, "Rotate the swarm CA - if no certificate or key are provided, new ones will be generated") flags.Var(&opts.rootCACert, flagCACert, "Path to the PEM-formatted root CA certificate to use for the new cluster") flags.Var(&opts.rootCAKey, flagCAKey, "Path to the PEM-formatted root CA key to use for the new cluster") @@ -51,7 +50,7 @@ func newRotateCACommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRotateCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error { +func runCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error { client := dockerCli.Client() ctx := context.Background() @@ -66,31 +65,10 @@ func runRotateCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) er return fmt.Errorf("`--%s` flag requires the `--rotate` flag to update the CA", f) } } - if swarmInspect.ClusterInfo.TLSInfo.TrustRoot == "" { - fmt.Fprintln(dockerCli.Out(), "No CA information available") - } else { - fmt.Fprintln(dockerCli.Out(), strings.TrimSpace(swarmInspect.ClusterInfo.TLSInfo.TrustRoot)) - } - return nil - } - - genRootCA := true - spec := &swarmInspect.Spec - opts.mergeSwarmSpec(spec, flags) // updates the spec given the cert expiry or external CA flag - if flags.Changed(flagCACert) { - spec.CAConfig.SigningCACert = opts.rootCACert.Contents() - genRootCA = false - } - if flags.Changed(flagCAKey) { - spec.CAConfig.SigningCAKey = opts.rootCAKey.Contents() - genRootCA = false - } - if genRootCA { - spec.CAConfig.ForceRotate++ - spec.CAConfig.SigningCACert = "" - spec.CAConfig.SigningCAKey = "" + return displayTrustRoot(dockerCli.Out(), swarmInspect) } + updateSwarmSpec(&swarmInspect.Spec, flags, opts) if err := client.SwarmUpdate(ctx, swarmInspect.Version, swarmInspect.Spec, swarm.UpdateFlags{}); err != nil { return err } @@ -98,7 +76,29 @@ func runRotateCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) er if opts.detach { return nil } + return attach(ctx, dockerCli, opts) +} +func updateSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, opts caOptions) { + opts.mergeSwarmSpecCAFlags(spec, flags) + caCert := opts.rootCACert.Contents() + caKey := opts.rootCAKey.Contents() + + if caCert != "" { + spec.CAConfig.SigningCACert = caCert + } + if caKey != "" { + spec.CAConfig.SigningCAKey = caKey + } + if caKey == "" && caCert == "" { + spec.CAConfig.ForceRotate++ + spec.CAConfig.SigningCACert = "" + spec.CAConfig.SigningCAKey = "" + } +} + +func attach(ctx context.Context, dockerCli command.Cli, opts caOptions) error { + client := dockerCli.Client() errChan := make(chan error, 1) pipeReader, pipeWriter := io.Pipe() @@ -111,7 +111,7 @@ func runRotateCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) er return <-errChan } - err = jsonmessage.DisplayJSONMessagesToStream(pipeReader, dockerCli.Out(), nil) + err := jsonmessage.DisplayJSONMessagesToStream(pipeReader, dockerCli.Out(), nil) if err == nil { err = <-errChan } @@ -119,15 +119,17 @@ func runRotateCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) er return err } - swarmInspect, err = client.SwarmInspect(ctx) + swarmInspect, err := client.SwarmInspect(ctx) if err != nil { return err } + return displayTrustRoot(dockerCli.Out(), swarmInspect) +} - if swarmInspect.ClusterInfo.TLSInfo.TrustRoot == "" { - fmt.Fprintln(dockerCli.Out(), "No CA information available") - } else { - fmt.Fprintln(dockerCli.Out(), strings.TrimSpace(swarmInspect.ClusterInfo.TLSInfo.TrustRoot)) +func displayTrustRoot(out io.Writer, info swarm.Swarm) error { + if info.ClusterInfo.TLSInfo.TrustRoot == "" { + return errors.New("No CA information available") } + fmt.Fprintln(out, strings.TrimSpace(info.ClusterInfo.TLSInfo.TrustRoot)) return nil } diff --git a/cli/command/swarm/ca_test.go b/cli/command/swarm/ca_test.go new file mode 100644 index 0000000000..f122567c47 --- /dev/null +++ b/cli/command/swarm/ca_test.go @@ -0,0 +1,88 @@ +package swarm + +import ( + "bytes" + "testing" + "time" + + "github.com/docker/docker/api/types/swarm" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func swarmSpecWithFullCAConfig() *swarm.Spec { + return &swarm.Spec{ + CAConfig: swarm.CAConfig{ + SigningCACert: "cacert", + SigningCAKey: "cakey", + ForceRotate: 1, + NodeCertExpiry: time.Duration(200), + ExternalCAs: []*swarm.ExternalCA{ + { + URL: "https://example.com/ca", + Protocol: swarm.ExternalCAProtocolCFSSL, + CACert: "excacert", + }, + }, + }, + } +} + +func TestDisplayTrustRootNoRoot(t *testing.T) { + buffer := new(bytes.Buffer) + err := displayTrustRoot(buffer, swarm.Swarm{}) + assert.EqualError(t, err, "No CA information available") +} + +func TestDisplayTrustRoot(t *testing.T) { + buffer := new(bytes.Buffer) + trustRoot := "trustme" + err := displayTrustRoot(buffer, swarm.Swarm{ + ClusterInfo: swarm.ClusterInfo{ + TLSInfo: swarm.TLSInfo{TrustRoot: trustRoot}, + }, + }) + require.NoError(t, err) + assert.Equal(t, trustRoot+"\n", buffer.String()) +} + +func TestUpdateSwarmSpecDefaultRotate(t *testing.T) { + spec := swarmSpecWithFullCAConfig() + flags := newCACommand(nil).Flags() + updateSwarmSpec(spec, flags, caOptions{}) + + expected := swarmSpecWithFullCAConfig() + expected.CAConfig.ForceRotate = 2 + expected.CAConfig.SigningCACert = "" + expected.CAConfig.SigningCAKey = "" + assert.Equal(t, expected, spec) +} + +func TestUpdateSwarmSpecPartial(t *testing.T) { + spec := swarmSpecWithFullCAConfig() + flags := newCACommand(nil).Flags() + updateSwarmSpec(spec, flags, caOptions{ + rootCACert: PEMFile{contents: "cacert"}, + }) + + expected := swarmSpecWithFullCAConfig() + expected.CAConfig.SigningCACert = "cacert" + assert.Equal(t, expected, 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}, + }) + + expected := swarmSpecWithFullCAConfig() + expected.CAConfig.SigningCACert = "cacert" + expected.CAConfig.SigningCAKey = "cakey" + expected.CAConfig.NodeCertExpiry = 3 * time.Minute + assert.Equal(t, expected, spec) +} diff --git a/cli/command/swarm/cmd.go b/cli/command/swarm/cmd.go index b7e6dcfda2..d2793f6b91 100644 --- a/cli/command/swarm/cmd.go +++ b/cli/command/swarm/cmd.go @@ -25,7 +25,7 @@ func NewSwarmCommand(dockerCli command.Cli) *cobra.Command { newUpdateCommand(dockerCli), newLeaveCommand(dockerCli), newUnlockCommand(dockerCli), - newRotateCACommand(dockerCli), + newCACommand(dockerCli), ) return cmd } diff --git a/cli/command/swarm/opts.go b/cli/command/swarm/opts.go index 0522f9fdfa..b3baba273e 100644 --- a/cli/command/swarm/opts.go +++ b/cli/command/swarm/opts.go @@ -36,10 +36,9 @@ const ( ) type swarmOptions struct { + swarmCAOptions taskHistoryLimit int64 dispatcherHeartbeat time.Duration - nodeCertExpiry time.Duration - externalCA ExternalCAOption maxSnapshots uint64 snapshotInterval uint64 autolock bool @@ -216,7 +215,7 @@ func parseExternalCA(caSpec string) (*swarm.ExternalCA, error) { return &externalCA, nil } -func addSwarmCAFlags(flags *pflag.FlagSet, opts *swarmOptions) { +func addSwarmCAFlags(flags *pflag.FlagSet, opts *swarmCAOptions) { flags.DurationVar(&opts.nodeCertExpiry, flagCertExpiry, 90*24*time.Hour, "Validity period for node certificates (ns|us|ms|s|m|h)") flags.Var(&opts.externalCA, flagExternalCA, "Specifications of one or more certificate signing endpoints") } @@ -228,7 +227,7 @@ func addSwarmFlags(flags *pflag.FlagSet, opts *swarmOptions) { flags.SetAnnotation(flagMaxSnapshots, "version", []string{"1.25"}) flags.Uint64Var(&opts.snapshotInterval, flagSnapshotInterval, 10000, "Number of log entries between Raft snapshots") flags.SetAnnotation(flagSnapshotInterval, "version", []string{"1.25"}) - addSwarmCAFlags(flags, opts) + addSwarmCAFlags(flags, &opts.swarmCAOptions) } func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet) { @@ -238,12 +237,6 @@ func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet) if flags.Changed(flagDispatcherHeartbeat) { spec.Dispatcher.HeartbeatPeriod = opts.dispatcherHeartbeat } - if flags.Changed(flagCertExpiry) { - spec.CAConfig.NodeCertExpiry = opts.nodeCertExpiry - } - if flags.Changed(flagExternalCA) { - spec.CAConfig.ExternalCAs = opts.externalCA.Value() - } if flags.Changed(flagMaxSnapshots) { spec.Raft.KeepOldSnapshots = &opts.maxSnapshots } @@ -253,6 +246,21 @@ func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet) if flags.Changed(flagAutolock) { spec.EncryptionConfig.AutoLockManagers = opts.autolock } + opts.mergeSwarmSpecCAFlags(spec, flags) +} + +type swarmCAOptions struct { + nodeCertExpiry time.Duration + externalCA ExternalCAOption +} + +func (opts *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet) { + if flags.Changed(flagCertExpiry) { + spec.CAConfig.NodeCertExpiry = opts.nodeCertExpiry + } + if flags.Changed(flagExternalCA) { + spec.CAConfig.ExternalCAs = opts.externalCA.Value() + } } func (opts *swarmOptions) ToSpec(flags *pflag.FlagSet) swarm.Spec {