TLS: converge asset naming of SH and non-SH etcd #621

hongchaodeng · 2017-06-30T17:21:15Z

fix #611

hongchaodeng · 2017-06-30T19:46:01Z

Manually verified recovery case too.

hongchaodeng · 2017-06-30T19:48:04Z

@xiang90 @diegs @aaronlevy
Can you take a review?

hongchaodeng · 2017-06-30T20:33:05Z

coreosbot run e2e

Seems flake:

reboot_test.go:44: rebooting node: ssh: handshake failed: EOF

hongchaodeng · 2017-06-30T23:38:20Z

coreosbot run e2e

hongchaodeng · 2017-07-05T17:10:34Z

@aaronlevy @diegs Can you approve this?

diegs

Somme comments based on what you wrote in #611 (comment)

Am I misunderstanding the distinction between client-ca and server-ca?

diegs · 2017-07-05T18:08:22Z

hack/multi-node/etcd-cloud-config.yaml

@@ -21,10 +21,10 @@ coreos:
          Environment="ETCD_LISTEN_PEER_URLS=https://$private_ipv4:2380"
          Environment="ETCD_INITIAL_CLUSTER={{ETCD_INITIAL_CLUSTER}}"
          Environment="ETCD_SSL_DIR=/etc/etcd/tls"
-          Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-ca.crt"
+          Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-client-ca.crt"


etcd-server-ca.rt?

(and the below should be etcd-server.crt and etcd-server.key

diegs · 2017-07-05T18:08:27Z

hack/multi-node/etcd-cloud-config.yaml

-          Environment="ETCD_PEER_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-ca.crt"
-          Environment="ETCD_PEER_CERT_FILE=/etc/ssl/certs/etcd-peer.crt"
-          Environment="ETCD_PEER_KEY_FILE=/etc/ssl/certs/etcd-peer.key"
+          Environment="ETCD_PEER_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-client-ca.crt"


shouldn't this be peer-ca.crt?

diegs · 2017-07-05T18:09:24Z

hack/single-node/user-data-etcd.sample

-            Environment="ETCD_PEER_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-ca.crt"
-            Environment="ETCD_PEER_CERT_FILE=/etc/ssl/certs/etcd-peer.crt"
-            Environment="ETCD_PEER_KEY_FILE=/etc/ssl/certs/etcd-peer.key"
+            Environment="ETCD_PEER_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-client-ca.crt"


etcd-peer-ca.crt?

diegs · 2017-07-05T18:10:20Z

hack/quickstart/init-master.sh

-Environment="ETCD_PEER_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-ca.crt"
-Environment="ETCD_PEER_CERT_FILE=/etc/ssl/certs/etcd-peer.crt"
-Environment="ETCD_PEER_KEY_FILE=/etc/ssl/certs/etcd-peer.key"
+Environment="ETCD_PEER_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-client-ca.crt"


peer-ca.crt?

diegs · 2017-07-05T18:10:36Z

hack/single-node/user-data-etcd.sample

@@ -12,11 +12,11 @@
            Environment="ETCD_LISTEN_CLIENT_URLS=https://0.0.0.0:2379"
            Environment="ETCD_LISTEN_PEER_URLS=https://0.0.0.0:2380"
            Environment="ETCD_SSL_DIR=/etc/etcd/tls"
-            Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-ca.crt"
+            Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-client-ca.crt"


etcd-server-ca.crt?

(and the two lines below)

hongchaodeng · 2017-07-05T18:18:02Z

@diegs
Let me answer all of the questions above.
So those cloud config yamls are for non-self-hosted cases.
In current setup, non-self-hosted assets for etcd includes:

etcd-client-ca.crt
etcd-client.[crt,key]
etcd-peer.[crt,key]

diegs · 2017-07-05T18:27:26Z

@hongchaodeng couldn't the non-self-hosted code path in bootkube render also output server-ca.crt and peer-ca.crt too (even if they're both copies of etcd-client-ca.crt) to make things consistent and clear?

