Add file paths to keys and certificates #28367

chrisnegus · 2021-06-10T16:54:48Z

The https://kubernetes.io/docs/setup/best-practices/certificates/ page provides excellent information on key and certificate files needed by Kubernetes. I felt it would help make it easier to consume that information if there were visual representations of the files contained in tables. So I added three tree outputs to illustrate:

Files needed for someone providing a single root CA from which all other files are automatically generated
All files required if you were generating all certs yourself
Which files contain user account certificates

This content is meant to address this issue: #28291

netlify · 2021-06-10T17:10:18Z

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: 0320d63

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/615f48d994915a0007ee34db

😎 Browse the preview: https://deploy-preview-28367--kubernetes-io-main-staging.netlify.app

chrisnegus · 2021-06-10T17:19:54Z

It looks like I need to crop the images better. I'll fix them and do a new commit.

fabriziopandini · 2021-06-10T17:34:23Z

/cc

chrisnegus · 2021-06-10T17:56:35Z

Preview page: https://deploy-preview-28367--kubernetes-io-master-staging.netlify.app/docs/setup/best-practices/certificates/
The images are cropped now the way I intended, so it is ready for review.

fabriziopandini

Thanks for this PR.
I left some comments, most of them considerations about the graphic format; feel free to ask for more opinions (opinion on graphs are usually very personal 😄)

content/en/docs/setup/best-practices/certificates.md

neolit123 · 2021-06-11T14:34:25Z

@kubernetes/sig-auth-pr-reviews
this document is owned by a group called SIG Auth.

/hold for their review

sftim · 2021-06-14T17:19:09Z

Would it be reasonable to mark this as a work-in-progress @chrisnegus ?

I'm keen on adding diagrams; we're also keen to get the content and style of those diagrams right (content more than style, if I'm honest).

chrisnegus · 2021-06-14T18:00:52Z

@sftim My original thought was just to have an easy way to visualize the locations of the files described on this page. I went to some trouble creating the diagrams (even trying to match the Kubernetes color scheme). But the feedback from @fabriziopandini and others I've shown them to is that they needed to be simpler. I'm thinking of reducing this PR to tree output of the files described, as suggested, and leaving it at that. Perhaps we can get agreement on that fairly quickly without making it a WIP.

chrisnegus · 2021-06-21T15:15:03Z

When the branch changed to main, the netlify output moved as well. You can view the PR changes here: https://deploy-preview-28367--kubernetes-io-main-staging.netlify.app/docs/setup/best-practices/certificates/

jimangel · 2021-07-13T18:15:17Z

/lgtm
/approve

I see this change being minimal, @neolit123 do you think it requires a hold for @kubernetes/sig-auth-pr-reviews? Reverting or correcting the change seems to be similar low impact.

k8s-ci-robot · 2021-07-13T18:15:25Z

LGTM label has been added.

Git tree hash: bb026f4a66ae0039616dfc036427e7eb13126bc2

deads2k · 2021-08-02T12:30:32Z

If more is needed to describe that (like pointing to which keys and certs are which)

Yes, it is this. Once you have divided the signers, you have to be sure the correct cert/key pairs are being signed and that the CAs match properly.

sftim · 2021-09-24T23:31:33Z

Hi @chrisnegus

What are your intentions with this PR? Would you like to get help from SIG Auth on revising this?

chrisnegus · 2021-09-30T20:29:48Z

What are your intentions with this PR? Would you like to get help from SIG Auth on revising this?

It seems that the consensus is to change this visualization to a full path file view that can be copied and pasted. So I did that. If it not ready to merge at this point, I suggest we abandon the PR and move on to something else.

sftim · 2021-09-30T20:47:09Z

These changes seem reasonable, they don't seem misleading. #28367 (comment) mentioned that “the locations are kubeadm specific” and I think that's still true. I also think that's fine given the added context that these are just examples.

I'd still like a review on this from SIG Auth (@kubernetes/sig-auth-misc FYI) - see also #28367 (comment)

chrisnegus · 2021-09-30T22:31:31Z

These changes seem reasonable, they don't seem misleading. #28367 (comment) mentioned that “the locations are kubeadm specific” and I think that's still true. I also think that's fine given the added context that these are just examples.

I'd still like a review on this from SIG Auth (@kubernetes/sig-auth-misc FYI) - see also #28367 (comment)

I'd be glad to open a separate issue to address the concerns from #28367 (comment). Should we have @deads2k look at this again, with the understanding that his comment about separating CAs that sign server and client certificates could be addressed in another issue?

kbhawkey · 2021-10-02T00:22:45Z

Page preview:
https://deploy-preview-28367--kubernetes-io-main-staging.netlify.app/docs/setup/best-practices/certificates/

content/en/docs/setup/best-practices/certificates.md

kbhawkey · 2021-10-02T00:49:53Z

content/en/docs/setup/best-practices/certificates.md

@@ -162,4 +198,11 @@ These files are used as follows:
 | controller-manager.conf | kube-controller-manager | Must be added to manifest in `manifests/kube-controller-manager.yaml` |
 | scheduler.conf          | kube-scheduler          | Must be added to manifest in `manifests/kube-scheduler.yaml`          |

+The following illustrates the files listed in the previous table that contain client user certificates and keys:


nit: The following ?

Okay. I updated it to say: "The following files illustrate full paths to the files listed in the previous table:"

kbhawkey · 2021-10-02T00:54:37Z

👋 @chrisnegus .
Are you waiting for a technical review or other feedback?

chrisnegus · 2021-10-07T19:23:31Z

👋 @chrisnegus . Are you waiting for a technical review or other feedback?

I don't think any further review is needed since the technical reviewer has already agreed that simply having listings from the files illustrated in the tables was okay. There was one other, separate issue raised. But I'm going to add that to a separate issue. Once that is done, I'll note the new issue here. I would be happy if this could be merged.

chrisnegus · 2021-10-07T19:58:26Z

I added Issue 29962 to request the change described in the comment above

neolit123

overall LGTM. this reflects the current state of kubeadm. as noted in the Certificate paths section the examples in this page seem to cover kubeadm mostly.

suggested to move https://github.com/kubernetes/website/issues/29962 to a k/kubeadm issue instead.

kbhawkey · 2021-10-08T00:39:17Z

/retitle Add file paths to keys and certificates

kbhawkey · 2021-10-08T00:39:33Z

/hold cancel

kbhawkey · 2021-10-08T00:39:47Z

/label tide/merge-method-squash

kbhawkey · 2021-10-08T00:41:06Z

Thanks @chrisnegus .
/lgtm

k8s-ci-robot · 2021-10-08T00:41:22Z

LGTM label has been added.

Git tree hash: 491c160d65e8ff86d7063891ebda994affa26408

Adding diagrams to certificates page

cef12bd

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 10, 2021

k8s-ci-robot requested review from celestehorgan and tengqm June 10, 2021 16:55

k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jun 10, 2021

k8s-ci-robot requested a review from fabriziopandini June 10, 2021 17:34

Cropped diagrams

0999d85

fabriziopandini reviewed Jun 11, 2021

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels Jun 11, 2021

Changed diagrams to tree output

e048a1c

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 14, 2021

Formatting fix

3a63602

chrisnegus mentioned this pull request Jul 13, 2021

Add content to https://kubernetes.io/docs/setup/best-practices/certificates/ #28291

Closed

k8s-ci-robot assigned jimangel Jul 13, 2021

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

Changed tree view of files to full-path view

6aa6b6a

Changed order of two cert files

a502850

kbhawkey reviewed Oct 2, 2021

View reviewed changes

content/en/docs/setup/best-practices/certificates.md Outdated Show resolved Hide resolved

kbhawkey reviewed Oct 2, 2021

View reviewed changes

content/en/docs/setup/best-practices/certificates.md Outdated Show resolved Hide resolved

kbhawkey reviewed Oct 2, 2021

View reviewed changes

chrisnegus added 2 commits October 7, 2021 16:27

Broke up links into separate sentences, per review comment

061144c

More changes per review comments

0320d63

neolit123 reviewed Oct 7, 2021

View reviewed changes

chrisnegus mentioned this pull request Oct 7, 2021

Creating separate CAs for client and server certificates kubernetes/kubeadm#2586

Open

k8s-ci-robot changed the title ~~Adding diagrams to certificates page~~ Add file paths to keys and certificates Oct 8, 2021

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 8, 2021

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 8, 2021

k8s-ci-robot assigned kbhawkey Oct 8, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 8, 2021

k8s-ci-robot merged commit 2a84b55 into kubernetes:main Oct 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add file paths to keys and certificates #28367

Add file paths to keys and certificates #28367

chrisnegus commented Jun 10, 2021 •

edited

Loading

netlify bot commented Jun 10, 2021 •

edited

Loading

chrisnegus commented Jun 10, 2021

fabriziopandini commented Jun 10, 2021

chrisnegus commented Jun 10, 2021 •

edited

Loading

fabriziopandini left a comment

neolit123 commented Jun 11, 2021

sftim commented Jun 14, 2021

chrisnegus commented Jun 14, 2021 •

edited

Loading

chrisnegus commented Jun 21, 2021

jimangel commented Jul 13, 2021

k8s-ci-robot commented Jul 13, 2021

deads2k commented Aug 2, 2021

sftim commented Sep 24, 2021

chrisnegus commented Sep 30, 2021

sftim commented Sep 30, 2021

chrisnegus commented Sep 30, 2021

kbhawkey commented Oct 2, 2021

kbhawkey Oct 2, 2021

chrisnegus Oct 7, 2021

kbhawkey commented Oct 2, 2021

chrisnegus commented Oct 7, 2021

chrisnegus commented Oct 7, 2021

neolit123 left a comment

kbhawkey commented Oct 8, 2021

kbhawkey commented Oct 8, 2021

kbhawkey commented Oct 8, 2021

kbhawkey commented Oct 8, 2021

k8s-ci-robot commented Oct 8, 2021

Add file paths to keys and certificates #28367

Add file paths to keys and certificates #28367

Conversation

chrisnegus commented Jun 10, 2021 • edited Loading

netlify bot commented Jun 10, 2021 • edited Loading

chrisnegus commented Jun 10, 2021

fabriziopandini commented Jun 10, 2021

chrisnegus commented Jun 10, 2021 • edited Loading

fabriziopandini left a comment

Choose a reason for hiding this comment

neolit123 commented Jun 11, 2021

sftim commented Jun 14, 2021

chrisnegus commented Jun 14, 2021 • edited Loading

chrisnegus commented Jun 21, 2021

jimangel commented Jul 13, 2021

k8s-ci-robot commented Jul 13, 2021

deads2k commented Aug 2, 2021

sftim commented Sep 24, 2021

chrisnegus commented Sep 30, 2021

sftim commented Sep 30, 2021

chrisnegus commented Sep 30, 2021

kbhawkey commented Oct 2, 2021

kbhawkey Oct 2, 2021

Choose a reason for hiding this comment

chrisnegus Oct 7, 2021

Choose a reason for hiding this comment

kbhawkey commented Oct 2, 2021

chrisnegus commented Oct 7, 2021

chrisnegus commented Oct 7, 2021

neolit123 left a comment

Choose a reason for hiding this comment

kbhawkey commented Oct 8, 2021

kbhawkey commented Oct 8, 2021

kbhawkey commented Oct 8, 2021

kbhawkey commented Oct 8, 2021

k8s-ci-robot commented Oct 8, 2021

chrisnegus commented Jun 10, 2021 •

edited

Loading

netlify bot commented Jun 10, 2021 •

edited

Loading

chrisnegus commented Jun 10, 2021 •

edited

Loading

chrisnegus commented Jun 14, 2021 •

edited

Loading