From 160f0b707fbd09eec0ca7e9850e30413bf16ec9b Mon Sep 17 00:00:00 2001 From: Abhishek Singh Baghel Date: Wed, 7 Apr 2021 20:01:09 +0000 Subject: [PATCH 1/6] feat: option to preserve default file mode bit in atomicwriter volumes --- .../README.md | 170 ++++++++++++++++++ .../kep.yaml | 29 +++ 2 files changed, 199 insertions(+) create mode 100644 keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md create mode 100644 keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/kep.yaml diff --git a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md new file mode 100644 index 00000000000..757ebce34ed --- /dev/null +++ b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md @@ -0,0 +1,170 @@ +# Option to Preserve default file mode bit (Mode\DefaultMode) set in AtomicWriter volumes + +## Table of Contents + + +- [Release Signoff Checklist](#release-signoff-checklist) +- [Summary](#summary) +- [Motivation](#motivation) +- [Proposal](#proposal) + - [Preserve DefaultMode Flag](#preserve-defaultMode-flag) + - [File Permission](#file-permission) + - [Proposed heuristics](#proposed-heuristics) + - [Alternatives considered](#alternatives-considered) + - [Scope Of the Change](#scope-of-the-change) +- [Graduation Criteria](#graduation-criteria) + + +## Release Signoff Checklist + +- [x] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR) +- [ ] (R) KEP approvers have approved the KEP status as `implementable` +- [ ] (R) Design details are appropriately documented +- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input +- [ ] (R) Graduation criteria is in place +- [ ] (R) Production readiness review completed +- [ ] Production readiness review approved +- [ ] "Implementation History" section is up-to-date for milestone +- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] +- [ ] Supporting documentation e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes + +## Summary + +Kubernetes allows users to use Secret (and other configs) as a VolumeSource +in pod spec. It also allows users to set a default file mode at the secret +or every file level. +The security context set for the pod (runAsUser or fsGroup) conflicts with +default mode specified for the secret\file. In current implementation of the +AtomicWriter volumes, default file mode is overwritten by the runAsUser or fsGroup +bit while setting the volume ownership. + +Above results in unexpected behavior in cases where users want to run multiple +containers in a single pod and want them to run under a fsGroup, have different UIDs etc. +and yet have the ability to share same secret\config volume across containers. + +This proposal proposes to provide users with ability to preserve the file Mode\DefaultMode bit set on the +AtomicWriter volumes (Secret, ConfigMap etc) in an opt-in and backward compatible manner. + +## Motivation + +Many workloads running on Kubernetes today need the ability to run multiple containers +in the same pod. Primitives such as main, sidecars and init containers form the core +of distributed application development in kubernetes and are very widely accepted\used patterns. + +Some workloads may want to run multiple containers in a single pod, under different UIDs and +have the ability to access a shared\common secret or config volume. + +As a platform, Kubernetes should evolve to allow the sharing of AtomicWriter volumes (secret, config etc.) +accross containers running under different UIDs as a first party scenario. + +With this feature, we try to provide a backwards compatible way for the users to +share a AtomicWriter volumes accross containers running under different UIDs. + +We intend to provide a way for the users to opt-in for the new behavior and expect not to break +any existing applications\configurations. + +## Proposal + +Kubernetes should implement a new boolean flag (PreserveDefaultMode) in the SecretVolumeSource and +other AtomicWriter volumes. + +The flag should be optional and default to false in code. This flag should be honored at the time of volume +set-up and call to SetVolumeOwnership should be skipped in case PreserveDefaultMode=true. + +### Preserve DefaultMode Flag + +A new PreserveDefaultMode boolean will be implemented in SecretVolumeSource and other AtomicWriter volumes. + +```go +// SecretVolumeSource adapts a Secret into a volume. +// +// The contents of the target Secret's Data field will be presented in a volume +// as files using the keys in the Data field as the file names. +// Secret volumes support ownership management and SELinux relabeling. +type SecretVolumeSource struct { + // Name of the secret in the pod's namespace to use. + // +optional + SecretName string + // If unspecified, each key-value pair in the Data field of the referenced + // Secret will be projected into the volume as a file whose name is the + // key and content is the value. If specified, the listed keys will be + // projected into the specified paths, and unlisted keys will not be + // present. If a key is specified which is not present in the Secret, + // the volume setup will error unless it is marked optional. Paths must be + // relative and may not contain the '..' path or start with '..'. + // +optional + Items []KeyToPath + // Mode bits to use on created files by default. Must be a value between + // 0 and 0777. + // Directories within the path are not affected by this setting. + // This might be in conflict with other options that affect the file + // mode, like fsGroup, and the result can be other mode bits set. + // If PreserveDefaultMode is set to true then file mode bit is not + // modified after the file creation, due to any other options set + // such as fsGroup etc + // +optional + DefaultMode *int32 + // Specify whether the Secret or its key must be defined + // +optional + Optional *bool + // Specify whether we want to preserve the DefaultMode set on the files + // at the time of file creation. + // If PreserveDefaultMode is set to true the file mode is not modified based + // on fsGroup after the file is created with the defaultMode. + // +optional + PreserveDefaultMode *bool +} +``` + +A secret volume source with preserveDefaultMode=true: + +```yaml + volumes: + - name: test-secret + secret: + preserveDefaultMode: true + defaultMode: 256 + secretName: test-secret +``` + +### File Permission + +For the volumes that use AtomicWriter today the files are created with the Mode set in the KeyToPath struct. +If the Mode is not specified then the DefaultMode set for the source is used. +If DefaultMode is not specified then the file is created with mode bit set to 0600. +If pod have fsGroup specified then all the files in the volume are walked, +chown and chmod to the fsGroup + +#### Proposed heuristics + +- *Case 1*: The volume has PreserveDefaultMode set to false. + In this case there is no behavioural change in how the file permissions are + set for the volume. In other words, we will continue to create the files + with Mode\Default and then set the volume ownership based on fsGroup if specified. + This is the default case. +- *Case 2*: The volume has PreserveDefaultMode set to true. + In this case the payload\volume\files are created using Mode\Default mode. + Then the call to SetVolumeOwnership is skipped, in turn preserving the Default file mode on the files. + +#### Alternatives considered + +- Instead of having PreserveDefaultMode flag at the volume source level, we can have the same flag in + the KeyToPath struct. That will allow us granular control over every file with-in the volume : + - We would have to pass the payload information along with PreserveDefaultMode to the SetVolumeOwnership routine + and skip chown\chmod if PreserveDefaultMode is set to true for the file. + - This will be more invasive change, but would allow more granular control at per file level + - If users want to preserve Mode for one file in the volume and not for another they can acheive that by creating + two different volumes, with PreserveDefaultMode set to true for the volume where they want to preserve default mode for all files + +### Scope Of the Change + +With this change we will try to update the behavior for below AtomicWriter volumes + +- SecretVolumeSource +- DownwardAPIVolumeSource +- ConfigMapVolumeSource +- ProjectedVolumeSource + +## Graduation Criteria + +TBD diff --git a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/kep.yaml b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/kep.yaml new file mode 100644 index 00000000000..51466fbf1b7 --- /dev/null +++ b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/kep.yaml @@ -0,0 +1,29 @@ +title: Option to Preserve default file mode bit (Mode\DefaultMode) set in AtomicWriter volumes +kep-number: 2605 +authors: + - "@abhisheksinghbaghel" +owning-sig: sig-storage +participating-sigs: +reviewers: + - "@jiawei0227" +approvers: + - "@jiawei0227" +editor: "@abhisheksinghbaghel" +creation-date: 2021-04-07 +last-updated: 2021-04-07 +status: implementable +see-also: + - "https://github.com/kubernetes/kubernetes/issues/57923" + - "https://github.com/kubernetes/enhancements/issues/2605" + +# The target maturity stage in the current dev cycle for this KEP. +stage: alpha + +# The most recent milestone for which work toward delivery of this KEP has been +# done. This can be the current (upcoming) milestone, if it is being actively +# worked on. +latest-milestone: "v1.22" + +# The milestone at which this feature was, or is targeted to be, at each stage. +milestone: + alpha: "v1.22" \ No newline at end of file From a0ccbb4aebd959af574e08719986099fe67898b8 Mon Sep 17 00:00:00 2001 From: Abhishek Singh Baghel Date: Wed, 7 Apr 2021 20:36:49 +0000 Subject: [PATCH 2/6] fix spell-check errors --- .../README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md index 757ebce34ed..8d66a71160c 100644 --- a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md +++ b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md @@ -55,10 +55,10 @@ Some workloads may want to run multiple containers in a single pod, under differ have the ability to access a shared\common secret or config volume. As a platform, Kubernetes should evolve to allow the sharing of AtomicWriter volumes (secret, config etc.) -accross containers running under different UIDs as a first party scenario. +across containers running under different UIDs as a first party scenario. With this feature, we try to provide a backwards compatible way for the users to -share a AtomicWriter volumes accross containers running under different UIDs. +share a AtomicWriter volumes across containers running under different UIDs. We intend to provide a way for the users to opt-in for the new behavior and expect not to break any existing applications\configurations. @@ -153,7 +153,7 @@ chown and chmod to the fsGroup - We would have to pass the payload information along with PreserveDefaultMode to the SetVolumeOwnership routine and skip chown\chmod if PreserveDefaultMode is set to true for the file. - This will be more invasive change, but would allow more granular control at per file level - - If users want to preserve Mode for one file in the volume and not for another they can acheive that by creating + - If users want to preserve Mode for one file in the volume and not for another they can achieve that by creating two different volumes, with PreserveDefaultMode set to true for the volume where they want to preserve default mode for all files ### Scope Of the Change From a8d85db06b1e9920a8c813c29de636c9a2493532 Mon Sep 17 00:00:00 2001 From: Abhishek Singh Baghel Date: Wed, 7 Apr 2021 21:23:04 +0000 Subject: [PATCH 3/6] add the status file under prod-readiness --- keps/prod-readiness/sig-storage/2605.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 keps/prod-readiness/sig-storage/2605.yaml diff --git a/keps/prod-readiness/sig-storage/2605.yaml b/keps/prod-readiness/sig-storage/2605.yaml new file mode 100644 index 00000000000..72d2f07ad0d --- /dev/null +++ b/keps/prod-readiness/sig-storage/2605.yaml @@ -0,0 +1,3 @@ +kep-number: 2605 +alpha: + approver: "@deads2k" From 9c0fc425cb1328172e6750476deb0c4d9f845cd7 Mon Sep 17 00:00:00 2001 From: Abhishek Singh Baghel Date: Wed, 7 Apr 2021 22:27:33 +0000 Subject: [PATCH 4/6] fix typo on toc --- .../README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md index 8d66a71160c..8d2e06f6105 100644 --- a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md +++ b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md @@ -7,7 +7,7 @@ - [Summary](#summary) - [Motivation](#motivation) - [Proposal](#proposal) - - [Preserve DefaultMode Flag](#preserve-defaultMode-flag) + - [Preserve DefaultMode Flag](#preserve-defaultmode-flag) - [File Permission](#file-permission) - [Proposed heuristics](#proposed-heuristics) - [Alternatives considered](#alternatives-considered) From 2211163f38820efaf3c6ee3e86d99a4f6d87af25 Mon Sep 17 00:00:00 2001 From: Abhishek Singh Baghel Date: Wed, 21 Apr 2021 07:51:21 +0000 Subject: [PATCH 5/6] address review comments --- .../README.md | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md index 8d2e06f6105..ef1ad9e789d 100644 --- a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md +++ b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md @@ -1,4 +1,4 @@ -# Option to Preserve default file mode bit (Mode\DefaultMode) set in AtomicWriter volumes +# Option to Preserve default file mode bit (Mode/DefaultMode) set in AtomicWriter volumes ## Table of Contents @@ -34,34 +34,42 @@ Kubernetes allows users to use Secret (and other configs) as a VolumeSource in pod spec. It also allows users to set a default file mode at the secret or every file level. The security context set for the pod (runAsUser or fsGroup) conflicts with -default mode specified for the secret\file. In current implementation of the +default mode specified for the secret/file. In current implementation of the AtomicWriter volumes, default file mode is overwritten by the runAsUser or fsGroup bit while setting the volume ownership. Above results in unexpected behavior in cases where users want to run multiple containers in a single pod and want them to run under a fsGroup, have different UIDs etc. -and yet have the ability to share same secret\config volume across containers. +and yet have the ability to share same secret/config volume across containers. -This proposal proposes to provide users with ability to preserve the file Mode\DefaultMode bit set on the +For ex: If user has a ssh key, exposed as a secret volume to the pod, which runs +under security context of a fsGroup. +If user sets the DefaultMode on the secret volume to 256. User expects file mode to be set to 0400 on all files. +However, the actual file mode set on the files is 0440, as SetVolumeOwnership applies +the group ownership permissions to the files in the volume. + +This proposal proposes to provide users with ability to preserve the file Mode/DefaultMode bit set on the AtomicWriter volumes (Secret, ConfigMap etc) in an opt-in and backward compatible manner. ## Motivation Many workloads running on Kubernetes today need the ability to run multiple containers in the same pod. Primitives such as main, sidecars and init containers form the core -of distributed application development in kubernetes and are very widely accepted\used patterns. +of distributed application development in kubernetes and are very widely accepted/used patterns. Some workloads may want to run multiple containers in a single pod, under different UIDs and -have the ability to access a shared\common secret or config volume. +have the ability to access a shared/common secret or config volume and yet have a +tighter control on the file mode set on the files inside the volume. As a platform, Kubernetes should evolve to allow the sharing of AtomicWriter volumes (secret, config etc.) -across containers running under different UIDs as a first party scenario. +across containers running under different UIDs as a first party scenario and allow users to control the +file mode bit set on the files in a predictable way. With this feature, we try to provide a backwards compatible way for the users to -share a AtomicWriter volumes across containers running under different UIDs. +to have a tighter/predictable control over the file mode set on the files in AtomicWriter volumes. We intend to provide a way for the users to opt-in for the new behavior and expect not to break -any existing applications\configurations. +any existing applications/configurations. ## Proposal @@ -111,6 +119,9 @@ type SecretVolumeSource struct { // at the time of file creation. // If PreserveDefaultMode is set to true the file mode is not modified based // on fsGroup after the file is created with the defaultMode. + // If the PreserveDefaultMode is set to false and fsGroup is specified + // the volume is modified to be owned by the fsGroup. + // If PreserveDefaultMode is not specified in the volume spec, it defaults to false. // +optional PreserveDefaultMode *bool } @@ -137,13 +148,13 @@ chown and chmod to the fsGroup #### Proposed heuristics -- *Case 1*: The volume has PreserveDefaultMode set to false. +- *Case 1*: The volume has PreserveDefaultMode set to false or not specified. In this case there is no behavioural change in how the file permissions are set for the volume. In other words, we will continue to create the files - with Mode\Default and then set the volume ownership based on fsGroup if specified. + with Mode/Default and then set the volume ownership based on fsGroup if specified. This is the default case. - *Case 2*: The volume has PreserveDefaultMode set to true. - In this case the payload\volume\files are created using Mode\Default mode. + In this case the payload/volume/files are created using Mode/Default mode. Then the call to SetVolumeOwnership is skipped, in turn preserving the Default file mode on the files. #### Alternatives considered @@ -151,7 +162,7 @@ chown and chmod to the fsGroup - Instead of having PreserveDefaultMode flag at the volume source level, we can have the same flag in the KeyToPath struct. That will allow us granular control over every file with-in the volume : - We would have to pass the payload information along with PreserveDefaultMode to the SetVolumeOwnership routine - and skip chown\chmod if PreserveDefaultMode is set to true for the file. + and skip chown/chmod if PreserveDefaultMode is set to true for the file. - This will be more invasive change, but would allow more granular control at per file level - If users want to preserve Mode for one file in the volume and not for another they can achieve that by creating two different volumes, with PreserveDefaultMode set to true for the volume where they want to preserve default mode for all files From 130a965b2a8ca03d7cc95bffdad596d7da57b612 Mon Sep 17 00:00:00 2001 From: Abhishek Singh Baghel Date: Sun, 25 Apr 2021 09:46:03 +0000 Subject: [PATCH 6/6] Address review comments --- .../README.md | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md index ef1ad9e789d..7ec7d2c5207 100644 --- a/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md +++ b/keps/sig-storage/2605-option-to-preserve-default-file-mode-bit-in-atomicwriter-volumes/README.md @@ -7,7 +7,7 @@ - [Summary](#summary) - [Motivation](#motivation) - [Proposal](#proposal) - - [Preserve DefaultMode Flag](#preserve-defaultmode-flag) + - [Preserve Permissions Flag](#preserve-permissions-flag) - [File Permission](#file-permission) - [Proposed heuristics](#proposed-heuristics) - [Alternatives considered](#alternatives-considered) @@ -73,15 +73,15 @@ any existing applications/configurations. ## Proposal -Kubernetes should implement a new boolean flag (PreserveDefaultMode) in the SecretVolumeSource and +Kubernetes should implement a new boolean flag (PreservePermissions) in the SecretVolumeSource and other AtomicWriter volumes. The flag should be optional and default to false in code. This flag should be honored at the time of volume -set-up and call to SetVolumeOwnership should be skipped in case PreserveDefaultMode=true. +set-up and call to SetVolumeOwnership should be skipped in case PreservePermissions=true. -### Preserve DefaultMode Flag +### Preserve Permissions Flag -A new PreserveDefaultMode boolean will be implemented in SecretVolumeSource and other AtomicWriter volumes. +A new PreservePermissions boolean will be implemented in SecretVolumeSource and other AtomicWriter volumes. ```go // SecretVolumeSource adapts a Secret into a volume. @@ -107,7 +107,7 @@ type SecretVolumeSource struct { // Directories within the path are not affected by this setting. // This might be in conflict with other options that affect the file // mode, like fsGroup, and the result can be other mode bits set. - // If PreserveDefaultMode is set to true then file mode bit is not + // If PreservePermissions is set to true then file mode bit is not // modified after the file creation, due to any other options set // such as fsGroup etc // +optional @@ -117,23 +117,23 @@ type SecretVolumeSource struct { Optional *bool // Specify whether we want to preserve the DefaultMode set on the files // at the time of file creation. - // If PreserveDefaultMode is set to true the file mode is not modified based + // If PreservePermissions is set to true the file mode is not modified based // on fsGroup after the file is created with the defaultMode. - // If the PreserveDefaultMode is set to false and fsGroup is specified + // If the PreservePermissions is set to false and fsGroup is specified // the volume is modified to be owned by the fsGroup. - // If PreserveDefaultMode is not specified in the volume spec, it defaults to false. + // If PreservePermissions is not specified in the volume spec, it defaults to false. // +optional - PreserveDefaultMode *bool + PreservePermissions *bool } ``` -A secret volume source with preserveDefaultMode=true: +A secret volume source with PreservePermissions=true: ```yaml volumes: - name: test-secret secret: - preserveDefaultMode: true + preservePermissions: true defaultMode: 256 secretName: test-secret ``` @@ -148,24 +148,24 @@ chown and chmod to the fsGroup #### Proposed heuristics -- *Case 1*: The volume has PreserveDefaultMode set to false or not specified. +- *Case 1*: The volume has PreservePermissions set to false or not specified. In this case there is no behavioural change in how the file permissions are set for the volume. In other words, we will continue to create the files with Mode/Default and then set the volume ownership based on fsGroup if specified. This is the default case. -- *Case 2*: The volume has PreserveDefaultMode set to true. +- *Case 2*: The volume has PreservePermissions set to true. In this case the payload/volume/files are created using Mode/Default mode. Then the call to SetVolumeOwnership is skipped, in turn preserving the Default file mode on the files. #### Alternatives considered -- Instead of having PreserveDefaultMode flag at the volume source level, we can have the same flag in +- Instead of having PreservePermissions flag at the volume source level, we can have the same flag in the KeyToPath struct. That will allow us granular control over every file with-in the volume : - - We would have to pass the payload information along with PreserveDefaultMode to the SetVolumeOwnership routine - and skip chown/chmod if PreserveDefaultMode is set to true for the file. + - We would have to pass the payload information along with PreservePermissions to the SetVolumeOwnership routine + and skip chown/chmod if PreservePermissions is set to true for the file. - This will be more invasive change, but would allow more granular control at per file level - If users want to preserve Mode for one file in the volume and not for another they can achieve that by creating - two different volumes, with PreserveDefaultMode set to true for the volume where they want to preserve default mode for all files + two different volumes, with PreservePermissions set to true for the volume where they want to preserve default mode for all files ### Scope Of the Change