Synchronous CLI command for root CA rotation - #48

cyli · 2017-05-08T21:51:22Z

Provide command line tool to view and rotate swarm's currently CA root certificate.

Signed-off-by: Ying Li ying.li@docker.com

This code was originally in moby/moby#32993.

root@ubuntu:~#  docker swarm ca
-----BEGIN CERTIFICATE-----
MIIBazCCARCgAwIBAgIUJPzo67QC7g8Ebg2ansjkZ8CbmaswCgYIKoZIzj0EAwIw
EzERMA8GA1UEAxMIc3dhcm0tY2EwHhcNMTcwNTAzMTcxMDAwWhcNMzcwNDI4MTcx
MDAwWjATMREwDwYDVQQDEwhzd2FybS1jYTBZMBMGByqGSM49AgEGCCqGSM49AwEH
A0IABKL6/C0sihYEb935wVPRA8MqzPLn3jzou0OJRXHsCLcVExigrMdgmLCC+Va4
+sJ+SLVO1eQbvLHH8uuDdF/QOU6jQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMB
Af8EBTADAQH/MB0GA1UdDgQWBBSfUy5bjUnBAx/B0GkOBKp91XvxzjAKBggqhkjO
PQQDAgNJADBGAiEAnbvh0puOS5R/qvy1PMHY1iksYKh2acsGLtL/jAIvO4ACIQCi
lIwQqLkJ48SQqCjG1DBTSBsHmMSRT+6mE2My+Z3GKA==
-----END CERTIFICATE-----

root@ubuntu:~# docker swarm ca --help

Usage:	docker swarm ca [OPTIONS]

Manage root CA

Options:
      --ca-cert pem-file          Path to the PEM-formatted root CA certificate to use for the new cluster
      --ca-key pem-file           Path to the PEM-formatted root CA key to use for the new cluster
      --cert-expiry duration      Validity period for node certificates (ns|us|ms|s|m|h) (default 2160h0m0s)
  -d, --detach                    Exit immediately instead of waiting for the root rotation to converge
      --external-ca external-ca   Specifications of one or more certificate signing endpoints
      --help                      Print usage
  -q, --quiet                     Suppress progress output
      --rotate                    Rotate the swarm CA - if no certificate or key are provided, new ones will be generated

Sample progress bar here, copied styling from the synchronous service create progress bar: https://asciinema.org/a/2em3i7y1u50ajeut13es5m6uh

Note: I originally thought about just doing docker swarm update --rotate-ca cert=/path/...,key=/path/... but I wasn't sure how to also do tell the CLI to just generate a cert and key - it'd probably have to look like docker swarm update --rotate-ca "" which seems weird, hence the new command.

Also a suggestion from @NathanMcCauley: if we have service identities, and those could be rotated, do we want to use the same CLI command to view/rotate those as well?

cyli · 2017-05-12T17:55:32Z

Have re-vendored the latest version of moby/moby master instead of my fork, so this is ready for review.

thaJeztah · 2017-05-15T09:43:50Z

Looking at this again, I wonder if;

