New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API changes to rotate swarm root CA #32993

Merged
merged 3 commits into from May 12, 2017

Conversation

@cyli
Contributor

cyli commented May 3, 2017

This is stacked on top of #32875. Merged.

This is the first stab the CLI command to actually rotate the root CA cert. I added a new one, because we want to optionally take a root certificate and key, and/or external CAs, but we also want the user to be able to tell swarm to generate one. So:

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/dcpy5y2fcueb8fpmokxvf3vec

If the design looks good, I'll add a bunch more tests and update the docs. cc @aaronlehmann @aluzzardi @diogomonica

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 command to view/rotate those as well?

cute

@cyli cyli referenced this pull request May 3, 2017

Closed

[feature] CA Root rotation #1990

10 of 10 tasks complete
@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann May 4, 2017

Contributor

CI faliing:

22:42:42 --- FAIL: TestProgress (0.00s)
22:42:42 panic: runtime error: makeslice: len out of range [recovered]
22:42:42 	panic: runtime error: makeslice: len out of range
Contributor

aaronlehmann commented May 4, 2017

CI faliing:

22:42:42 --- FAIL: TestProgress (0.00s)
22:42:42 panic: runtime error: makeslice: len out of range [recovered]
22:42:42 	panic: runtime error: makeslice: len out of range
@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann May 4, 2017

Contributor

Design LGTM

Contributor

aaronlehmann commented May 4, 2017

Design LGTM

@thaJeztah thaJeztah added the impact/cli label May 5, 2017

@thaJeztah thaJeztah added this to the 17.06.0 milestone May 5, 2017

@diogomonica

This comment has been minimized.

Show comment
Hide comment
@diogomonica

diogomonica May 8, 2017

Contributor

LGTM, still playing with it.

Contributor

diogomonica commented May 8, 2017

LGTM, still playing with it.

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 8, 2017

Contributor

I've split out the CLI code to docker/cli#48.

Contributor

cyli commented May 8, 2017

I've split out the CLI code to docker/cli#48.

@cyli cyli changed the title from WIP: Synchronous root rotation CLI command to WIP: API to rotate swarm root CA May 9, 2017

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure May 9, 2017

Contributor

@cyli

00:15:37 These files are not properly gofmt'd:
00:15:37  - integration-cli/docker_api_swarm_test.go
00:15:37 
00:15:37 Please reformat the above files using "gofmt -s -w" and commit the result.
Contributor

mlaventure commented May 9, 2017

@cyli

00:15:37 These files are not properly gofmt'd:
00:15:37  - integration-cli/docker_api_swarm_test.go
00:15:37 
00:15:37 Please reformat the above files using "gofmt -s -w" and commit the result.
@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 10, 2017

Contributor

@mlaventure Thank you - fixed.

Contributor

cyli commented May 10, 2017

@mlaventure Thank you - fixed.

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 10, 2017

Contributor

Also updated the api version history here. cc @thaJeztah if you wouldn't mind checking the docs here as well :)

Contributor

cyli commented May 10, 2017

Also updated the api version history here. cc @thaJeztah if you wouldn't mind checking the docs here as well :)

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure May 10, 2017

Contributor

@cyli it looks like the cli revision is invalid now

Contributor

mlaventure commented May 10, 2017

@cyli it looks like the cli revision is invalid now

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann May 10, 2017

Contributor

That was my branch, which I had to rebase to get merged and allow moby to switch back to the official repo. I've restored that revision. A simple rebuild should work.

Contributor

aaronlehmann commented May 10, 2017

That was my branch, which I had to rebase to get merged and allow moby to switch back to the official repo. I've restored that revision. A simple rebuild should work.

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 10, 2017

Contributor

Thanks @aaronlehmann!

Contributor

cyli commented May 10, 2017

Thanks @aaronlehmann!

@cyli cyli referenced this pull request May 10, 2017

Merged

Root CA rotation CLI docs #33152

@cyli cyli changed the title from WIP: API to rotate swarm root CA to API changes to rotate swarm root CA May 10, 2017

Propagate the desired CA certificate and CAConfig ForceRotate parameter
in the Docker REST APIs when viewing or updating the swarm spec info, and
also propagate the desired CA key in the Docker REST APIs when updating
swarm spec info only (it is not available for viewing).

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

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 11, 2017

Contributor

Rebased again. PTAL.

Contributor

cyli commented May 11, 2017

Rebased again. PTAL.

cyli added some commits May 3, 2017

Update the stream formatter to display custom unit numbers.
Signed-off-by: Ying Li <ying.li@docker.com>
Add API test to rotate the swarm CA certificate
Signed-off-by: Ying Li <ying.li@docker.com>
@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann May 11, 2017

Contributor

LGTM

Contributor

aaronlehmann commented May 11, 2017

LGTM

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 12, 2017

Contributor

Also pinging @cpuguy83 for review: since you reviewed the last API changes that included TLS info (sorry, no good deed goes unpunished :)) - this is the follow up that will trigger the root rotation that can make use of the TLS info.

Alternately @vdemeester or @tiborvass or may or may not be familiar with the feature.

Contributor

cyli commented May 12, 2017

Also pinging @cpuguy83 for review: since you reviewed the last API changes that included TLS info (sorry, no good deed goes unpunished :)) - this is the follow up that will trigger the root rotation that can make use of the TLS info.

Alternately @vdemeester or @tiborvass or may or may not be familiar with the feature.

@cpuguy83

LGTM

@aaronlehmann aaronlehmann merged commit eb8abc9 into moby:master May 12, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 34012 has succeeded
Details
janky Jenkins build Docker-PRs 42613 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 2988 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 13845 has succeeded
Details
z Jenkins build Docker-PRs-s390x 2714 has succeeded
Details
@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 12, 2017

Contributor

Thanks @cpuguy83 and @aaronlehmann! :)

Contributor

cyli commented May 12, 2017

Thanks @cpuguy83 and @aaronlehmann! :)

@cyli cyli deleted the cyli:root-rotation-cli branch May 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment