[WIP] Rewrite Secret concept #24169

sftim · 2020-09-27T22:14:35Z

Fix bug Secrets documentation incorrectly states that kubelet/node compromise gives access to all secrets in the cluster #21435 (dated information about all nodes having access to all Secrets)
Tidy to match website style guide
Expand the overview
Use callout blocks for notes, warnings and cautions where it seems appropriate
General rewording
Page restructuring
- Move all the information security details into one place
- Consolidate details and advice about Kubernetes behavior if a Secret is missing, or if it exists but lacks a mandatory key
Trim detailed information about checking permissions on projected data
(because that level of detail belongs in a task page, not a concept page)

Preview

Note to reviewers:

this PR does not excise or migrate https://kubernetes.io/docs/concepts/configuration/secret/#use-cases. I think it would be good for a future PR / separate piece of work to look at moving that part of the page into the Tasks part of the documentation.
this PR does not switch the code samples into {{< codenew >}} blocks; it's size/XL as it is!

/language en
/sig apps
/sig security

Fixes: #21435

netlify · 2020-09-27T22:25:01Z

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

🔨 Explore the source changes: 22c5b45

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

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

kbhawkey · 2020-09-28T14:17:30Z

Related to #23825

content/en/docs/concepts/configuration/secret.md

tengqm · 2020-09-30T00:50:16Z

content/en/docs/concepts/configuration/secret.md

+or encryption keys.
+
+To use that confidential data, a Pod needs to reference the Secret in a container
+definition.


This is inaccurate. Not all Secret are referenced in container definition. Right?

Good point, this could be misleading.

nit: You could write,
However, Secrets are ...
instead of
However, Secret objects

@sftim, Have you resolved the question about lines 40-41?

Yep, I think this is addressed.

tengqm · 2020-09-30T00:53:42Z

content/en/docs/concepts/configuration/secret.md

+it becomes available, you can mark the Secret as optional in the Pod specification.  
+None of the Pod's containers will start until all non-optional Secrets are
+available.
+{{< /caution >}}


Please remove the 'caution' shortcode?

None of the Pod's containers will start until all non-optional Secrets are available.

Please revise this "none ... until all non-optional ..." sentence.

I could switch this from a caution to a note.

This is a bit of a mashup of note and warning.
You are indicating to the reader that Secrets can be declared "optional" in the Pod configuration.
Then secondly, you warn that a Pod's containers are prevented from starting until the "required" or "non-optional" Secrets are ready. Could the first sentence move out of the callout?

tengqm · 2020-09-30T00:54:25Z

content/en/docs/concepts/configuration/secret.md

-### Using Secrets as files from a Pod
+{{< caution >}}
+Secret volume sources are validated to ensure that the specified object
+reference actually points to an object of type Secret. Therefore, a secret


Suggested change

reference actually points to an object of type Secret. Therefore, a secret

reference actually points to an object of type Secret. Therefore, a Secret

tengqm · 2020-09-30T00:55:26Z

content/en/docs/concepts/configuration/secret.md

+needs to be created before any Pods that depend on it.  
+
+If the secret cannot be fetched because it does not exist or
+because of a temporary lack of connection to the API server, the kubelet


we use 'because of' for positive conditions and 'due to' for negative ones?

I'm not sure that's true, but varying the wording makes the text more pleasing anyway. I'll reword.

tengqm · 2020-09-30T00:55:51Z

content/en/docs/concepts/configuration/secret.md

+reference actually points to an object of type Secret. Therefore, a secret
+needs to be created before any Pods that depend on it.  
+
+If the secret cannot be fetched because it does not exist or


Suggested change

If the secret cannot be fetched because it does not exist or

If the Secret cannot be fetched because it does not exist or

tengqm · 2020-09-30T00:59:56Z

content/en/docs/concepts/configuration/secret.md

+available.
+{{< /caution >}}
+
+#### Using Secrets as files from a Pod


In general, I'd suggest we move these contents (with examples) about using secret into the tasks section. We already have a lot of duplicated contents on this topic.

I hope that can wait for a follow-up PR as I'd love to get these changes merged.

tengqm · 2020-09-30T01:00:45Z

content/en/docs/concepts/configuration/secret.md

+Secret volume sources are validated to ensure that the specified object
+reference actually points to an object of type Secret. Therefore, a secret
+needs to be created before any Pods that depend on it.
+{{< /note >}}


please remove the 'note' shortcode unless we have a good reason to do so.

I think this is a detail readers need to know. Is it not?

Everything documented here is something readers need to know, but that doesn't mean we need to wrap them into a note.

Fair enough!

tengqm · 2020-09-30T01:01:35Z

content/en/docs/concepts/configuration/secret.md


-#### Projection of Secret keys to specific paths
+##### Projection of Secret keys to specific paths


I don't think we are encouraged to use FIVE '#' as subsections. If we do, we have a problem with content organization.

I agree. When the content gets to heading level 5, the readability of the content decreases. That said, I need to look at the rendered page again.

we have a problem with content organization

This, I think. However I think this addition helps get this change mergeable. We can track further work to improve the updated page.

tengqm · 2020-09-30T01:02:45Z

content/en/docs/concepts/configuration/secret.md

-* `username` secret is stored under `/etc/foo/my-group/my-username` file instead of `/etc/foo/username`.
-* `password` secret is not projected.
+* the `username` key from `mysecret` is available to the container at the path
+  `/etc/foo/my-group/my-username` file instead of at `/etc/foo/username`.


at the path .... file

This reads strange.

tengqm · 2020-09-30T01:05:20Z

content/en/docs/concepts/configuration/secret.md

+{{< note >}}
+If you list keys explicitly, then all listed keys must exist in the corresponding Secret.
+Otherwise, the volume is not created.
+{{< /note >}}


Please avoid flooding the page with 'note' shortcode. We can treat all contents as noteworthy or else we won't document them.

I wonder if the answer here is a new kind of callout with a more subtle highlight color (eg gray).

I want to call this out but only a little bit.

Seems like you could make it a note or not.

Callouts are useful for unexpected warnings, subtle hints, ...
Since shortcodes are creating quite some issues for localization, I'd suggest we use them carefully.

tengqm · 2020-09-30T01:06:47Z

content/en/docs/concepts/configuration/secret.md

-Then, the secret will be mounted on `/etc/foo` and all the files created by the
-secret volume mount will have permission `0400`.
-
-Note that the JSON spec doesn't support octal notation, so use the value 256 for
-0400 permissions. If you use YAML instead of JSON for the Pod, you can use octal
-notation to specify permissions in a more natural way.
-
-Note if you `kubectl exec` into the Pod, you need to follow the symlink to find
-the expected file mode. For example,
-
-Check the secrets file mode on the pod.
-```
-kubectl exec mypod -it sh
-
-cd /etc/foo
-ls -l
-```
+The secret is mounted on `/etc/foo`; all the files created by the
+secret volume mount have permission `0400`.


Where are these content now?

Gone. This page is about Secrets and is quite long. I propose not telling readers here about how to represent POSIX file modes in JSON, because it's several steps removed from answering the question “In Kubernetes, what is a Secret?”

I agree this is not the right place to mention POSIX file modes. Somewhere we still need to call this out because it is something that may get people trapped.

Could that be on a (future) task page? I could log an issue to to that migration etc.

Logging an issue sounds an option, but the rule of thumb here is do not aimlessly drop useful information.

I've revised the wording to include the cautionary message, less verbosely.

content/en/docs/concepts/configuration/secret.md

kbhawkey · 2020-09-30T12:38:10Z

content/en/docs/concepts/configuration/secret.md

