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

Update security considerations for CSI inline ephemeral volumes #32667

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

dobsonj
Copy link
Member

@dobsonj dobsonj commented Mar 31, 2022

This PR:

  • Updates the documentation for CSI inline volumes per the KEP's Security Considerations section.
  • Removes the reference to PodSecurityPolicy for both CSI inline volumes AND Generic ephemeral volumes. PodSecurityPolicy has been deprecated since 1.21 and will be removed in 1.25.

KEP: https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/596-csi-inline-volumes
Enhancement: kubernetes/enhancements#596

@k8s-ci-robot k8s-ci-robot added this to the 1.24 milestone Mar 31, 2022
@k8s-ci-robot 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 Mar 31, 2022
@netlify
Copy link

netlify bot commented Mar 31, 2022

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit f9ba38b
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/62559f67d30bcc00086745e5

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Mar 31, 2022
@dobsonj dobsonj force-pushed the kep-596-1.24-updates branch from 312195d to 55af29e Compare March 31, 2022 22:34
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 31, 2022
@dobsonj
Copy link
Member Author

dobsonj commented Mar 31, 2022

/retitle Update security considerations for CSI inline ephemeral volumes
/cc @jsafrane

@k8s-ci-robot k8s-ci-robot requested a review from jsafrane March 31, 2022 22:43
@k8s-ci-robot k8s-ci-robot changed the title [WIP] Update security considerations for CSI inline ephemeral volumes Update security considerations for CSI inline ephemeral volumes Mar 31, 2022
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 31, 2022
@dobsonj dobsonj force-pushed the kep-596-1.24-updates branch from 55af29e to ee88864 Compare April 1, 2022 18:03
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 1, 2022
@reylejano
Copy link
Member

/sig storage

@reylejano reylejano added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Apr 4, 2022
@jsafrane
Copy link
Member

jsafrane commented Apr 5, 2022

/lgtm

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

LGTM label has been added.

Git tree hash: 80f9e32e9f020d8653a947b92ca69c0b5420e860

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.

Grammatical nits + rephrasing for clarity.

content/en/docs/concepts/storage/ephemeral-volumes.md Outdated Show resolved Hide resolved
content/en/docs/concepts/storage/ephemeral-volumes.md Outdated Show resolved Hide resolved
content/en/docs/concepts/storage/ephemeral-volumes.md Outdated Show resolved Hide resolved
@dobsonj dobsonj force-pushed the kep-596-1.24-updates branch from ee88864 to a664a29 Compare April 6, 2022 17:05
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2022
@dobsonj
Copy link
Member Author

dobsonj commented Apr 11, 2022

/assign @xing-yang @jingxu97

@jingxu97
Copy link
Contributor

/lgtm

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

LGTM label has been added.

Git tree hash: df9f8bd610519d587d15e4a079a41fc1d57f179f

@jingxu97
Copy link
Contributor

/approve

@dobsonj
Copy link
Member Author

dobsonj commented Apr 12, 2022

/assign @kbhawkey

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.

Minor nits + rephrasing for clarity.

@@ -127,14 +127,15 @@ instructions.

### CSI driver restrictions

{{< feature-state for_k8s_version="v1.21" state="deprecated" >}}
CSI ephemeral volumes allow users to provide volumeAttributes
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
CSI ephemeral volumes allow users to provide volumeAttributes
CSI ephemeral volumes allow users to provide `volumeAttributes`

Copy link
Contributor

Choose a reason for hiding this comment

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

Would we also want to link the volumeAttributes to its description at: https://kubernetes.io/docs/concepts/storage/volumes/ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would we also want to link the volumeAttributes to its description at: https://kubernetes.io/docs/concepts/storage/volumes/ ?

These attributes are driver-specific, and there is already a bit of text above this section referring to the driver-specific documentation:

The `volumeAttributes` determine what volume is prepared by the
driver. These attributes are specific to each driver and not
standardized. See the documentation of each CSI driver for further
instructions.

{{< feature-state for_k8s_version="v1.21" state="deprecated" >}}
CSI ephemeral volumes allow users to provide volumeAttributes
directly to the CSI driver as part of the Pod spec. A CSI driver
requiring volumeAttributes which are typically restricted to
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
requiring volumeAttributes which are typically restricted to
requiring `volumeAttributes`, typically restricted to

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @divya-mohan0209 I want to clarify that not all volumeAttributes are restricted to administrators. I updated the wording to (hopefully) clarify what to look for.

CSI ephemeral volumes allow users to provide volumeAttributes
directly to the CSI driver as part of the Pod spec. A CSI driver
requiring volumeAttributes which are typically restricted to
administrators is NOT suitable for use in an inline ephemeral volume.
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
administrators is NOT suitable for use in an inline ephemeral volume.
administrators, is NOT suitable for use in an inline ephemeral volume.

@dobsonj dobsonj force-pushed the kep-596-1.24-updates branch from a664a29 to e043203 Compare April 12, 2022 13:55
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 12, 2022
{{< /note >}}
Cluster administrators who need to restrict the CSI drivers that are
allowed to be used as inline volumes within a Pod spec may do so by:
- Removing `Ephemeral` from `volumeLifecycleModes` in the CSIDriver spec.
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing Ephemeral from volumeLifecycleModes in the CSIDriver spec means this CSI Driver cannot support CSI ephemeral volumes, right? Can you clarify?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's correct, I updated this line to clarify.

@dobsonj dobsonj force-pushed the kep-596-1.24-updates branch from e043203 to f9ba38b Compare April 12, 2022 15:48
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 12, 2022
@xing-yang
Copy link
Contributor

/lgtm

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

LGTM label has been added.

Git tree hash: 53e6aaf62be28281b957fa505197c7940c117c4c

@tengqm
Copy link
Contributor

tengqm commented Apr 13, 2022

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingxu97, tengqm

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 Apr 13, 2022
@k8s-ci-robot k8s-ci-robot merged commit f80cf4d into kubernetes:dev-1.24 Apr 13, 2022
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. 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.

9 participants