SSL certificates for etcd cluster. #35516

jszczepkowski · 2016-10-25T12:53:47Z

Added generation of SSL certificates for etcd cluster's internal communication.
Turned on on GCE (gci, trusty and debain).

This change is

k8s-ci-robot · 2016-10-25T13:16:04Z

Jenkins Kubemark GCE e2e failed for commit 28571bf. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-25T13:16:44Z

Jenkins GCI GCE e2e failed for commit 28571bf. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-25T13:18:04Z

Jenkins GCE e2e failed for commit 28571bf. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-25T13:19:03Z

Jenkins GCE etcd3 e2e failed for commit 28571bf. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

roberthbailey · 2016-10-25T15:35:30Z

cluster/common.sh

+                "usages": [
+                    "signing",
+                    "key encipherment",
+                    "server auth",


i don't think you should ever add client or server auth to a ca cert.

done (removed)

roberthbailey · 2016-10-25T15:37:14Z

cluster/common.sh

+  ./cfssl print-defaults csr > ca-csr.json
+  ./cfssl gencert -initca ca-csr.json | ./cfssljson -bare ca -
+
+  if [[ ! -z "${ca_key}" && ! -z "${ca_cert}" ]]; then


what about wrapping the if around the whole first block, so that we don't generate the ca if it isn't needed?

roberthbailey · 2016-10-25T15:39:02Z

cluster/gce/gci/configure-helper.sh

@@ -375,6 +375,13 @@ current-context: service-account-context
 EOF
 }

