Add mountOptions on volumeMounts#2681
Draft
nispriha wants to merge 2 commits into
Draft
Conversation
openshift-ci-robot
added
the
backports/unvalidated-commits
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
@nispriha: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
WalkthroughThis PR adds support for per-volume mount options via a new ChangesVolume Mount Options Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nispriha The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/kubelet/kubelet_pods.go (1)
402-413:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce the feature gate before propagating
MountOptions.
makeMountsforwardsmount.MountOptionsunconditionally. This bypasses gate intent for kubelet-managed static pods (which do not go through apiserver validation). Add a local check mirroring theRecursiveReadOnlyMountspattern and reject non-emptymountOptionswhenVolumeMountOptionsis disabled.Suggested fix
rro, err := resolveRecursiveReadOnly(mount, supportsRRO) if err != nil { return nil, cleanupAction, fmt.Errorf("failed to resolve recursive read-only mode: %w", err) } if rro && !utilfeature.DefaultFeatureGate.Enabled(features.RecursiveReadOnlyMounts) { return nil, cleanupAction, fmt.Errorf("recursive read-only mount needs feature gate %q to be enabled", features.RecursiveReadOnlyMounts) } + if len(mount.MountOptions) > 0 && !utilfeature.DefaultFeatureGate.Enabled(features.VolumeMountOptions) { + return nil, cleanupAction, fmt.Errorf("mount options require feature gate %q to be enabled", features.VolumeMountOptions) + } mounts = append(mounts, kubecontainer.Mount{🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/kubelet/kubelet_pods.go` around lines 402 - 413, In makeMounts, enforce the VolumeMountOptions feature gate before propagating mount.MountOptions into the kubecontainer.Mount; mirror the RecursiveReadOnlyMounts pattern used elsewhere: check utilfeature.DefaultFeatureGate.Enabled(features.VolumeMountOptions) and if the gate is disabled and the incoming mount.MountOptions is non-empty, reject it (return an error or otherwise fail pod validation) instead of unconditionally assigning MountOptions, otherwise safely assign mount.MountOptions to kubecontainer.Mount.MountOptions.
🧹 Nitpick comments (2)
staging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.json (1)
620-624: 💤 Low valueConsider using valid mount option values in test fixtures.
The
mountOptionsfield contains"mountOptionsValue", which is not a valid mount option. According to the API definition, only VFS-level options like"noexec","nodev", or"nosuid"are permitted. While test fixtures commonly use placeholder values, using a semantically valid value would better exercise validation logic and serve as a clearer example.📝 Suggested improvement
"subPathExpr": "subPathExprValue", "mountOptions": [ - "mountOptionsValue" + "noexec" ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@staging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.json` around lines 620 - 624, The test fixture uses an invalid placeholder "mountOptionsValue" for the mountOptions array; update the "mountOptions" entry in the extensions.v1beta1.ReplicaSet test JSON to use a semantically valid VFS-level option (e.g., "noexec", "nodev" or "nosuid") so validation logic is exercised—locate the JSON object containing "subPathExpr" and "mountOptions" and replace the placeholder string with one of those valid option values.staging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.yaml (1)
421-429: 💤 Low valueInconsistent field ordering between JSON and YAML fixtures.
In the JSON fixture,
mountOptionsis placed at the end of thevolumeMountsobject (aftersubPathExpr), but in this YAML fixture,mountOptionsappears at the beginning (beforemountPath). While field order doesn't affect functionality, test fixtures should maintain consistent ordering across formats for maintainability and to serve as canonical examples.🔄 Suggested alignment
Consider reordering fields to match the JSON fixture structure:
volumeMounts: - - mountOptions: - - mountOptionsValue - mountPath: mountPathValue + - mountPath: mountPathValue mountPropagation: mountPropagationValue name: nameValue readOnly: true recursiveReadOnly: recursiveReadOnlyValue subPath: subPathValue subPathExpr: subPathExprValue + mountOptions: + - mountOptionsValue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@staging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.yaml` around lines 421 - 429, Move the mountOptions entry within the volumeMounts mapping so its key order matches the JSON fixture: place mountOptions after subPathExpr (i.e., reorder the fields in the volumeMounts block so the sequence is mountPath, mountPropagation, name, readOnly, recursiveReadOnly, subPath, subPathExpr, then mountOptions). Update the YAML fixture's volumeMounts section (look for the mountOptions and subPathExpr keys) to reflect this ordering for consistency across fixtures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.yaml`:
- Around line 481-483: Update the fixture to use allowlisted mountOptions values
instead of the placeholder "mountOptionsValue": replace each occurrence of the
mountOptions entry that uses "mountOptionsValue" (the YAML key "mountOptions"
with value "mountOptionsValue") with one or more of the validated options
("noexec", "nodev", or "nosuid") so the CronJob fixture passes validation;
ensure the corresponding mountPaths (e.g., the block containing "mountPath:
mountPathValue") remain unchanged.
---
Outside diff comments:
In `@pkg/kubelet/kubelet_pods.go`:
- Around line 402-413: In makeMounts, enforce the VolumeMountOptions feature
gate before propagating mount.MountOptions into the kubecontainer.Mount; mirror
the RecursiveReadOnlyMounts pattern used elsewhere: check
utilfeature.DefaultFeatureGate.Enabled(features.VolumeMountOptions) and if the
gate is disabled and the incoming mount.MountOptions is non-empty, reject it
(return an error or otherwise fail pod validation) instead of unconditionally
assigning MountOptions, otherwise safely assign mount.MountOptions to
kubecontainer.Mount.MountOptions.
---
Nitpick comments:
In `@staging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.json`:
- Around line 620-624: The test fixture uses an invalid placeholder
"mountOptionsValue" for the mountOptions array; update the "mountOptions" entry
in the extensions.v1beta1.ReplicaSet test JSON to use a semantically valid
VFS-level option (e.g., "noexec", "nodev" or "nosuid") so validation logic is
exercised—locate the JSON object containing "subPathExpr" and "mountOptions" and
replace the placeholder string with one of those valid option values.
In `@staging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.yaml`:
- Around line 421-429: Move the mountOptions entry within the volumeMounts
mapping so its key order matches the JSON fixture: place mountOptions after
subPathExpr (i.e., reorder the fields in the volumeMounts block so the sequence
is mountPath, mountPropagation, name, readOnly, recursiveReadOnly, subPath,
subPathExpr, then mountOptions). Update the YAML fixture's volumeMounts section
(look for the mountOptions and subPathExpr keys) to reflect this ordering for
consistency across fixtures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a0eff700-c5c8-4fa8-8873-e4907f4e7318
⛔ Files ignored due to path filters (5)
pkg/apis/core/v1/zz_generated.conversion.gois excluded by!**/zz_generated*pkg/apis/core/zz_generated.deepcopy.gois excluded by!**/zz_generated*pkg/generated/openapi/zz_generated.openapi.gois excluded by!**/generated/**,!**/zz_generated*staging/src/k8s.io/api/core/v1/generated.pb.gois excluded by!**/*.pb.gostaging/src/k8s.io/api/core/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*
📒 Files selected for processing (51)
pkg/apis/core/types.gopkg/apis/core/validation/validation.gopkg/features/kube_features.gopkg/kubelet/container/runtime.gopkg/kubelet/kubelet_pods.gopkg/kubelet/kuberuntime/kuberuntime_container.gostaging/src/k8s.io/api/core/v1/generated.protostaging/src/k8s.io/api/core/v1/types.gostaging/src/k8s.io/api/core/v1/types_swagger_doc_generated.gostaging/src/k8s.io/api/testdata/HEAD/apps.v1.DaemonSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.DaemonSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1.ReplicaSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.ReplicaSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1.StatefulSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.StatefulSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.StatefulSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.StatefulSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.DaemonSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.DaemonSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.ReplicaSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.ReplicaSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.StatefulSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.StatefulSet.yamlstaging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.jsonstaging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.yamlstaging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.jsonstaging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.yamlstaging/src/k8s.io/api/testdata/HEAD/batch.v1beta1.CronJob.jsonstaging/src/k8s.io/api/testdata/HEAD/batch.v1beta1.CronJob.yamlstaging/src/k8s.io/api/testdata/HEAD/core.v1.Pod.jsonstaging/src/k8s.io/api/testdata/HEAD/core.v1.Pod.yamlstaging/src/k8s.io/api/testdata/HEAD/core.v1.PodTemplate.jsonstaging/src/k8s.io/api/testdata/HEAD/core.v1.PodTemplate.yamlstaging/src/k8s.io/api/testdata/HEAD/core.v1.ReplicationController.jsonstaging/src/k8s.io/api/testdata/HEAD/core.v1.ReplicationController.yamlstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.DaemonSet.jsonstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.DaemonSet.yamlstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.jsonstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.yamlstaging/src/k8s.io/client-go/applyconfigurations/core/v1/volumemount.gostaging/src/k8s.io/client-go/applyconfigurations/internal/internal.gostaging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.prototest/compatibility_lifecycle/reference/versioned_feature_list.yaml
Comment on lines
+481
to
+483
| - mountOptions: | ||
| - mountOptionsValue | ||
| mountPath: mountPathValue |
There was a problem hiding this comment.
Use allowlisted mountOptions values in fixtures.
Line 481, Line 712, and Line 944 set mountOptionsValue, but validation only accepts noexec, nodev, and nosuid for mountOptions. This can break validation-aligned fixture usage.
Suggested patch
- - mountOptions:
- - mountOptionsValue
+ - mountOptions:
+ - noexec
mountPath: mountPathValue
...
- - mountOptions:
- - mountOptionsValue
+ - mountOptions:
+ - noexec
mountPath: mountPathValue
...
- - mountOptions:
- - mountOptionsValue
+ - mountOptions:
+ - noexec
mountPath: mountPathValueAlso applies to: 712-714, 944-946
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.yaml` around lines 481
- 483, Update the fixture to use allowlisted mountOptions values instead of the
placeholder "mountOptionsValue": replace each occurrence of the mountOptions
entry that uses "mountOptionsValue" (the YAML key "mountOptions" with value
"mountOptionsValue") with one or more of the validated options ("noexec",
"nodev", or "nosuid") so the CronJob fixture passes validation; ensure the
corresponding mountPaths (e.g., the block containing "mountPath:
mountPathValue") remain unchanged.
|
@nispriha: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.json`:
- Around line 702-705: The fixture uses an invalid mount option token
"mountOptionsValue" under the JSON key "mountOptions" (adjacent to
"subPathExpr"); replace it with one or more allowlisted values accepted by
ValidateVolumeMounts (e.g., "noexec", "nodev", or "nosuid") and ensure each
occurrence (including the other two similar places) is updated to use only those
allowed strings so the fixture passes validation.
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.json`:
- Around line 653-656: The test fixture uses an invalid mountOptions value
"mountOptionsValue"; locate the occurrences of the "mountOptions" array (near
the "subPathExpr" entry) in
staging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.json and replace the
placeholder with one of the server-allowed strings ("noexec", "nodev", or
"nosuid") so the JSON conforms to server validation for mount options; apply the
same replacement to the other reported occurrences in the file.
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.yaml`:
- Around line 445-447: Replace the placeholder mount option value
"mountOptionsValue" with an allowed mount option (one of noexec, nodev, or
nosuid) wherever a mountOptions list is defined in this YAML (look for the
"mountOptions:" entries paired with "mountPath:" and the VolumeMount examples);
update each occurrence (the three places with "- mountOptionsValue") to a valid
value such as "noexec" so the entries conform to VolumeMount validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 10350882-81fa-484a-870a-448acea55467
⛔ Files ignored due to path filters (6)
pkg/apis/core/v1/zz_generated.conversion.gois excluded by!**/zz_generated*pkg/apis/core/zz_generated.deepcopy.gois excluded by!**/zz_generated*pkg/generated/openapi/zz_generated.openapi.gois excluded by!**/generated/**,!**/zz_generated*staging/src/k8s.io/api/core/v1/generated.pb.gois excluded by!**/*.pb.gostaging/src/k8s.io/api/core/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (51)
pkg/apis/core/types.gopkg/apis/core/validation/validation.gopkg/features/kube_features.gopkg/kubelet/container/runtime.gopkg/kubelet/kubelet_pods.gopkg/kubelet/kuberuntime/kuberuntime_container.gostaging/src/k8s.io/api/core/v1/generated.protostaging/src/k8s.io/api/core/v1/types.gostaging/src/k8s.io/api/core/v1/types_swagger_doc_generated.gostaging/src/k8s.io/api/testdata/HEAD/apps.v1.DaemonSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.DaemonSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1.ReplicaSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.ReplicaSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1.StatefulSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1.StatefulSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.StatefulSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta1.StatefulSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.DaemonSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.DaemonSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.ReplicaSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.ReplicaSet.yamlstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.StatefulSet.jsonstaging/src/k8s.io/api/testdata/HEAD/apps.v1beta2.StatefulSet.yamlstaging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.jsonstaging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.yamlstaging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.jsonstaging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.yamlstaging/src/k8s.io/api/testdata/HEAD/batch.v1beta1.CronJob.jsonstaging/src/k8s.io/api/testdata/HEAD/batch.v1beta1.CronJob.yamlstaging/src/k8s.io/api/testdata/HEAD/core.v1.Pod.jsonstaging/src/k8s.io/api/testdata/HEAD/core.v1.Pod.yamlstaging/src/k8s.io/api/testdata/HEAD/core.v1.PodTemplate.jsonstaging/src/k8s.io/api/testdata/HEAD/core.v1.PodTemplate.yamlstaging/src/k8s.io/api/testdata/HEAD/core.v1.ReplicationController.jsonstaging/src/k8s.io/api/testdata/HEAD/core.v1.ReplicationController.yamlstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.DaemonSet.jsonstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.DaemonSet.yamlstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.Deployment.jsonstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.Deployment.yamlstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.jsonstaging/src/k8s.io/api/testdata/HEAD/extensions.v1beta1.ReplicaSet.yamlstaging/src/k8s.io/client-go/applyconfigurations/core/v1/volumemount.gostaging/src/k8s.io/client-go/applyconfigurations/internal/internal.gostaging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.prototest/compatibility_lifecycle/reference/versioned_feature_list.yaml
Comment on lines
+3150
to
+3160
| if len(mnt.MountOptions) > 0 { | ||
| if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeMountOptions) { | ||
| allErrs = append(allErrs, field.Forbidden(idxPath.Child("mountOptions"), "mount options require the VolumeMountOptions feature gate to be enabled")) | ||
| } else { | ||
| allowedOptions := sets.New("noexec", "nodev", "nosuid") | ||
| for j, opt := range mnt.MountOptions { | ||
| if !allowedOptions.Has(opt) { | ||
| allErrs = append(allErrs, field.NotSupported(idxPath.Child("mountOptions").Index(j), opt, sets.List(allowedOptions))) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== References to VolumeMountOptions feature gate =="
rg -n -C3 'VolumeMountOptions|MountOptions' pkg staging/src/k8s.io 2>/dev/null || true
echo
echo "== Pod strategy/drop-disabled handling =="
rg -n -C4 'dropDisabled|DropDisabled|VolumeMountOptions|mountOptions' pkg/registry/core 2>/dev/null || true
echo
echo "== Pod validation options plumbing for create/update =="
rg -n -C4 'PodValidationOptions|ValidationOptionsForPod|ValidatePod(Create|Update|TemplateSpec)' pkg/apis/core 2>/dev/null || trueRepository: openshift/kubernetes
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate ValidateVolumeMounts call sites =="
rg -n "ValidateVolumeMounts" pkg/apis/core/validation/validation.go pkg/apis/core/validation -S || true
echo
echo "== Feature gate check usage (VolumeMountOptions) =="
rg -n "VolumeMountOptions" pkg/apis/core/validation/validation.go pkg/apis/core/validation -S || true
echo
echo "== Pod validation entrypoints (create/update/template) =="
rg -n "ValidatePod(Create|Update|TemplateSpec)" pkg/apis/core/validation -S || true
rg -n "type PodValidationOptions|PodValidationOptions" pkg/apis/core/validation -S || true
echo
echo "== Any pod field-dropping / dropDisabled logic touching mountOptions =="
rg -n "(dropDisabled|DropDisabled)" pkg/registry/core -S || true
rg -n "mountOptions" pkg/registry/core -S | head -n 80 || trueRepository: openshift/kubernetes
Length of output: 37115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for mountOptions in pod disabled field dropping =="
rg -n "mountOptions|VolumeMountOptions" pkg/registry/core/pod pkg/registry/core/podtemplate pkg/registry/core/replicationcontroller pkg/registry/core -S || true
echo
echo "== Locate podutil.DropDisabledPodFields implementation =="
rg -n "func DropDisabledPodFields" -S pkg | head -n 50
echo
# Search for the helper that might drop mountOptions inside podutil or related utils
rg -n "DropDisabledPodFields" -S pkg | head -n 50
echo
echo "== Search for VolumeMountOptions feature gate in registry pod util code =="
rg -n "VolumeMountOptions" -S pkg/registry pkg/util pkg/apis | head -n 200Repository: openshift/kubernetes
Length of output: 1823
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show DropDisabledPodFields logic around where spec fields might be dropped
nl -ba pkg/api/pod/util.go | sed -n '650,780p'
echo
echo "== Search for mountOptions/VolumeMountOptions inside util.go =="
rg -n "mountOptions|VolumeMountOptions" pkg/api/pod/util.go -S || trueRepository: openshift/kubernetes
Length of output: 107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DropDisabledPodFields implementation (pkg/api/pod/util.go) =="
cat -n pkg/api/pod/util.go | sed -n '660,780p'
echo
echo "== Search for mountOptions/VolumeMountOptions inside util.go =="
rg -n "mountOptions|VolumeMountOptions" pkg/api/pod/util.go -S || trueRepository: openshift/kubernetes
Length of output: 5813
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Does podutil/dropDisabled handle mountOptions? =="
rg -n "mountOptions" pkg/api/pod -S || true
rg -n "VolumeMountOptions" pkg/api/pod -S || true
echo
echo "== Remaining part of dropDisabledFields (look for any mountOptions-related helpers) =="
rg -n "dropDisabledFields|dropDisabled.*mount|mountOptions" pkg/api/pod/util.go -S || true
echo
echo "== PodValidationOptions fields mentioning mountOptions / gate =="
rg -n "type PodValidationOptions" -n pkg/apis/core/validation/validation.go
sed -n '4450,4555p' pkg/apis/core/validation/validation.go
echo
echo "== Any other uses of VolumeMountOptions beyond ValidateVolumeMounts =="
rg -n "VolumeMountOptions" pkg/apis/core/validation -S || true
echo
echo "== Any other gate-drop / field-drop logic for mountOptions elsewhere =="
rg -n "mountOptions.*featuregate|featuregate.*mountOptions|VolumeMountOptions.*drop" -S pkg | head -n 200 || trueRepository: openshift/kubernetes
Length of output: 6360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for VolumeMountOptions in pod util (pkg/api/pod/util.go) =="
rg -n "VolumeMountOptions" pkg/api/pod/util.go pkg/api/pod -S || true
echo
echo "== Inspect GetValidationOptionsFromPodSpecAndMeta in pkg/api/pod/util.go =="
rg -n "func GetValidationOptionsFromPodSpecAndMeta" pkg/api/pod/util.go -S
# print a window around the function
start=$(rg -n "func GetValidationOptionsFromPodSpecAndMeta" pkg/api/pod/util.go -n | head -n1 | cut -d: -f1)
end=$((start+260))
sed -n "${start},${end}p" pkg/api/pod/util.go
echo
echo "== Inspect pod strategy PrepareForUpdate/PrepareForCreate calling DropDisabledPodFields =="
rg -n "PrepareForUpdate|PrepareForCreate|DropDisabledPodFields" pkg/registry/core/pod/strategy.go -S
cat -n pkg/registry/core/pod/strategy.go | sed -n '60,180p'
echo
echo "== Inspect ValidateVolumeMounts call site in ValidateVolumeMounts flow (validateContainerCommon) =="
cat -n pkg/apis/core/validation/validation.go | sed -n '3870,3960p'Repository: openshift/kubernetes
Length of output: 22390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for mountOptions / VolumeMountOptions in pod drop-disabled code =="
rg -n "mountOptions|VolumeMountOptions|MountOptions" pkg/api/pod -S || true
echo
echo "== Search for DropDisabledTemplateFields call sites (pods embedded in controllers) =="
rg -n "DropDisabledTemplateFields" pkg/registry/core -S
echo
echo "== Search for PodTemplate validation update flow and whether it uses old spec for gating =="
rg -n "ValidatePodTemplateUpdate" pkg/apis/core/validation/validation.go
# show the function body window
start=$(rg -n "func ValidatePodTemplateUpdate" pkg/apis/core/validation/validation.go | head -n1 | cut -d: -f1)
end=$((start+220))
sed -n "${start},${end}p" pkg/apis/core/validation/validation.goRepository: openshift/kubernetes
Length of output: 11216
Fix/update compatibility when disabling the VolumeMountOptions feature gate
ValidateVolumeMounts hard-forbids volumeMount.mountOptions whenever features.VolumeMountOptions is disabled. Pod/controller create/update flows call podutil.DropDisabledPodFields/DropDisabledTemplateFields, but those drop helpers have no mountOptions/VolumeMountOptions handling, and GetValidationOptionsFromPodSpecAndMeta has no update-mode allowance for previously-set mountOptions.
So disabling the gate can make API updates to existing pods/templates that still contain mountOptions fail.
Comment on lines
+702
to
+705
| "subPathExpr": "subPathExprValue", | ||
| "mountOptions": [ | ||
| "mountOptionsValue" | ||
| ] |
There was a problem hiding this comment.
Use allowlisted mountOptions values in this fixture.
mountOptionsValue is not accepted by ValidateVolumeMounts; only noexec, nodev, and nosuid are allowed. Keeping this token can break validation-aligned fixture paths.
Suggested patch
- "mountOptions": [
- "mountOptionsValue"
- ]
+ "mountOptions": [
+ "noexec"
+ ]Also applies to: 1019-1022, 1336-1339
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.CronJob.json` around lines 702
- 705, The fixture uses an invalid mount option token "mountOptionsValue" under
the JSON key "mountOptions" (adjacent to "subPathExpr"); replace it with one or
more allowlisted values accepted by ValidateVolumeMounts (e.g., "noexec",
"nodev", or "nosuid") and ensure each occurrence (including the other two
similar places) is updated to use only those allowed strings so the fixture
passes validation.
Comment on lines
+653
to
+656
| "subPathExpr": "subPathExprValue", | ||
| "mountOptions": [ | ||
| "mountOptionsValue" | ||
| ] |
There was a problem hiding this comment.
mountOptions fixture value is outside server validation allowlist.
mountOptionsValue is rejected by validation; use one of noexec, nodev, or nosuid so this testdata stays contract-aligned.
Suggested patch
- "mountOptions": [
- "mountOptionsValue"
- ]
+ "mountOptions": [
+ "noexec"
+ ]Also applies to: 970-973, 1287-1290
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.json` around lines 653 -
656, The test fixture uses an invalid mountOptions value "mountOptionsValue";
locate the occurrences of the "mountOptions" array (near the "subPathExpr"
entry) in staging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.json and replace the
placeholder with one of the server-allowed strings ("noexec", "nodev", or
"nosuid") so the JSON conforms to server validation for mount options; apply the
same replacement to the other reported occurrences in the file.
Comment on lines
+445
to
+447
| - mountOptions: | ||
| - mountOptionsValue | ||
| mountPath: mountPathValue |
There was a problem hiding this comment.
Replace placeholder mountOptionsValue with an allowed mount option.
These entries should use an allowlisted option (noexec, nodev, nosuid) to match VolumeMount validation behavior.
Suggested patch
- - mountOptions:
- - mountOptionsValue
+ - mountOptions:
+ - noexec
mountPath: mountPathValueAlso applies to: 676-678, 908-910
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@staging/src/k8s.io/api/testdata/HEAD/batch.v1.Job.yaml` around lines 445 -
447, Replace the placeholder mount option value "mountOptionsValue" with an
allowed mount option (one of noexec, nodev, or nosuid) wherever a mountOptions
list is defined in this YAML (look for the "mountOptions:" entries paired with
"mountPath:" and the VolumeMount examples); update each occurrence (the three
places with "- mountOptionsValue") to a valid value such as "noexec" so the
entries conform to VolumeMount validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR is related to:
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Summary by CodeRabbit
Release Notes
noexec,nodev,nosuid), controlled by theVolumeMountOptionsfeature gate (alpha, available in v1.37).