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

Revise wording for Secret concept #27716

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Apr 25, 2021

This change is a subset of the changes from PR #24169 (a PR that has proved too big to review).

I will follow this up with further PRs to incorporate the other changes that #24169 suggests.

/sig storage

@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language labels Apr 25, 2021
@k8s-ci-robot k8s-ci-robot added sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 25, 2021
@netlify
Copy link

netlify bot commented Apr 25, 2021

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

🔨 Explore the source changes: 7a8389c

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

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

@@ -31,7 +35,10 @@ access, or anyone with access to Kubernetes' underlying data store, etcd. In
order to safely use Secrets, it is recommended you (at a minimum):

1. [Enable Encryption at Rest](/docs/tasks/administer-cluster/encrypt-data/) for Secrets.
2. [Enable or configure RBAC rules](/docs/reference/access-authn-authz/authorization/) that restrict reading and writing the Secret. Be aware that secrets can be obtained implicitly by anyone with the permission to create a Pod.
2. [Enable or configure RBAC rules](/docs/reference/access-authn-authz/authorization/) that
restrict reading and writing the Secret. Be aware that secrets can be obtained
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restrict reading from and writing to the Secret.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is a subset of the changes from PR #24169 (a PR that has proved too big to review).

I will follow this up with further PRs to incorporate the other changes that #24169 suggests. Anyone else can make those follow-ups; it doesn't have to be me.

2. [Enable or configure RBAC rules](/docs/reference/access-authn-authz/authorization/) that restrict reading and writing the Secret. Be aware that secrets can be obtained implicitly by anyone with the permission to create a Pod.
2. [Enable or configure RBAC rules](/docs/reference/access-authn-authz/authorization/) that
restrict reading and writing the Secret. Be aware that secrets can be obtained
implicitly by anyone with the permission to create a Pod.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Be aware that Secrets can be obtained.... (capitalize S)
  2. What does "can be obtained implicitly" mean? Maybe reword to "Be aware that anyone with the permission to create a pod can retrieve Secrets."?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is a subset of the changes from PR #24169 (a PR that has proved too big to review).

I will follow this up with further PRs to incorporate the other changes that #24169 suggests. Anyone else can make those follow-ups; it doesn't have to be me.