(Discussed in 2 comments at #611 (comment), I know it's a long thread so thanks for helping to clean all this up)

hongchaodeng · 2017-07-05T18:32:44Z

@diegs
Could we do it in another PR? This PR is already huge. And this PR is only about converging the paths.

diegs

I'd really just rather get this right in this PR. It's only a few more lines, and then it's done. If we punt to another PR we are going to change some of the exact same lines again.

diegs · 2017-07-05T19:13:44Z

pkg/asset/tls.go

@@ -139,7 +139,7 @@ func newEtcdTLSAssets(etcdCACert, etcdClientCert *x509.Certificate, etcdClientKe
 	}

 	assets = append(assets, []Asset{
-		{Name: AssetPathEtcdCA, Data: tlsutil.EncodeCertificatePEM(etcdCACert)},
+		{Name: AssetPathEtcdClientCA, Data: tlsutil.EncodeCertificatePEM(etcdCACert)},


Please add AssetPathEtcdServer[CA,Key,Cert] here.

Please add AssetPathEtcdPeerCA on line 136 above.

diegs · 2017-07-05T19:15:29Z

hack/multi-node/etcd-cloud-config.yaml

@@ -21,10 +21,10 @@ coreos:
          Environment="ETCD_LISTEN_PEER_URLS=https://$private_ipv4:2380"
          Environment="ETCD_INITIAL_CLUSTER={{ETCD_INITIAL_CLUSTER}}"
          Environment="ETCD_SSL_DIR=/etc/etcd/tls"
-          Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-ca.crt"
+          Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-client-ca.crt"


(and the below should be etcd-server.crt and etcd-server.key

diegs · 2017-07-05T19:15:56Z

hack/single-node/user-data-etcd.sample

@@ -12,11 +12,11 @@
            Environment="ETCD_LISTEN_CLIENT_URLS=https://0.0.0.0:2379"
            Environment="ETCD_LISTEN_PEER_URLS=https://0.0.0.0:2380"
            Environment="ETCD_SSL_DIR=/etc/etcd/tls"
-            Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-ca.crt"
+            Environment="ETCD_TRUSTED_CA_FILE=/etc/ssl/certs/etcd-client-ca.crt"


(and the two lines below)

hongchaodeng · 2017-07-05T19:44:28Z

All fixed

diegs

awesome, thanks!

xiang90 · 2017-07-05T20:57:11Z

coreosbot run e2e

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 30, 2017

hongchaodeng force-pushed the up2 branch 9 times, most recently from d8aaeae to f2f768b Compare June 30, 2017 19:11

hongchaodeng force-pushed the up2 branch from f2f768b to fc36d69 Compare June 30, 2017 19:49

s-urbaniak mentioned this pull request Jul 3, 2017

bootkube/etcd/tls: align certificates with etcd operator and bootkube coreos/tectonic-installer#1276

Merged

diegs suggested changes Jul 5, 2017

View reviewed changes

diegs reviewed Jul 5, 2017

View reviewed changes

hongchaodeng force-pushed the up2 branch from fc36d69 to b1ff341 Compare July 5, 2017 19:43

TLS: converge asset naming of SH and non-SH etcd

4a87785

hongchaodeng force-pushed the up2 branch from b1ff341 to 4a87785 Compare July 5, 2017 19:44

diegs approved these changes Jul 5, 2017

View reviewed changes

hongchaodeng merged commit ed2e94e into kubernetes-retired:master Jul 5, 2017

hongchaodeng deleted the up2 branch July 5, 2017 21:43

dghubble mentioned this pull request Jul 12, 2017

Update non-terraform Kubernetes to use bootkube v0.5.0 poseidon/matchbox#603

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TLS: converge asset naming of SH and non-SH etcd #621

TLS: converge asset naming of SH and non-SH etcd #621

hongchaodeng commented Jun 30, 2017 •

edited

Loading

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jul 5, 2017

diegs left a comment

diegs Jul 5, 2017

diegs Jul 5, 2017

diegs Jul 5, 2017

diegs Jul 5, 2017

diegs Jul 5, 2017

diegs Jul 5, 2017

diegs Jul 5, 2017

hongchaodeng commented Jul 5, 2017

diegs commented Jul 5, 2017 •

edited

Loading

hongchaodeng commented Jul 5, 2017

diegs left a comment

diegs Jul 5, 2017

diegs Jul 5, 2017

diegs Jul 5, 2017

hongchaodeng commented Jul 5, 2017

diegs left a comment

xiang90 commented Jul 5, 2017

TLS: converge asset naming of SH and non-SH etcd #621

TLS: converge asset naming of SH and non-SH etcd #621

Conversation

hongchaodeng commented Jun 30, 2017 • edited Loading

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jun 30, 2017

hongchaodeng commented Jul 5, 2017

diegs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongchaodeng commented Jul 5, 2017

diegs commented Jul 5, 2017 • edited Loading

hongchaodeng commented Jul 5, 2017

diegs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongchaodeng commented Jul 5, 2017

diegs left a comment

Choose a reason for hiding this comment

xiang90 commented Jul 5, 2017

hongchaodeng commented Jun 30, 2017 •

edited

Loading

diegs commented Jul 5, 2017 •

edited

Loading