+function create-master-etcd-auth {
+  local -r auth_dir="/etc/srv/kubernetes"
+  echo "${ETCD_CA_CERT}" | base64 --decode > "${auth_dir}/etcd-ca.crt"


this doesn't appear to be optional, so anyone (like gke) who calls this script w/o setting the new fields will fail here.

roberthbailey · 2016-10-25T15:40:10Z

cluster/gce/gci/configure-helper.sh

@@ -609,7 +616,7 @@ function prepare-etcd-manifest {
  local etcd_cluster=""
  local cluster_state="new"
  for host in $(echo "${INITIAL_ETCD_CLUSTER:-${host_name}}" | tr "," "\n"); do
-    etcd_host="etcd-${host}=http://${host}:$3"
+    etcd_host="etcd-${host}=https://${host}:$3"


this looks like it sets https for every setup, regardless of HA or non-HA. should we leave explicit non-HA using localhost http, so that we don't have to have the apiserver connect insecurely to etcd?

I think we should use https in every configuration: user should always have a possibility to make his cluster HA in future. In case of cluster with one master, there will be only one etcd server, so etcd server will not communicate with its peers (so, there is no different between http and https).

jszczepkowski · 2016-10-26T10:05:41Z

Comments applied, PTAL

k8s-ci-robot · 2016-11-01T09:17:32Z

Jenkins GCE Node e2e failed for commit 7b093b58259d5624f3b24a56735080bd2002012c. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

jszczepkowski · 2016-11-01T21:58:01Z

@k8s-bot node e2e test this
issue: #35983

roberthbailey · 2016-11-01T21:57:00Z

cluster/common.sh

+
+  mkdir -p "${KUBE_TEMP}/cfssl"
+  pushd "${KUBE_TEMP}/cfssl"
+  curl -s -L -o cfssl https://pkg.cfssl.org/R1.2/cfssl_linux-amd64


This doesn't work on a mac (and from what I remember common.sh executes on the host system where you are running kube-up.sh).

Done (moved under gce directory).

roberthbailey · 2016-11-01T21:59:51Z

cluster/saltbase/salt/etcd/etcd.manifest

+{% if pillar.get('etcd_over_ssl', '').lower() == 'true' -%}
+  {% set etcd_protocol = 'https' -%}
+  {% set etcd_creds = '--peer-trusted-ca-file /etc/srv/kubernetes/etcd-ca.crt --peer-cert-file /etc/srv/kubernetes/etcd-peer.crt --peer-key-file /etc/srv/kubernetes/etcd-peer.key -peer-client-cert-auth' -%}
+{% else -%}


The else seems unnecessary

wojtek-t · 2016-11-02T08:58:57Z

cluster/gce/gci/configure-helper.sh

@@ -626,6 +635,8 @@ function prepare-etcd-manifest {
  sed -i -e "s@{{ *etcd_cluster *}}@$etcd_cluster@g" "${temp_file}"
  sed -i -e "s@{{ *storage_backend *}}@${STORAGE_BACKEND:-}@g" "${temp_file}"
  sed -i -e "s@{{ *cluster_state *}}@$cluster_state@g" "${temp_file}"
+  sed -i -e "s@{{ *etcd_protocol *}}@https@g" "${temp_file}"


It seems that with GCI we are always using https and it's not possible to use http. Is that what we want?
Do we have any performance results for this change?

We want always to secure peer communication inside etcd cluster by SSL, to prevent malicious users from changing etcd state. It shouldn't have any performance impact in case of a single node clusters, as SSL is used only for communication between peers (so, it will only affect HA deployments).

So apiserver(s) will still talk to etcd using pure http, right?

Yes, we are not changing it.

jszczepkowski · 2016-11-03T08:56:54Z

Added support for Darwin kernel.

@roberthbailey
PTAL

jszczepkowski · 2016-11-08T21:17:18Z

Comments applied, PTAL

saad-ali · 2016-11-08T21:33:22Z

Applying do-not-merge until the post-code freeze merge exception for this feature for 1.5 has been approved.

https://groups.google.com/forum/#!topic/kubernetes-milestone-burndown/DTZu98YNuUE

roberthbailey · 2016-11-08T23:05:39Z

cluster/gce/configure-vm.sh

@@ -458,6 +458,7 @@ num_nodes: $(echo "${NUM_NODES:-}" | sed -e "s/'/''/g")
 e2e_storage_test_environment: '$(echo "$E2E_STORAGE_TEST_ENVIRONMENT" | sed -e "s/'/''/g")'
 kube_uid: '$(echo "${KUBE_UID}" | sed -e "s/'/''/g")'
 initial_etcd_cluster: '$(echo "${INITIAL_ETCD_CLUSTER:-}" | sed -e "s/'/''/g")'
+etcd_over_ssl: '$(if [[ -n "${ETCD_CA_KEY:-}" && -n "${ETCD_CA_CERT:-}" && -n "${ETCD_PEER_KEY:-}" && -n "${ETCD_PEER_CERT:-}" ]]; then echo "true"; else echo "false"; fi)'


This should be an if/else block below the EOF rather than inline (like the storage backend, admission control, etc below).

jszczepkowski · 2016-11-09T12:31:21Z

I've applied the comments, run manual tests for debian OS and did some fixes for debian.

roberthbailey · 2016-11-09T21:54:53Z

lgtm

saad-ali · 2016-11-09T23:31:19Z

Applying do-not-merge until the post-code freeze merge exception for this feature for 1.5 has been approved.

https://groups.google.com/forum/#!topic/kubernetes-milestone-burndown/DTZu98YNuUE

The request for exception to 1.5 release code freeze for the "Simplify HA setup for master on GCE" feature has been approved. Adding 1.5 milestone and removing do-not-merge label.

k8s-ci-robot · 2016-11-10T13:44:47Z

Jenkins unit/integration failed for commit 777cd95eb21b844891866e15e6b9b389841088ba. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

jszczepkowski · 2016-11-10T13:54:41Z

@k8s-bot unit test this
issue #36473

Added generation of SSL certificates for etcd cluster internal communication. Turned on on gci & trusty.

k8s-github-robot · 2016-11-10T15:23:14Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2016-11-10T15:59:01Z

Automatic merge from submit-queue

jszczepkowski · 2016-11-25T15:39:43Z

Part of kubernetes/enhancements#48

googlebot added the cla: yes label Oct 25, 2016

jszczepkowski assigned roberthbailey Oct 25, 2016

jszczepkowski added this to the v1.5 milestone Oct 25, 2016

jszczepkowski added release-note-none Denotes a PR that doesn't merit a release note. area/HA labels Oct 25, 2016

k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 25, 2016

roberthbailey reviewed Oct 25, 2016

View reviewed changes

jszczepkowski force-pushed the ha-etcd-certs branch from 28571bf to 665c746 Compare October 26, 2016 10:02

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2016

jszczepkowski force-pushed the ha-etcd-certs branch from 665c746 to 7b093b5 Compare November 1, 2016 08:54

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 1, 2016

roberthbailey reviewed Nov 1, 2016

View reviewed changes

jszczepkowski force-pushed the ha-etcd-certs branch from 7b093b5 to 086bf17 Compare November 1, 2016 22:13

wojtek-t reviewed Nov 2, 2016

View reviewed changes

jszczepkowski force-pushed the ha-etcd-certs branch from 086bf17 to 2a11d2f Compare November 2, 2016 09:02

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2016

jszczepkowski force-pushed the ha-etcd-certs branch from 2a11d2f to 2f5a6a6 Compare November 2, 2016 21:53

jszczepkowski removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2016

jszczepkowski force-pushed the ha-etcd-certs branch 2 times, most recently from fcb894b to 1a713a3 Compare November 3, 2016 08:55

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2016

jszczepkowski force-pushed the ha-etcd-certs branch from 1a713a3 to 55d4a24 Compare November 7, 2016 10:17

jszczepkowski force-pushed the ha-etcd-certs branch from 4425ed1 to c5a5dfc Compare November 8, 2016 20:38

saad-ali added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 8, 2016

roberthbailey reviewed Nov 8, 2016

View reviewed changes

jszczepkowski force-pushed the ha-etcd-certs branch from c5a5dfc to cc7c11d Compare November 9, 2016 11:52

roberthbailey added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2016

saad-ali removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 9, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2016

jszczepkowski force-pushed the ha-etcd-certs branch from cc7c11d to 777cd95 Compare November 10, 2016 13:28

jszczepkowski removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2016

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2016

jszczepkowski added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2016

SSL certificates for etcd cluster.

ab7266b

Added generation of SSL certificates for etcd cluster internal communication. Turned on on gci & trusty.

jszczepkowski force-pushed the ha-etcd-certs branch from 777cd95 to ab7266b Compare November 10, 2016 14:26

jszczepkowski removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2016

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2016

jszczepkowski added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2016

k8s-github-robot merged commit a787044 into kubernetes:master Nov 10, 2016

roberthbailey mentioned this pull request Nov 28, 2016

cluster/gce/upgrade.sh fails to upgrade nodes when run from a mac #37474

Closed

jszczepkowski mentioned this pull request Jan 10, 2017

Able to quick create a HA cluster by kube-up.sh centos provider #39462

Merged

SSL certificates for etcd cluster. #35516

SSL certificates for etcd cluster. #35516

Conversation

jszczepkowski commented Oct 25, 2016 • edited

k8s-ci-robot commented Oct 25, 2016

k8s-ci-robot commented Oct 25, 2016

k8s-ci-robot commented Oct 25, 2016

k8s-ci-robot commented Oct 25, 2016

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

jszczepkowski commented Oct 26, 2016

k8s-ci-robot commented Nov 1, 2016

jszczepkowski commented Nov 1, 2016

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

jszczepkowski commented Nov 3, 2016

jszczepkowski commented Nov 8, 2016

saad-ali commented Nov 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jszczepkowski commented Nov 9, 2016

roberthbailey commented Nov 9, 2016

saad-ali commented Nov 9, 2016

k8s-ci-robot commented Nov 10, 2016

jszczepkowski commented Nov 10, 2016

k8s-github-robot commented Nov 10, 2016

k8s-github-robot commented Nov 10, 2016

jszczepkowski commented Nov 25, 2016

jszczepkowski commented Oct 25, 2016 •

edited