@@ -47,6 +54,10 @@ A Secret can be used with a Pod in three ways:
- As [container environment variable](#using-secrets-as-environment-variables).
- By the [kubelet when pulling images](#using-imagepullsecrets) for the Pod.

The Kubernetes control plane also uses Secrets; for example,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also uses Secrets. For example, ...

Copy link
Contributor

@shannonxtreme shannonxtreme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some suggestions for clarity and consistency :)

@kbhawkey
Copy link
Contributor

{{< glossary_tooltip term_id="pod" >}} definition or in a
{{< glossary_tooltip text="container image" term_id="image" >}}.
See [Secrets design document](https://git.k8s.io/community/contributors/design-proposals/auth/secrets.md) for more information.

A Secret is an object that contains a small amount of sensitive data such as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would help to add that a Secret is a core Kubernetes object (or something similar to that text)?
https://kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/secret-v1/#get-read-the-specified-secret

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is a subset of the changes from PR #24169 (a PR that has proved too big to review).

I will follow this up with further PRs to incorporate the other changes that #24169 suggests. Anyone else can make those follow-ups; it doesn't have to be me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's nice.

@sftim sftim force-pushed the 20210425_revise_secret_concept branch 2 times, most recently from e6ca115 to 9a946d2 Compare June 9, 2021 17:10
strings. By default they can be retrieved - as plain text - by anyone with API
access, or anyone with access to Kubernetes' underlying data store, etcd. In
order to safely use Secrets, it is recommended you (at a minimum):
Kubernetes Secrets are, by default, stored unencrypted in the API server's underlying data store (etcd). Anyone with API access can retrieve or modify a Secret, and so can anyone with access to etcd.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sftim, These changes look good.
Should these new lines be wrapped at around the same character length as the other lines?

@kbhawkey
Copy link
Contributor

@kbhawkey
Copy link
Contributor

kbhawkey commented Jul 3, 2021

/kind cleanup
/approve

@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jul 3, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kbhawkey

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 3, 2021
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2021
@sftim sftim force-pushed the 20210425_revise_secret_concept branch from 9a946d2 to 7a8389c Compare July 24, 2021 01:24
@sftim
Copy link
Contributor Author

sftim commented Jul 24, 2021

Rebased, should be ready for review.

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2021
@celestehorgan
Copy link
Contributor

I'm making an executive call here, @sftim, and choosing to treat the remaining unresolved issues as non-blocking :)

Thank you so much for your hard work on this.

/lgtm

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

LGTM label has been added.

Git tree hash: 3b21e8e7d8d96c5ffe7fec90f76716c1f148a023

@k8s-ci-robot k8s-ci-robot merged commit 0f8a345 into kubernetes:main Jul 27, 2021
@zhangguanzhang
Copy link
Contributor

@tengqm need sync to zh

@tengqm
Copy link
Contributor

tengqm commented Aug 6, 2021

@zhangguanzhang With dev-1.22 "becoming" the new "main" branch, we have a lot of pages to resync. Will initiate an sprint to get this done. Thanks for reminding.

shannonxtreme added a commit to shannonxtreme/website that referenced this pull request Nov 11, 2021
I’ve been a member for a fair bit. Reviewed, worked on, or been involved in tracking 50+ PRs.

Some highlights:

* kubernetes#26848
* kubernetes#27716
* kubernetes#27739
* kubernetes#28870
* kubernetes#28740
* kubernetes#28617

Search for PRs involving me: https://github.com/kubernetes/website/pulls?page=2&q=is%3Apr+involves%3Ashannonxtreme
trierra pushed a commit to trierra/website that referenced this pull request Nov 16, 2021
I’ve been a member for a fair bit. Reviewed, worked on, or been involved in tracking 50+ PRs.

Some highlights:

* kubernetes#26848
* kubernetes#27716
* kubernetes#27739
* kubernetes#28870
* kubernetes#28740
* kubernetes#28617

Search for PRs involving me: https://github.com/kubernetes/website/pulls?page=2&q=is%3Apr+involves%3Ashannonxtreme
trierra pushed a commit to trierra/website that referenced this pull request Nov 16, 2021
I’ve been a member for a fair bit. Reviewed, worked on, or been involved in tracking 50+ PRs.

Some highlights:

* kubernetes#26848
* kubernetes#27716
* kubernetes#27739
* kubernetes#28870
* kubernetes#28740
* kubernetes#28617

Search for PRs involving me: https://github.com/kubernetes/website/pulls?page=2&q=is%3Apr+involves%3Ashannonxtreme
trierra pushed a commit to trierra/website that referenced this pull request Nov 16, 2021
I’ve been a member for a fair bit. Reviewed, worked on, or been involved in tracking 50+ PRs.

Some highlights:

* kubernetes#26848
* kubernetes#27716
* kubernetes#27739
* kubernetes#28870
* kubernetes#28740
* kubernetes#28617

Search for PRs involving me: https://github.com/kubernetes/website/pulls?page=2&q=is%3Apr+involves%3Ashannonxtreme
trierra pushed a commit to trierra/website that referenced this pull request Nov 16, 2021
I’ve been a member for a fair bit. Reviewed, worked on, or been involved in tracking 50+ PRs.

Some highlights:

* kubernetes#26848
* kubernetes#27716
* kubernetes#27739
* kubernetes#28870
* kubernetes#28740
* kubernetes#28617

Search for PRs involving me: https://github.com/kubernetes/website/pulls?page=2&q=is%3Apr+involves%3Ashannonxtreme
kevindelgado pushed a commit to kevindelgado/kubernetes.github.io that referenced this pull request Nov 17, 2021
I’ve been a member for a fair bit. Reviewed, worked on, or been involved in tracking 50+ PRs.

Some highlights:

* kubernetes#26848
* kubernetes#27716
* kubernetes#27739
* kubernetes#28870
* kubernetes#28740
* kubernetes#28617

Search for PRs involving me: https://github.com/kubernetes/website/pulls?page=2&q=is%3Apr+involves%3Ashannonxtreme
@sftim sftim deleted the 20210425_revise_secret_concept branch December 8, 2021 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants