cloud: add secure kubernetes config (and other tweaks)

[mberhault]

fixes #16385

• add cockroachdb-statefulset-secure.yaml which requests node
  certificates from the kubernetes CA.
• yamlify cockroachdb-statefulset.yaml (from the kubernetes/kubernetes repo)
• add Azure to README
• drop the cloud scripts, they're just kubectl create and kubectl delete

The only differences between the secure and non-secure configs are:

• init-certs init container that requests certs and copies the CA
• cert volume added to the cockroachdb container
• --certs-dir being used instead of --insecure

This only brings up the cluster. Anything outside the cluster will
probably not be able to talk to it since the external addresses are not
passed to the cert. One step at a time though.

[cockroach-teamcity]

This change is 

[a-robinson]

This is very cool, thanks for working on it! A blog post on how you threw this together would probably do pretty well.

---

Reviewed 5 of 6 files at r1.
Review status: 5 of 6 files reviewed at latest revision, 8 unresolved discussions, all commit checks successful.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 109 at r1 (raw file):

The init-certs container sends a certificate signing request to the

kubernetes cluster.

Is this idempotent? i.e. what happens if you run kubectl delete cockroachdb-2 and let the statefulset recreate the pod?

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 123 at r1 (raw file):

$(POD_NAME).cockroachdb.$(POD_NAMESPACE).svc.cluster.local

This is... less than ideal. It's possible for people to specify custom DNS domains other than cluster.local (see the comment about the -domain flag on the init container above). Can you just use $(hostname -f) in this list instead?

---

