Skip to content
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

kubelet: Move RotateCertificates to the KubeletConfiguration struct #63912

Merged
merged 2 commits into from May 23, 2018

Conversation

@luxas
Copy link
Member

luxas commented May 16, 2018

What this PR does / why we need it:
Moves .RotateCertificates to the KubeletConfiguration struct, so it can be configured via the Config file smoothly.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #63878
Fixes #61653

Special notes for your reviewer:
Pretty similar to #62352

Release note:

The kubelet certificate rotation feature can now be enabled via the `.RotateCertificates` field in the kubelet's config file. The `--rotate-certificates` flag is now deprecated, and will be removed in a future release.

@kubernetes/sig-node-pr-reviews @kubernetes/sig-cluster-lifecycle-pr-reviews

@k8s-ci-robot k8s-ci-robot requested review from derekwaynecarr and ncdc May 16, 2018

@luxas luxas added this to the v1.11 milestone May 16, 2018

@luxas luxas changed the title Move rotatecerts kubelet kubelet: Move RotateCertificates to the KubeletConfiguration struct May 16, 2018

@@ -560,7 +560,7 @@ func run(s *options.KubeletServer, kubeDeps *kubelet.Dependencies) (err error) {
}

var clientCertificateManager certificate.Manager
if s.RotateCertificates && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletClientCertificate) {
if s.KubeletConfiguration.RotateCertificates && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletClientCertificate) {

This comment has been minimized.

@mtaufen

mtaufen May 18, 2018

Contributor

Both KubeletFlags and KubeletConfiguration are inline in KubeletServer, so you shouldn't have to qualify it when you move it.

// certificate signing requests. The RotateKubeletClientCertificate feature
// must be enabled.
// Default: false
RotateCertificates bool `json:"rotateCertificates,omitempty"`

This comment has been minimized.

@mtaufen

mtaufen May 18, 2018

Contributor

add // +optional

@mtaufen

This comment has been minimized.

Copy link
Contributor

mtaufen commented May 18, 2018

couple small things otherwise lgtm

@mtaufen

This comment has been minimized.

Copy link
Contributor

mtaufen commented May 21, 2018

For the release note, how about:
Kubelet certificate rotation can now be enabled via the RotateCertificates field in the Kubelet's config file. The --rotate-certificates flag is now deprecated, and will be removed in a future release.

@mtaufen

This comment has been minimized.

Copy link
Contributor

mtaufen commented May 21, 2018

/status approved-for-milestone

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented May 21, 2018

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@luxas @mikedanese @mtaufen @tallclair

Pull Request Labels
  • sig/cluster-lifecycle sig/node: Pull Request will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/cleanup: Adding tests, refactoring, fixing old bugs.
Help

luxas added some commits May 22, 2018

@luxas luxas force-pushed the luxas:move_rotatecerts_kubelet branch from ace62a1 to 57e74f9 May 22, 2018

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 22, 2018

@mtaufen updated the PR and release notes, please LGTM :)

@mtaufen

This comment has been minimized.

Copy link
Contributor

mtaufen commented May 22, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 22, 2018

@yujuhong

This comment has been minimized.

Copy link
Member

yujuhong commented May 23, 2018

/approve
based on @mtaufen's lgtm

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 23, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: luxas, mtaufen, yujuhong

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented May 23, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@luxas

This comment has been minimized.

Copy link
Member Author

luxas commented May 23, 2018

/retest

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented May 23, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented May 23, 2018

Automatic merge from submit-queue (batch tested with PRs 59851, 64114, 63912, 64156, 64191). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 02818ed into kubernetes:master May 23, 2018

18 checks passed

Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation luxas authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

k8s-github-robot pushed a commit that referenced this pull request May 24, 2018

Kubernetes Submit Queue
Merge pull request #64187 from luxas/kubeadm_kubelet_improve_security
Automatic merge from submit-queue (batch tested with PRs 64174, 64187, 64216, 63265, 64223). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Improve the kubelet default configuration security-wise

**What this PR does / why we need it**:
 - Disables the readonly port for the kubelets in the cluster
 - Enables delegated SA token authentication for the secure kubelet port (GCE also did this ref: #58178)
 - Follows up #63912 to move the last flag from the system dropin to the ComponentConfig

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes kubernetes/kubeadm#732
Fixes kubernetes/kubeadm#650
Replaces #57997

**Special notes for your reviewer**:
In order to make sure this actually works, or that clusters actually are secure, we're adding e2e tests for this: kubernetes/kubeadm#838 & #64140
Depends on #63912

**Release note**:

```release-note
[action required] kubeadm: kubelets in kubeadm clusters now disable the readonly port (10255). If you're relying on unauthenticated access to the readonly port, please switch to using the secure port (10250). Instead, you can now use ServiceAccount tokens when talking to the secure port, which will make it easier to get access to e.g. the `/metrics` endpoint of the kubelet securely.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews 
@kubernetes/sig-auth-pr-reviews FYI

rfranzke added a commit to gardener/gardener that referenced this pull request Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.