kubeadm: Mount additional paths inside apiserver/controller-manager for working CA root #59122

klausenbusk · 2018-01-31T17:24:03Z

This is required for a working CA root, as /etc/ssl/certs on a few
Linux distributions just contains a bunch of symlinks.
Container Linux and Debian have symlinks pointing to
/usr/share/ca-certificates, ArchLinux has symlinks pointing
to /etc/ca-certificates.
On Debian /etc/ssl/certs can also include symlinks pointing
to /usr/local/share/ca-certificates for local CA certificates.

Fix: kubeadm/#671

What this PR does / why we need it:

Without this PR, controller-manager and apiserver would lack a CA root on some Linux distro (ex: Container Linux) which for example break flexplugins which require a CA root [1].

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#671

Special notes for your reviewer:

Release note:

Mount additional paths required for a working CA root, for setups where /etc/ssl/certs doesn't contains certificates but just symlink.

/sig sig-kubeadm

klausenbusk · 2018-02-07T13:12:51Z

/assign @krousey

dixudx · 2018-02-13T06:23:49Z

/area kubeadm

k8s-ci-robot · 2018-02-13T06:23:51Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

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.

dixudx

In your case, you can try to use config file to add extra volumes. Please refer to Using kubeadm init with a configuration file.

dixudx · 2018-02-13T06:36:07Z

cmd/kubeadm/app/phases/controlplane/volumes.go

+// caCertsExtraVolumePath specifies the paths that can be conditionally mounted into the apiserver and controller-manager containers
+// as /etc/ssl/certs might be or contain a symlink to them. It's a variable since it may be changed in unit testing. This var MUST
+// NOT be changed in normal codepaths during runtime.
+var caCertsExtraVolumePaths = []string{"/etc/pki", "/usr/share/ca-certificates"}


I don't think we should include this.

Dunno, it is required at least on Container Linux if you want a working CA root. Maybe we should just mention it on the Install kubeadm page?

I don't see any issue with adding this as a stopgap for now, since we are already special casing /etc/pki.

jdumars · 2018-03-02T16:57:01Z

/ok-to-test

klausenbusk · 2018-03-02T20:51:55Z

/retest

klausenbusk · 2018-03-03T20:14:09Z

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

I think the failed test is unrelated to this PR.

klausenbusk · 2018-03-03T20:19:52Z

I think the failed test is unrelated to this PR.

The test was fixed in master ~ 22 hours ago (#60730). I have just rebased the PR and force pushed.

timothysc · 2018-04-13T15:03:43Z

/cc @detiber @stealthybox - want to quick review.

detiber · 2018-04-13T18:33:59Z

cmd/kubeadm/app/phases/controlplane/volumes.go

+// caCertsExtraVolumePath specifies the paths that can be conditionally mounted into the apiserver and controller-manager containers
+// as /etc/ssl/certs might be or contain a symlink to them. It's a variable since it may be changed in unit testing. This var MUST
+// NOT be changed in normal codepaths during runtime.
+var caCertsExtraVolumePaths = []string{"/etc/pki", "/usr/share/ca-certificates"}


I don't see any issue with adding this as a stopgap for now, since we are already special casing /etc/pki.

detiber · 2018-04-13T18:38:30Z

cmd/kubeadm/app/phases/controlplane/volumes.go

-// as /etc/ssl/certs might be a symlink to it. It's a variable since it may be changed in unit testing. This var MUST NOT be changed
-// in normal codepaths during runtime.
-var caCertsPkiVolumePath = "/etc/pki"
+// caCertsExtraVolumePath specifies the paths that can be conditionally mounted into the apiserver and controller-manager containers


Comment needs to be updated to caCertsExtraVolumePaths

detiber

Outside of the minor comments, this looks okay to me as a bandaid fix.

klausenbusk · 2018-04-13T19:04:29Z

@detiber Good catch :) I have fixed the comments, rebased and squashed the commits.

klausenbusk · 2018-04-13T20:36:24Z

/test pull-kubernetes-e2e-kops-aws

detiber · 2018-04-13T21:07:43Z

/lgtm

stealthybox · 2018-04-13T21:47:40Z

cmd/kubeadm/app/phases/controlplane/volumes.go

+// caCertsExtraVolumePaths specifies the paths that can be conditionally mounted into the apiserver and controller-manager containers
+// as /etc/ssl/certs might be or contain a symlink to them. It's a variable since it may be changed in unit testing. This var MUST
+// NOT be changed in normal codepaths during runtime.
+var caCertsExtraVolumePaths = []string{"/etc/pki", "/usr/share/ca-certificates"}


Do we want to address the other paths for OS's mentioned in kubernetes/kubeadm#671 (comment) ?

Cc @klausenbusk

Sorry about the late response, I will add the extra paths when I get home. :)

timothysc

/approve

@stealthybox feel free to lgtm when you think the changes are ready.

…or working CA root This is required for a working CA root, as /etc/ssl/certs on a few Linux distributions just contains a bunch of symlinks. Container Linux and Debian have symlinks pointing to /usr/share/ca-certificates, ArchLinux has symlinks pointing to /etc/ca-certificates. On Debian /etc/ssl/certs can also include symlinks pointing to /usr/local/share/ca-certificates for local CA certificates. Fix: kubeadm/#671

