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

Move and revise overview for Security section #43176

Merged

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Sep 23, 2023

A fix for #36591 (fixes #36591)

We should make this change because https://kubernetes.io/docs/concepts/security/overview/ is not really giving readers a good overview of how to secure a Kubernetes cluster. That page was much better than no advice; this PR aims to improve it further by explicitly aligning with the CNCF security white paper.

/language en
/sig security

@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/security Categorizes an issue or PR as relevant to SIG Security. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 23, 2023
* [Certified Kubernetes Security Specialist](https://training.linuxfoundation.org/certification/certified-kubernetes-security-specialist/)
certification and official training course.

Read more in this section:
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 relies on simple_list: true in the front matter. See the preview.

@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Sep 23, 2023
@sftim
Copy link
Contributor Author

sftim commented Sep 23, 2023

/label refactor

@k8s-ci-robot k8s-ci-robot added the refactor Indicates a PR with large refactoring changes e.g. removes files or moves content label Sep 23, 2023
@netlify
Copy link

netlify bot commented Sep 23, 2023

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 9323995
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/65cf4b639588670008aea233
😎 Deploy Preview https://deploy-preview-43176--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sftim sftim force-pushed the 20230923_revise_security_concept_section branch from d3fea01 to 8a368b9 Compare September 23, 2023 21:08
@sftim sftim force-pushed the 20230923_revise_security_concept_section branch from 8a368b9 to d7c996a Compare October 24, 2023 09:14
Copy link
Member

@Gauravpadam Gauravpadam left a comment

Choose a reason for hiding this comment

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

Thanks,

A partial review from my end

content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved

### Observbility and runtime security

As your applications run they - and the cluster they run on - give you ways to
Copy link
Member

Choose a reason for hiding this comment

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

The statement is semantically and grammatically correct, But should we make this more clear and concise?

Suggested change
As your applications run they - and the cluster they run on - give you ways to
Kubernetes enables you to observe and monitor your applications and the clusters they are running on

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 doesn't produce a sentence that makes sense. I'll adapt it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How's this now?

@sftim sftim force-pushed the 20230923_revise_security_concept_section branch 3 times, most recently from 6c62b42 to 857eb1b Compare October 27, 2023 15:30
Copy link
Member

@aj11anuj aj11anuj left a comment

Choose a reason for hiding this comment

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

Some minor suggestions from my side

content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
## _Deploy_ lifecycle phase {#lifecycle-phase-deploy}

Ensure appropriate restrictions on what can be deployed, who can deploy it,
and where it can be deployed to.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
and where it can be deployed to.
and where it can be deployed.

Copy link
Contributor Author

@sftim sftim Oct 29, 2023

Choose a reason for hiding this comment

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

I (still) think “where it can be deployed to” is how people who speak English would say it.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to what Tim said.

content/en/docs/concepts/security/_index.md Show resolved Hide resolved
@sftim
Copy link
Contributor Author

sftim commented Oct 29, 2023

Thanks for the review @aj11anuj. However, please watch out for making suggestions that change text from good use of English to invalid or unidiomatic English.

@sftim
Copy link
Contributor Author

sftim commented Nov 4, 2023

Part of #25119

@sftim sftim force-pushed the 20230923_revise_security_concept_section branch 2 times, most recently from 08439ea to 2f45b51 Compare November 16, 2023 12:47
@sftim sftim force-pushed the 20230923_revise_security_concept_section branch 2 times, most recently from f56031d to cf22354 Compare December 7, 2023 19:56
Copy link
Contributor

@divya-mohan0209 divya-mohan0209 left a comment

Choose a reason for hiding this comment

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

Overall this LGTM. However, I am just curious if we want to link to the hardening guide for further reading?

## _Deploy_ lifecycle phase {#lifecycle-phase-deploy}

Ensure appropriate restrictions on what can be deployed, who can deploy it,
and where it can be deployed to.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to what Tim said.

content/en/docs/concepts/security/_index.md Outdated Show resolved Hide resolved
@sftim
Copy link
Contributor Author

sftim commented Dec 8, 2023

link to the hardening guide

We actually have several (some are labelled as good practice, one as hardening, another as a security checklist).

I'd love to see a follow-up land that tidies things up. See https://deploy-preview-43176--kubernetes-io-main-staging.netlify.app/docs/concepts/security/#what-s-next for a selection of what we have now.

@sftim sftim force-pushed the 20230923_revise_security_concept_section branch from f414fc6 to fc9f380 Compare February 3, 2024 17:23
@PushkarJ
Copy link
Member

PushkarJ commented Feb 3, 2024

Looks good to me now.

Hopefully one more person from sig security can review too

A key security mechanism for any Kubernetes is to
[control access to the Kubernetes API](/docs/concepts/security/controlling-access).

You can define [encryption at rest](/docs/tasks/administer-cluster/encrypt-data/)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Recommend calling out TLS first in this paragraph, since it is essential to virtually any effort to secure a kubernetes cluster. Then move to "You can also configure encryption at rest..." because it's a good idea but less critical for many common use cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(reworded)

{{< glossary_tooltip text="Containers" term_id="container" >}} provide two
things: isolation between different applications, and a mechanism to combine
those isolated applications to run on the same host computer. Those two
aspects, isolation and aggregation, mean that runtime security involves
Copy link
Member

Choose a reason for hiding this comment

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

Just wanna say that I LOVE this sentence especially.

@tabbysable
Copy link
Member

I left several nits, none of which should be interpreted as merge-blocking.

This is beautiful! Thank you for the huge amount of thoughtful work that has gone into it.

/lgtm

Please ping me to re-LGTM if you choose to address any of the nits and the LGTM thus falls off.

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

LGTM label has been added.

Git tree hash: ccf8350debc22d3f702fda57085974e2bc413e0b

Comment on lines +38 to +41
The [Secret](/docs/concepts/configuration/secret/) API provides basic protection for
configuration values that require confidentiality.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add that secrets are only base64 encoded and are stored unencrypted in the API server? (A premonition to the caution block under the secrets documentation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe you'd like to propose that for a follow-up PR?

content/en/docs/concepts/security/cloud-native-security.md Outdated Show resolved Hide resolved
You can enforce measures from the _distribute_ phase, such as verifying the
cryptographic identity of container image artefacts.

When you deploy Kubernetes, you also set the foundation for your
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
When you deploy Kubernetes, you also set the foundation for your
When you deploy Kubernetes, you also set the foundation for your

When you deploy Kubernetes seems confusing. Do mean Deploy to Kubernetes or Deploy a Kubernetes cluster?

Copy link
Contributor Author

@sftim sftim Feb 16, 2024

Choose a reason for hiding this comment

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

I mean deploy at least one Kubernetes cluster; is that not as obvious as I'd've hoped?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it makes sense. 👍

@AnshumanTripathi
Copy link
Contributor

Missed to comment lgtm

/lgtm

@k8s-ci-robot
Copy link
Contributor

@AnshumanTripathi: changing LGTM is restricted to collaborators

In response to this:

Missed to comment lgtm

/lgtm

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 k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 16, 2024
sftim and others added 4 commits February 16, 2024 11:43
Co-authored-by: Anshuman Tripathi <anshuman.tripathi305@gmail.com>
Co-authored-by: Anuj Tiwari <aj11anuj123@gmail.com>
Co-authored-by: Gaurav Padam <1032201077@tcetmumbai.in>
Co-authored-by: Tabitha Sable <51767484+tabbysable@users.noreply.github.com>
@sftim sftim force-pushed the 20230923_revise_security_concept_section branch from 9a51f85 to 9323995 Compare February 16, 2024 11:47
@sftim
Copy link
Contributor Author

sftim commented Feb 16, 2024

@tabbysable, I think this is ready for another LGTM if you're happy with it.

@PushkarJ
Copy link
Member

/lgtm

( @tabbysable hoping to save you some keystrokes )

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

LGTM label has been added.

Git tree hash: a1b1242f846cb0ea779b57876b229ff1720b9a6a

@reylejano
Copy link
Member

This PR improves the security content in the docs
Thank you
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: reylejano

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 Feb 16, 2024
@k8s-ci-robot k8s-ci-robot merged commit da14ff8 into kubernetes:main Feb 16, 2024
6 checks passed
@sftim sftim deleted the 20230923_revise_security_concept_section branch February 18, 2024 00:15
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. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. refactor Indicates a PR with large refactoring changes e.g. removes files or moves content sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/security Categorizes an issue or PR as relevant to SIG Security. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update or remove https://kubernetes.io/docs/concepts/security/overview/