swarm ca should be a separate command; it's showing a single field in the swarm configuration, does this open the door to creating a separate command for each property? Is this information part of docker system info (or should we have docker swarm info? Possibly --rotate should be part of docker swarm update
if we do think ca should be its own "entity" / "object type" in the CLI, should it follow the ls / list / inspect etc subcommands? (will be a bit odd as well, given that there's only a single CA, not a list)
If we want to keep it similar to join-token, then swarm ca --rotate should print the ca after it was rotated

Happy to hear thoughts @aaronlehmann @diogomonica @dnephin

cyli · 2017-05-15T17:33:19Z

Possibly --rotate should be part of docker swarm update

I originally thought about just doing docker swarm update --rotate-ca cert=/path/...,key=/path/... but I wasn't sure how to also do tell the CLI to just generate a cert and key - it'd probably have to look like docker swarm update --rotate-ca "" which seems weird, hence the new command - but happy to have other opinions and suggestions instead - maybe the cert and key can be different flags?

The one argument I'd have for a separate command is that docker swarm update already has a bunch of flags, and the extra ones here (--ca-cert, --ca-key, -d and -q) for instance all seem to be bundled together, so maybe including them in a separate command makes sense?

Is this information part of docker system info (or should we have docker swarm info)?

Some of this info is currently on docker system info - I don't mind adding a docker swarm info, if that'd be useful - it seems like system info includes swarm info though.

If we want to keep it similar to join-token, then swarm ca --rotate should print the ca after it was rotated.

Happy to do that as well.

diogomonica · 2017-05-15T19:40:44Z

I have a slight preference for having the ca subcommand

diogomonica · 2017-05-15T19:41:05Z

LGTM

cyli · 2017-05-16T00:17:51Z

Updated the --rotate command to print out the final CA certificate after the root rotation is complete.

aaronlehmann · 2017-05-16T00:21:31Z

LGTM

tonistiigi · 2017-05-16T18:54:48Z

+		go func() {
+			for {
+				var buf [1024]byte
+				if _, err := pipeReader.Read(buf[:]); err != nil {


nit: this could be simpler with ioutil.Discard

Updated with io.Copy(ioutil.Discard, pipeReader)

Don't need a for or function wrapper anymore.

thaJeztah

left one comment, LGTM otherwise

thaJeztah · 2017-05-16T21:13:02Z

+		Args:  cli.NoArgs,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runRotateCA(dockerCli, cmd.Flags(), opts)
+		},


Can you add

Tags: map[string]string{"version": "1.30"},

like we have on cli/command/system/prune.go#L31, so that the command is hidden when talking to an older daemon, and produces an error that it requires a newer daemon

…t certificate. Signed-off-by: Ying Li <ying.li@docker.com>

Signed-off-by: Ying Li <ying.li@docker.com>

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

aaronlehmann · 2017-05-16T21:36:24Z

@tonistiigi: Pushed a commit that simplifies the use of io.Copy

tonistiigi · 2017-05-16T21:37:21Z

LGTM

cyli · 2017-05-16T22:01:08Z

Thanks @aaronlehmann!

katieannemills · 2017-06-19T13:59:27Z

Hi folks - Training wants to highlight this feature in our workshops, but I'm a bit confused; docker swarm ca --cert-expiry 720h spits a key back at me on 17.06.0-ce-rc2 but shows no change to the CA configuration / expiry duration key in the output of docker system info. I assumed (wrongly?) this command was equivalent to docker swarm update --cert-expiry 720h, which does show an updated expiry duration immediately. Thoughts?

cyli · 2017-06-19T17:52:02Z

@billmills Ah, apologies, that was intended to be used with the --rotate flag (e.g. you can rotate the CA cert and update the cert expiry to be used with that new cert at the same time. If you don't pass --rotate, it ignores all the other flags, since cert expiry could also be changed with docker swarm update --cert-expiry ....

Wondering if we should update the flags, if used by themselves, to specify that they are ignored unless --rotate is passed?

dnephin · 2017-06-19T18:31:40Z

A warning would be great. We might even want to consider it an error.

Don't create source directory while the daemon is being shutdown, fix…

cyli mentioned this pull request May 8, 2017

API changes to rotate swarm root CA moby/moby#32993

Merged

cyli changed the title ~~WIP: CLI command for root CA rotation~~ WIP: Synchronous CLI command for root CA rotation May 9, 2017

cyli mentioned this pull request May 10, 2017

Root CA rotation CLI docs moby/moby#33152

Merged

cyli changed the title ~~WIP: Synchronous CLI command for root CA rotation~~ Synchronous CLI command for root CA rotation May 12, 2017

tonistiigi reviewed May 16, 2017

View reviewed changes

cyli mentioned this pull request May 16, 2017

External CA is not being accepted by new swarm node moby/moby#33216

Closed

thaJeztah requested changes May 16, 2017

View reviewed changes

cyli and others added 3 commits May 16, 2017 14:31

Provide command line tool to view and rotate swarm's currently CA roo…

3fe8321

…t certificate. Signed-off-by: Ying Li <ying.li@docker.com>

Re-vendor docker/docker.

103bfb2

Signed-off-by: Ying Li <ying.li@docker.com>

Avoid loop when discarding contents of progress pipe

51f6983

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

aaronlehmann merged commit c17acee into docker:master May 16, 2017

GordonTheTurtle added this to the 17.06.0 milestone May 16, 2017

cyli deleted the root-rotation-cli branch May 16, 2017 22:00

cyli mentioned this pull request Jun 19, 2017

If docker swarm ca is not called with the --rotate flag, warn if other flags are passed #207

Merged

thaJeztah added impact/changelog impact/documentation labels Jun 27, 2017

nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025

Merge pull request docker#48 from seemethere/cherry_pick_33330

0cbae9b

Don't create source directory while the daemon is being shutdown, fix…

Conversation

cyli commented May 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyli commented May 12, 2017

Uh oh!

thaJeztah commented May 15, 2017

Uh oh!

cyli commented May 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

diogomonica commented May 15, 2017

Uh oh!

diogomonica commented May 15, 2017

Uh oh!

cyli commented May 16, 2017

Uh oh!

aaronlehmann commented May 16, 2017

Uh oh!

tonistiigi May 16, 2017

Choose a reason for hiding this comment

Uh oh!

cyli May 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonistiigi May 16, 2017

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah May 16, 2017

Choose a reason for hiding this comment

Uh oh!

aaronlehmann commented May 16, 2017

Uh oh!

tonistiigi commented May 16, 2017

Uh oh!

cyli commented May 16, 2017

Uh oh!

katieannemills commented Jun 19, 2017

Uh oh!

cyli commented Jun 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnephin commented Jun 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

cyli commented May 8, 2017 •

edited

Loading

cyli commented May 15, 2017 •

edited

Loading

cyli May 16, 2017 •

edited

Loading

cyli commented Jun 19, 2017 •

edited

Loading