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

Juju: Disable anonymous auth on kubelet #41919

Merged
merged 1 commit into from Mar 4, 2017

Conversation

Projects
None yet
8 participants
@Cynerva
Copy link
Contributor

Cynerva commented Feb 22, 2017

What this PR does / why we need it:

This disables anonymous authentication on kubelet when deployed via Juju.

I've also adjusted a few other TLS options for kubelet and kube-apiserver. The end result is that:

  1. kube-apiserver can now authenticate with kubelet
  2. kube-apiserver now verifies the integrity of kubelet

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged):

juju-solutions/bundle-canonical-kubernetes#219

Special notes for your reviewer:

This is dependent on PR #41251, where the tactics changes are being merged in separately.

Some useful pages from the documentation:

Release note:

Juju: Disable anonymous auth on kubelet
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Feb 22, 2017

Hi @Cynerva. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-reviewable

This comment has been minimized.

Copy link

k8s-reviewable commented Feb 22, 2017

This change is Reviewable

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 22, 2017

@k8s-bot ok to test

@lazypower

This comment has been minimized.

Copy link
Member

lazypower commented Feb 22, 2017

I ran some preliminary testing on this and encountered a weird race condition during the turnup.

Feb 22 21:36:08 juju-58c337-2 systemd[1]: Stopped Kubernetes API Server.
Feb 22 21:36:08 juju-58c337-2 systemd[1]: Starting Kubernetes API Server...
Feb 22 21:36:09 juju-58c337-2 kube-apiserver[13937]: F0222 21:36:09.187162   13937 universal_validation.go:84] Validate server run options failed: No --service-cluster-ip-range specified
Feb 22 21:36:09 juju-58c337-2 systemd[1]: kube-apiserver.service: Main process exited, code=exited, status=255/n/a
Feb 22 21:36:09 juju-58c337-2 systemd[1]: Failed to start Kubernetes API Server.

it looks like start_master was invoked before the auth was setup with these refactors. Attached is the unit artifact from juju-crashdump.

10.122.28.75.tar.gz

@Cynerva Cynerva force-pushed the Cynerva:gkk/kubelet-auth branch from 57cd1d6 to d1e55e2 Feb 22, 2017

@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 22, 2017

it looks like start_master was invoked before the auth was setup with these refactors.

Thanks @chuckbutler. Looks like that's the case - it's a race condition I didn't hit, my bad!

I've amended this PR with a fix for that, by adding the authentication.setup state as a prereq to start_master here.

@lazypower

This comment has been minimized.

Copy link
Member

lazypower commented Feb 23, 2017

+1 confirmed this works as expected.

Thanks for the quick turnaround on the fix @Cynerva

/approve

@eparis

This comment has been minimized.

Copy link
Member

eparis commented Feb 24, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Feb 24, 2017

@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 24, 2017

Thanks @chuckbutler and @eparis!

Looks like the tests are the only thing left, so lemme try to get those running...

@k8s-bot node e2e test this
@k8s-bot non-cri node e2e test this
@k8s-bot non-cri e2e test this

Juju: Disable anonymous auth on kubelet
Adds TLS verification between kube-apiserver and kubelet in both directions

@Cynerva Cynerva force-pushed the Cynerva:gkk/kubelet-auth branch from d1e55e2 to 27504d8 Feb 27, 2017

@k8s-merge-robot k8s-merge-robot added size/M and removed lgtm size/L labels Feb 27, 2017

@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 27, 2017

Verification failed in hack/verify-flags-underscore.py due to a conflict with #41971 - a new flag was introduced, and an exception added for us, but this PR happens to change that line of code. Oops!

I've rebased to origin/master and updated the exceptions file accordingly.

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 27, 2017

@k8s-bot gce etcd3 e2e test this

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 27, 2017

@k8s-bot cvm gce e2e test this

@marcoceppi

This comment has been minimized.

Copy link
Member

marcoceppi commented Feb 27, 2017

@k8s-bot non-cri e2e test this

@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 27, 2017

@k8s-bot gce etcd3 e2e test this

@Cynerva

This comment has been minimized.

Copy link
Contributor

Cynerva commented Feb 27, 2017

Looks like the test reruns passed. @eparis could you give this another look and /lgtm? Thanks!

@calebamiles calebamiles added this to the v1.6 milestone Mar 1, 2017

@calebamiles

This comment has been minimized.

Copy link
Member

calebamiles commented Mar 1, 2017

PR had lgtm before code freeze, added to 1.6 milestone.

cc: @ethernetdan, @kubernetes/kubernetes-release-managers

@eparis

This comment has been minimized.

Copy link
Member

eparis commented Mar 3, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Mar 3, 2017

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Mar 3, 2017

[APPROVALNOTIFIER] This PR is APPROVED

The following people have approved this PR: Cynerva, chuckbutler, eparis

Needs approval from an approver in each of these OWNERS Files:

We suggest the following people:
cc @janetkuo
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Mar 3, 2017

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

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Mar 4, 2017

Automatic merge from submit-queue (batch tested with PRs 41919, 41149, 42350, 42351, 42285)

@k8s-merge-robot k8s-merge-robot merged commit 5b8d600 into kubernetes:master Mar 4, 2017

12 of 15 checks passed

Jenkins GCI GCE e2e Build started.
Details
Jenkins kops AWS e2e Build started.
Details
Submit Queue Required Github CI test is not green: Jenkins kops AWS e2e
Details
Jenkins Bazel Build Build succeeded.
Details
Jenkins GCE Node e2e Build succeeded.
Details
Jenkins GCE e2e Build succeeded.
Details
Jenkins GCE etcd3 e2e Build succeeded.
Details
Jenkins GCI GKE smoke e2e Build succeeded.
Details
Jenkins GKE smoke e2e Build succeeded.
Details
Jenkins Kubemark GCE e2e Build succeeded.
Details
Jenkins non-CRI GCE Node e2e Build succeeded.
Details
Jenkins non-CRI GCE e2e Build succeeded.
Details
Jenkins unit/integration Build succeeded.
Details
Jenkins verification Build succeeded.
Details
cla/linuxfoundation Cynerva authorized
Details
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 4, 2017

@Cynerva: The following test(s) failed:

Test name Commit Details Rerun command
Jenkins kops AWS e2e 27504d8 link @k8s-bot kops aws e2e test this
Jenkins GCI GCE e2e 27504d8 link @k8s-bot gci gce e2e test this

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@Cynerva Cynerva deleted the Cynerva:gkk/kubelet-auth branch Mar 6, 2017

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