timothysc · 2018-04-20T22:20:53Z

@stealthybox @detiber - requires re-eval b/c of PR change.

detiber · 2018-04-23T15:31:36Z

@stealthybox I do not believe that this will conflict with the other changes in flight, lgtm

timothysc · 2018-04-24T16:42:30Z

/lgtm

k8s-ci-robot · 2018-04-24T16:42:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: detiber, klausenbusk, timothysc

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:

~~cmd/kubeadm/OWNERS~~ [timothysc]

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

klausenbusk · 2018-04-24T19:55:57Z

/test pull-kubernetes-node-e2e

Looks unrelated: There was a problem refreshing your current auth tokens: Unable to find the server at accounts.google.com

k8s-github-robot · 2018-04-24T20:28:19Z

Automatic merge from submit-queue (batch tested with PRs 62655, 61711, 59122, 62853, 62390). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 31, 2018

k8s-ci-robot requested review from luxas and mattmoyer January 31, 2018 17:24

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

klausenbusk mentioned this pull request Jan 31, 2018

CoreOS: Mount /usr/share/ca-certificates inside kube-controller-manager kubernetes/kubeadm#671

Closed

k8s-ci-robot assigned krousey Feb 7, 2018

k8s-ci-robot added the area/kubeadm label Feb 13, 2018

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 13, 2018

dixudx suggested changes Feb 13, 2018

View reviewed changes

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 2, 2018

timothysc assigned timothysc and unassigned krousey Apr 13, 2018

timothysc added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Apr 13, 2018

detiber reviewed Apr 13, 2018

View reviewed changes

detiber suggested changes Apr 13, 2018

View reviewed changes

k8s-ci-robot assigned detiber Apr 13, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 13, 2018

stealthybox reviewed Apr 13, 2018

View reviewed changes

timothysc removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2018

timothysc approved these changes Apr 18, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 18, 2018

klausenbusk changed the title ~~kubeadm: Mount /usr/share/ca-certificates inside apiserver/controller…~~ kubeadm: Mount additional paths inside apiserver/controller-manager for working CA root Apr 18, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 24, 2018

k8s-github-robot merged commit f388fcb into kubernetes:master Apr 24, 2018

This was referenced Aug 28, 2020

Add mount for KCM required for certain OS gardener/gardener#2789

Closed

Add kcm mounts when not using csi gardener/gardener-extension-provider-aws#173

Merged

maxime1907 mentioned this pull request Feb 13, 2023

fix: correctly mount ssl ca directories kubernetes-sigs/kubespray#9794

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubeadm: Mount additional paths inside apiserver/controller-manager for working CA root #59122

kubeadm: Mount additional paths inside apiserver/controller-manager for working CA root #59122

klausenbusk commented Jan 31, 2018 •

edited

Loading

klausenbusk commented Feb 7, 2018

dixudx commented Feb 13, 2018

k8s-ci-robot commented Feb 13, 2018

dixudx left a comment

dixudx Feb 13, 2018

klausenbusk Feb 13, 2018

detiber Apr 13, 2018

jdumars commented Mar 2, 2018

klausenbusk commented Mar 2, 2018

klausenbusk commented Mar 3, 2018

klausenbusk commented Mar 3, 2018

timothysc commented Apr 13, 2018

detiber Apr 13, 2018

detiber Apr 13, 2018

detiber left a comment

klausenbusk commented Apr 13, 2018

klausenbusk commented Apr 13, 2018

detiber commented Apr 13, 2018

stealthybox Apr 13, 2018

stealthybox Apr 18, 2018

klausenbusk Apr 18, 2018

klausenbusk Apr 18, 2018

timothysc left a comment

timothysc commented Apr 20, 2018

detiber commented Apr 23, 2018

timothysc commented Apr 24, 2018

k8s-ci-robot commented Apr 24, 2018

klausenbusk commented Apr 24, 2018

k8s-github-robot commented Apr 24, 2018

kubeadm: Mount additional paths inside apiserver/controller-manager for working CA root #59122

kubeadm: Mount additional paths inside apiserver/controller-manager for working CA root #59122

Conversation

klausenbusk commented Jan 31, 2018 • edited Loading

klausenbusk commented Feb 7, 2018

dixudx commented Feb 13, 2018

k8s-ci-robot commented Feb 13, 2018

dixudx left a 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

jdumars commented Mar 2, 2018

klausenbusk commented Mar 2, 2018

klausenbusk commented Mar 3, 2018

klausenbusk commented Mar 3, 2018

timothysc commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

detiber left a comment

Choose a reason for hiding this comment

klausenbusk commented Apr 13, 2018

klausenbusk commented Apr 13, 2018

detiber commented Apr 13, 2018

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

timothysc left a comment

Choose a reason for hiding this comment

timothysc commented Apr 20, 2018

detiber commented Apr 23, 2018

timothysc commented Apr 24, 2018

k8s-ci-robot commented Apr 24, 2018

klausenbusk commented Apr 24, 2018

k8s-github-robot commented Apr 24, 2018

klausenbusk commented Jan 31, 2018 •

edited

Loading