cloud/kubernetes/cockroachdb-statefulset.yaml, line 83 at r1 (raw file):

        scheduler.alpha.kubernetes.io/affinity: >
            {
              "podAntiAffinity": {

Whether we want to move this and the init container annotation is debatable. It's probably fine, but moving them restricts us to only working on the most recent version of Kubernetes (1.6), whereas we could otherwise run on 1.4: #15695

Our config in the Kubernetes repo is up-to-date because they like all their examples to be on the latest hotness, but I don't know if the same tradeoffs apply here. What do you think?

---

cloud/kubernetes/minikube.sh, line 5 at r1 (raw file):

set -exuo pipefail

# Make persistent volumes and (correctly named) claims. We must create the

This file actually isn't needed anymore either, since minikube has been able to create PVs on demand for a while now.

---

cloud/kubernetes/README.md, line 67 at r1 (raw file):

[instructions provided in the Kubernetes docs](http://kubernetes.io/docs/getting-started-guides/minikube/).

Then once you have a Kubernetes cluster running on minikube, follow the steps

Ditto on this bit no longer being needed

---

cloud/kubernetes/README.md, line 104 at r1 (raw file):

Run: `kubectl create -f cockroachdb-statefulset-secure.yaml`

Each new node will request a certificate from the kubernetes CA during its initialization phase.

It might be worth mentioning that this'll happen one at a time, since if it seemed weird to you it may seem weird to others

---

cloud/kubernetes/README.md, line 106 at r1 (raw file):

Each new node will request a certificate from the kubernetes CA during its initialization phase.

You can view pending certificates and approve them using:

If auto-approval is an option, it might be worth linking to the docs section about it as an option for anyone that's just kicking the tires.

---

cloud/kubernetes/README.md, line 189 at r1 (raw file):

If running in secure mode, you want to cleanup old certificate requests:

s/you/you'll/

---

Comments from Reviewable

[a-robinson]

cc cockroachdb/docs#875 @jseldess

[mberhault]

Review status: 5 of 6 files reviewed at latest revision, 8 unresolved discussions, all commit checks successful.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 109 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

The init-certs container sends a certificate signing request to the

kubernetes cluster.

Is this idempotent? i.e. what happens if you run kubectl delete cockroachdb-2 and let the statefulset recreate the pod?

I'll test this. From what you said earlier, I take it the disk will be reused. The init containers still runs and just skips it, right? If that's the case, then as long as I don't fail overwriting the files, it should be fine.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 123 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

$(POD_NAME).cockroachdb.$(POD_NAMESPACE).svc.cluster.local

This is... less than ideal. It's possible for people to specify custom DNS domains other than cluster.local (see the comment about the -domain flag on the init container above). Can you just use $(hostname -f) in this list instead?

oh. good point. Switched over to $(hostname -f), which requires switching to a command field instead of args ($(..) is environment expansion). That also means that I need to run it inside a shell, so I call /bin/ash first (no bash in busybox)

---

cloud/kubernetes/cockroachdb-statefulset.yaml, line 83 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

Whether we want to move this and the init container annotation is debatable. It's probably fine, but moving them restricts us to only working on the most recent version of Kubernetes (1.6), whereas we could otherwise run on 1.4: #15695

Our config in the Kubernetes repo is up-to-date because they like all their examples to be on the latest hotness, but I don't know if the same tradeoffs apply here. What do you think?

We already require 1.5 for statefulsets, right? Is it that inconvenient to require 1.6? I'm happy leaving it, but I do like the clarify of the yaml format.

---

cloud/kubernetes/minikube.sh, line 5 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

This file actually isn't needed anymore either, since minikube has been able to create PVs on demand for a while now.

ok. Deleted.

---

cloud/kubernetes/README.md, line 67 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

Ditto on this bit no longer being needed

Done.

---

cloud/kubernetes/README.md, line 104 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

It might be worth mentioning that this'll happen one at a time, since if it seemed weird to you it may seem weird to others

Done.

---

cloud/kubernetes/README.md, line 106 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

If auto-approval is an option, it might be worth linking to the docs section about it as an option for anyone that's just kicking the tires.

It's not, the docs just say you can either do it manually or have a process that uses the API to do this.
What I do is just a quick back loop approving all pending csrs: for i in $(kubectl get csr|grep -w Pending|awk '{print $1}'); do kubectl certificate approve $i; done; and stick that inside another infinite loop. Doesn't seem important to include that though, I don't want to encourage people to use auto-approval.

---

cloud/kubernetes/README.md, line 189 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

s/you/you'll/

Done.

---

Comments from Reviewable

[mberhault]

Added the config files for the load generator example, one for secure and another for insecure. Small section in the README for how to run those.

---

Review status: 2 of 8 files reviewed at latest revision, 8 unresolved discussions, some commit checks pending.

---

Comments from Reviewable

[a-robinson]

Reviewed 4 of 6 files at r2.
Review status: 6 of 8 files reviewed at latest revision, 11 unresolved discussions, all commit checks successful.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 123 at r1 (raw file):

Previously, mberhault (marc) wrote…

oh. good point. Switched over to $(hostname -f), which requires switching to a command field instead of args ($(..) is environment expansion). That also means that I need to run it inside a shell, so I call /bin/ash first (no bash in busybox)

Heh, I'm not used to seeing this as /bin/ash instead of /bin/sh. Today I learned.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 20 at r2 (raw file):

    targetPort: 8080
    name: http
  # Uncomment the line below to publish the service externally.

This only works in certain environments (e.g. it won't work in minikube AFAIK), so I don't know how valuable it is to include. It may be better to just link to https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services---service-types from the README

---

cloud/kubernetes/cockroachdb-statefulset.yaml, line 83 at r1 (raw file):

Previously, mberhault (marc) wrote…

We already require 1.5 for statefulsets, right? Is it that inconvenient to require 1.6? I'm happy leaving it, but I do like the clarify of the yaml format.

You're right, we have been limited to 1.5 due to the rename. I guess I'm fine with this, anyone who needs an older version can grab it from the git history. Might be worth a disclaimer in the README/docs about which versions the configs work with, though.

---

cloud/kubernetes/example_app.yaml, line 4 at r2 (raw file):

kind: Deployment
metadata:
  name: example-secure

Looks like you forgot to remove the -secure suffix

---

cloud/kubernetes/example_app_secure.yaml, line 20 at r2 (raw file):

      # kubernetes cluster.
      # You can see pending requests using: kubectl get csr
      # CSRs can be approved using:         kubectl certificate approve <csr name>

This will require an administrator to approve a cert every time a new pod starts, right? So every time a node goes down and a pod has to move elsewhere, or every time the admin does a rolling upgrade on the deployment?

If so, I'm not sure whether this is the best model in practice, since it requires an admin to be around to handle any sort of event that causes the pods to be recreated. The alternative would be to pre-create certs and store them as Secrets which then get mounted by client pods. Let's chat about this today/tomorrow.

---

Comments from Reviewable

[mberhault]

Review status: 6 of 8 files reviewed at latest revision, 11 unresolved discussions, all commit checks successful.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 20 at r2 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

This only works in certain environments (e.g. it won't work in minikube AFAIK), so I don't know how valuable it is to include. It may be better to just link to https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services---service-types from the README

ok, removed. I think we can leave people to figure that out by themselves for now.

---

cloud/kubernetes/cockroachdb-statefulset.yaml, line 83 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

You're right, we have been limited to 1.5 due to the rename. I guess I'm fine with this, anyone who needs an older version can grab it from the git history. Might be worth a disclaimer in the README/docs about which versions the configs work with, though.

Added version information to README.

---

cloud/kubernetes/example_app.yaml, line 4 at r2 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

Looks like you forgot to remove the -secure suffix

Done.

---

cloud/kubernetes/example_app_secure.yaml, line 20 at r2 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

This will require an administrator to approve a cert every time a new pod starts, right? So every time a node goes down and a pod has to move elsewhere, or every time the admin does a rolling upgrade on the deployment?

If so, I'm not sure whether this is the best model in practice, since it requires an admin to be around to handle any sort of event that causes the pods to be recreated. The alternative would be to pre-create certs and store them as Secrets which then get mounted by client pods. Let's chat about this today/tomorrow.

Ok. I'm adding a bold warning to the readme. Going through secrets is probably the right way to go.

---

Comments from Reviewable

[smarterclayton]

I'll note that eventually it should be easier to get certs from Kube for containers and service identities - when that happens the expectation would be that someone else creates the cert for you in a secret that your pods prereference. But we're still a ways away from that.

[a-robinson]

---

Reviewed 1 of 6 files at r2, 2 of 4 files at r3, 2 of 2 files at r4.
Review status: 7 of 8 files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 109 at r1 (raw file):

Previously, mberhault (marc) wrote…

I'll test this. From what you said earlier, I take it the disk will be reused. The init containers still runs and just skips it, right? If that's the case, then as long as I don't fail overwriting the files, it should be fine.

Ever get around to testing this?

---

cloud/kubernetes/README.md, line 33 at r4 (raw file):

StatefulSets were broadly introduced in Kubernetes version 1.5. If using an
older version of Kubernetes, you may want to look at [an older version of these

Might want to remove or rework this warning now. Sorry for missing that earlier.

---

cloud/kubernetes/README.md, line 167 at r4 (raw file):

WARNING: there is not secure mode equivalent of doing this (yet).

Tiny nit, but this doesn't read well as written. Should be either "there is no secure ..." or "there is not a secure ..."

---

Comments from Reviewable

[mberhault]

Review status: 7 of 8 files reviewed at latest revision, 4 unresolved discussions.

---

cloud/kubernetes/cockroachdb-statefulset-secure.yaml, line 109 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

Ever get around to testing this?

Not yet. given the manual process involved in renewing certs, I think I'll keep my more intensive testing for after the secrets-based implementation. As is, this is mostly a proof of concept (and me learning how k8s works).

---

cloud/kubernetes/README.md, line 33 at r4 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

Might want to remove or rework this warning now. Sorry for missing that earlier.

Done. I've also removed some other parts that referred to the now-removed minikube.sh. I believe you said dynamic provisioning and stateful sets now work properly there?

---

cloud/kubernetes/README.md, line 167 at r4 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

Tiny nit, but this doesn't read well as written. Should be either "there is no secure ..." or "there is not a secure ..."

Done.

---

Comments from Reviewable

[a-robinson]

I hope you haven't been waiting on me, but if you were this still LGTM

---

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

---

Comments from Reviewable