Update kubelet to be secureKubelet by default #4307

audip · 2018-01-20T19:27:08Z

Turns on secureKubelet by default based on @gambol99 's work on #3125
Towards an effort to complete issue #3998

chrislovecnm · 2018-01-20T19:43:09Z

pkg/model/components/kubelet.go

@@ -60,6 +60,7 @@ func (b *KubeletOptionsBuilder) BuildOptions(o interface{}) error {
 	clusterSpec.Kubelet.ClusterDNS = ip.String()
 	clusterSpec.Kubelet.ClusterDomain = clusterSpec.ClusterDNSDomain
 	clusterSpec.Kubelet.NonMasqueradeCIDR = clusterSpec.NonMasqueradeCIDR
+	clusterSpec.Kubelet.anonymousAuth = fi.Bool(false)


should we check that it is not set first? The user may have set it to true already.

Good catch, adding the logic

chrislovecnm · 2018-01-20T19:46:39Z

/ok-to-test

CI unit tests seemed to have failed as well

chrislovecnm · 2018-01-24T16:37:34Z

/retest

@audip your code is failing CI. please run make ci locally.

/assign @gambol99

@gambol99 you understand this better than I ... thanks for your help!

chrislovecnm · 2018-01-24T16:37:58Z

/retest

audip · 2018-01-24T18:44:18Z

@chrislovecnm Thanks for re-testing this, the end-to-end test has passed and in the Travis build log:

--- FAIL: TestMinimalCloudformation (3.78s)
	integration_test.go:544: diff for key Resources.AWSAutoScalingLaunchConfigurationnodesminimalexamplecom.Properties.UserData:
		...
		  kubelet:
		    allowPrivileged: true
		+   anonymousAuth: false
		    apiServers: https://api.internal.minimal.example.com
		    babysitDaemons: true

The AnonymousAuth flag is being run as anonymousAuth which is causing the test to fail. I'm not sure where the problem lies.

gambol99 · 2018-01-26T13:32:44Z

docs/security.md

@@ -48,7 +48,7 @@ By default AnonymousAuth on the kubelet is 'on' and so communication between kub
 # In the cluster spec
 spec:
  kubelet:
-    anonymousAuth: false
+    AnonymousAuth: false


pretty sure this should remain anonymousAuth not AnonymousAuth

I hadn't looked at componentconfig.go, will revert this change.

gambol99 · 2018-01-26T13:37:53Z

pkg/model/components/kubelet.go

@@ -61,6 +61,11 @@ func (b *KubeletOptionsBuilder) BuildOptions(o interface{}) error {
 	clusterSpec.Kubelet.ClusterDomain = clusterSpec.ClusterDNSDomain
 	clusterSpec.Kubelet.NonMasqueradeCIDR = clusterSpec.NonMasqueradeCIDR

+	// Set secure kubelet on by default when the user has not set this flag
+	if clusterSpec.Kubelet.AnonymousAuth == nil {
+		clusterSpec.Kubelet.AnonymousAuth = fi.Bool(false)


Yes .. but this would force all clusters to suddenly use mutal tls on the kubelet api, so it would need to be highlighted vigorously not to surprise. I know @justinsb did some work on state to detect is new cluster or updating a cluster .. i'm not sure if this information is communicated here though ... @justinsb?

@gambol99 to see if I understand what you are saying, and thanks for the review. We would need to determine how an update would work with this. I think this is an ok change for a new cluster, but with an existing cluster, this would cause downtime. When the masters change to using TLS first during an update, not sure how the nodes would respond. @audip have you tested an upgrade?

@gambol99 we need rolling update e2e 🙊

I have not tested the upgrade yet.

gambol99 · 2018-01-26T13:43:35Z

also the TestMinimalCloudformation needs fixing .. I believe these are the files located in kops/tests/integration/create_cluster/minimal and or kops/tests/integration/update_cluster/minimal

k8s-ci-robot · 2018-01-29T18:08:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: audip
We suggest the following additional approver: gambol99

Assign the PR to them by writing /assign @gambol99 in a comment when ready.

The full list of commands accepted by this bot can be found here.

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

OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

audip · 2018-02-22T17:23:54Z

@chrislovecnm @justinsb can you point me to where I'm going wrong? I'm stuck on the tests not passing, @gambol99 gave me some direction, but still, the tests are failing.

chrislovecnm · 2018-03-25T21:22:59Z

/retest

Bump

k8s-ci-robot · 2018-03-25T21:26:49Z

@audip: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kops-bazel-test	`e4dd534`	link	`/test pull-kops-bazel-test`

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.

fejta-bot · 2018-09-08T22:39:37Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2018-10-08T23:03:01Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

fejta-bot · 2018-11-07T23:50:33Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

k8s-ci-robot · 2018-11-07T23:50:40Z

@fejta-bot: Closing this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

Update secureKubelet flag to false by default kubernetes#3998 kuberne…

49bfcc1

…tes#3125

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 20, 2018

chrislovecnm reviewed Jan 20, 2018

View reviewed changes

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 20, 2018

Add logic for securekubelet and fix typo in security.md

5508dc1

k8s-ci-robot assigned gambol99 Jan 24, 2018

gambol99 suggested changes Jan 26, 2018

View reviewed changes

Adding a few files, dont know which are needed

e4dd534

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 29, 2018

justinsb added this to the 1.9 milestone Feb 21, 2018

justinsb modified the milestones: 1.9.0, 1.10 May 26, 2018

justinsb modified the milestones: 1.10, 1.11 Jun 10, 2018

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 8, 2018

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 8, 2018

k8s-ci-robot added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Oct 8, 2018

k8s-ci-robot closed this Nov 7, 2018

jaredallard mentioned this pull request Nov 17, 2018

feat(cmd/kops/create_cluster): default to kubelet.anonymousAuth false on k8s versions >=1.10 #6091

Merged

audip deleted the feature/enable-default-securekubelet-flag branch May 5, 2020 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update kubelet to be secureKubelet by default #4307

Update kubelet to be secureKubelet by default #4307

audip commented Jan 20, 2018 •

edited

Loading

chrislovecnm Jan 20, 2018

audip Jan 20, 2018

chrislovecnm commented Jan 20, 2018

chrislovecnm commented Jan 24, 2018

chrislovecnm commented Jan 24, 2018

audip commented Jan 24, 2018

gambol99 Jan 26, 2018

audip Jan 27, 2018

gambol99 Jan 26, 2018

chrislovecnm Jan 26, 2018

audip Jan 29, 2018

gambol99 commented Jan 26, 2018

k8s-ci-robot commented Jan 29, 2018

audip commented Feb 22, 2018

chrislovecnm commented Mar 25, 2018

k8s-ci-robot commented Mar 25, 2018

fejta-bot commented Sep 8, 2018

fejta-bot commented Oct 8, 2018

fejta-bot commented Nov 7, 2018

k8s-ci-robot commented Nov 7, 2018

Update kubelet to be secureKubelet by default #4307

Update kubelet to be secureKubelet by default #4307

Conversation

audip commented Jan 20, 2018 • edited Loading

chrislovecnm Jan 20, 2018

Choose a reason for hiding this comment

audip Jan 20, 2018

Choose a reason for hiding this comment

chrislovecnm commented Jan 20, 2018

chrislovecnm commented Jan 24, 2018

chrislovecnm commented Jan 24, 2018

audip commented Jan 24, 2018

gambol99 Jan 26, 2018

Choose a reason for hiding this comment

audip Jan 27, 2018

Choose a reason for hiding this comment

gambol99 Jan 26, 2018

Choose a reason for hiding this comment

chrislovecnm Jan 26, 2018

Choose a reason for hiding this comment

audip Jan 29, 2018

Choose a reason for hiding this comment

gambol99 commented Jan 26, 2018

k8s-ci-robot commented Jan 29, 2018

audip commented Feb 22, 2018

chrislovecnm commented Mar 25, 2018

k8s-ci-robot commented Mar 25, 2018

fejta-bot commented Sep 8, 2018

fejta-bot commented Oct 8, 2018

fejta-bot commented Nov 7, 2018

k8s-ci-robot commented Nov 7, 2018

audip commented Jan 20, 2018 •

edited

Loading