-secret.
+Kubernetes automatically creates Secrets that contain credentials for
+accessing the Kubernetes API. By default, the Kubernetes control plane automatically
+modifies newly-added Pods to use this type of Secret.


nit: double use of "automatically". Could write:
By default, the Kubernetes control plane modifies newly-added Pods to use this type of Secret.
Which type of Secret?

I think "this type of Secret" means service account Secrets.

content/en/docs/concepts/configuration/secret.md

kbhawkey · 2020-09-30T12:50:49Z

content/en/docs/concepts/configuration/secret.md

@@ -314,18 +301,20 @@ The output is similar to:
 1f2d1e2e67df
 ```

-The program in a container is responsible for reading the secrets from the
-files.
+The program in a container is responsible for reading the secret data from these


Wording: "The program" to "The application"? There are a few places in this page where "program" is used in a sentence.

Is “program” wrong? It makes sense to me.

The program in a container ...
I'd have to reread again. Which program?

A running container always has one main process (PID 1 on POSIX), and has 0 or more other processes (typically zero). Each process is running program code.

sftim · 2021-04-03T12:25:00Z

BTW @mikedanese would you like to nominate additional reviewer(s) for this concept?

k8s-ci-robot · 2021-04-03T12:25:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mbevc1
To complete the pull request process, please ask for approval from sftim after the PR has been reviewed.

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

Needs approval from an approver in each of these files:

content/en/OWNERS

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

sftim · 2021-04-03T12:35:28Z

To me, this is a rewrite rather than a revision to the Secret concept

Yep. I think it needs a rewrite. I'm open to persuasion that the existing page is already of good enough quality, but I think it would take a lot to convince me.

Rather than reviewing as a set of changes, I recommend comparing the preview to the current page.

If the new page is better than the existing one, then I recommend merging it. If not, I'd like feedback on the new page (as if we were adding a new document), so that I can understand how to improve the rewritten page.

I did not excise or migrate https://kubernetes.io/docs/concepts/configuration/secret/#use-cases. I can do, but I think that this PR is large enough without also touching that section.

@tengqm mentioned localization.
If the existing page requires improvement in English, then in my view it requires improvement in all its downstream localizations as well.

/retitle Rewrite Secret concept

sftim · 2021-04-03T12:39:30Z

/hold
This should not merge until v1.21 is released, because the merge conflict in that case would be more effort than is justified.
After v1.21 is released I can rebase this.

kbhawkey · 2021-04-03T16:57:35Z

Hi. I will read the changes again, looking at the organization of the concepts.
It is reasonable to rewrite or accept improvements to the existing content.
If there are technical questions, I'd suggest having the content reviewed by more than one member of SIG Security or SIG apps.
This is an important concept and I'd like to see this PR progress. Thanks.

sftim · 2021-04-03T17:41:56Z

Thanks for all reviews so far!

k8s-ci-robot · 2021-04-08T22:32:57Z

@sftim: PR needs rebase.

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.

jihoon-seo · 2021-09-09T04:40:58Z

/hold
This should not merge until v1.21 is released, because the merge conflict in that case would be more effort than is justified.
After v1.21 is released I can rebase this.

@sftim As of now, K8s v1.21 and v1.22 are released! 😊

sftim · 2021-09-09T13:56:50Z

My plan for this PR is to break into into smaller PRs (such as #27716) that are easier to review.

There is more to do on that front.

sftim · 2021-09-09T13:57:09Z

/retitle [WIP] Rewrite Secret concept

jimangel · 2021-12-06T04:26:13Z

@sftim this is k/website's oldest open PR. Shall we close it (I would like to see the content merge) or do you need help getting it out of WIP?

sftim · 2021-12-06T10:45:39Z

I already split out #27716 and do intend to split other PRs out of this one. I'd like to leave this (as WIP) for a bit longer, hoping to allocate time to the other changes that this larger PR wanted to make.

sftim · 2021-12-06T10:46:03Z

If anyone else wants to help with that splitting up, I shan't complain.

sftim · 2021-12-15T17:17:10Z

I'll close this and work on new PRs.

sftim · 2021-12-15T18:10:00Z

/close

k8s-ci-robot · 2021-12-15T18:10:23Z

@sftim: Closed this PR.

In response to this:

/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.

k8s-ci-robot requested review from mikedanese and steveperry-53 September 27, 2020 22:14

mbevc1 reviewed Sep 29, 2020

View reviewed changes

content/en/docs/concepts/configuration/secret.md Outdated Show resolved Hide resolved

mbevc1 reviewed Sep 29, 2020

View reviewed changes

content/en/docs/concepts/configuration/secret.md Show resolved Hide resolved

sftim commented Sep 29, 2020

View reviewed changes

content/en/docs/concepts/configuration/secret.md Show resolved Hide resolved

mbevc1 approved these changes Sep 29, 2020

View reviewed changes

tengqm reviewed Sep 30, 2020

View reviewed changes

kbhawkey reviewed Sep 30, 2020

View reviewed changes

content/en/docs/concepts/configuration/secret.md Outdated Show resolved Hide resolved

kbhawkey reviewed Sep 30, 2020

View reviewed changes

content/en/docs/concepts/configuration/secret.md Outdated Show resolved Hide resolved

kbhawkey reviewed Sep 30, 2020

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 3, 2021

sftim force-pushed the 20200927_revise_secret_concept branch from 79418fd to c8afd49 Compare April 3, 2021 12:25

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 3, 2021

Tidy & reword Secret concept

22c5b45

sftim force-pushed the 20200927_revise_secret_concept branch from c8afd49 to 22c5b45 Compare April 3, 2021 12:32

k8s-ci-robot changed the title ~~Revise Secret concept~~ Rewrite Secret concept Apr 3, 2021

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

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2021

sftim mentioned this pull request Apr 25, 2021

Revise wording for Secret concept #27716

Merged

k8s-ci-robot changed the title ~~Rewrite Secret concept~~ [WIP] Rewrite Secret concept Sep 9, 2021

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 9, 2021

k8s-ci-robot closed this Dec 15, 2021

sftim mentioned this pull request Dec 15, 2021

Tidy & reword Secret concept #30984

Merged

sftim deleted the 20200927_revise_secret_concept branch September 23, 2023 20:32

	reference actually points to an object of type Secret. Therefore, a secret
	reference actually points to an object of type Secret. Therefore, a Secret

	If the secret cannot be fetched because it does not exist or
	If the Secret cannot be fetched because it does not exist or


		#### Projection of Secret keys to specific paths
		##### Projection of Secret keys to specific paths

[WIP] Rewrite Secret concept #24169

[WIP] Rewrite Secret concept #24169

Conversation

sftim commented Sep 27, 2020 • edited Loading

netlify bot commented Sep 27, 2020 • edited Loading

kbhawkey commented Sep 28, 2020

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

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

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

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

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

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

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

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

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

kbhawkey Sep 30, 2020 • edited Loading

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

sftim commented Apr 3, 2021

k8s-ci-robot commented Apr 3, 2021

sftim commented Apr 3, 2021

sftim commented Apr 3, 2021

kbhawkey commented Apr 3, 2021

sftim commented Apr 3, 2021

k8s-ci-robot commented Apr 8, 2021

jihoon-seo commented Sep 9, 2021

sftim commented Sep 9, 2021

sftim commented Sep 9, 2021

jimangel commented Dec 6, 2021

sftim commented Dec 6, 2021

sftim commented Dec 6, 2021

sftim commented Dec 15, 2021

sftim commented Dec 15, 2021

k8s-ci-robot commented Dec 15, 2021

sftim commented Sep 27, 2020 •

edited

Loading

netlify bot commented Sep 27, 2020 •

edited

Loading

kbhawkey Sep 30, 2020 •

edited

Loading