cleanup: validate unknown parameters in ControllerModifyVolume by nixpanic · Pull Request #6458 · ceph/ceph-csi

nixpanic · 2026-08-05T11:22:40Z

Add validation to reject unknown mutable parameters in ControllerModifyVolume.
Previously, unknown parameters were silently ignored, which could lead to
confusion when users specify incorrect parameter names.

Related: #6443

iPraveenParihar

few questions

Outstanding reviews.

Pull request has been modified.

Madhu-1 · 2026-08-06T07:22:38Z

+	// Validate that only known parameters are provided (before any I/O)
+	knownParams := []string{
+		nfs.ParameterServer,
+		nfs.ParameterClients,
+	}
+
+	for param := range req.GetMutableParameters() {
+		if !slices.Contains(knownParams, param) {
+			return nil, status.Errorf(codes.InvalidArgument,
+				"unknown mutable parameter: %s", param)
+		}
+	}


we need to have similar check before creating the subvolume or else we need a defer to cleanup the subvolume if anything fails after creation, i prefer to have input validation before loop of create/delete

Currently CephFS does not support mutable parameters. But, well, #6390 is almost there now too. I'll add the extra checks/filtering in CreateVolume too, similar as what is done for nvmeof.

This is done in the last update of this PR.

Copilot

Pull request overview

This PR adds explicit validation for unknown mutable_parameters keys in ControllerModifyVolume (and NVMe-oF CreateVolume request validation), to avoid silently accepting typos/mistakes in VolumeAttributesClass mutable parameters.

Changes:

NVMe-oF: validate ControllerModifyVolume and CreateVolume request mutable_parameters to reject unknown keys.
NVMe-oF: avoid passing MutableParameters through to the RBD backend CreateVolume call.
NFS: avoid passing MutableParameters through to the CephFS backend CreateVolume call; validate ControllerModifyVolume mutable parameters.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
internal/nvmeof/controller/controllerserver.go	Adds allowlist validation for NVMe-oF mutable parameters and strips mutable parameters from the RBD backend create request.
internal/nfs/controller/controllerserver.go	Strips mutable parameters from the CephFS backend create request and adds allowlist validation for NFS mutable parameters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gadididi

looks good thanks!

Add validation for unknown mutable parameters in ControllerModifyVolume to catch configuration errors early. Create a shared nvmeofMutableParams slice to define all valid NVMe-oF mutable parameters in one place, eliminating code duplication. Filter out NVMe-oF-specific mutable parameters before passing the request to the RBD backend. NVMe-oF handles these parameters separately through ControllerModifyVolume. This prevents the RBD backend from receiving parameters it doesn't understand, which would cause validation errors. Remove redundant rbd.HasQoSParams() checks that became dead code after the validation was moved earlier in the flow. Assisted-by: AskBob <askbob@ibm.com> Signed-off-by: Niels de Vos <ndevos@ibm.com>

ceph-csi-bot · 2026-08-07T16:20:19Z

/test ci/centos/k8s-e2e-external-storage/1.34

ceph-csi-bot · 2026-08-07T16:20:19Z

/test ci/centos/mini-e2e-helm/k8s-1.34

ceph-csi-bot · 2026-08-07T16:20:20Z

/test ci/centos/k8s-e2e-external-storage/1.35

ceph-csi-bot · 2026-08-07T16:20:20Z

/test ci/centos/mini-e2e/k8s-1.34

ceph-csi-bot · 2026-08-07T16:20:21Z

/test ci/centos/mini-e2e-helm/k8s-1.35

ceph-csi-bot · 2026-08-07T16:20:21Z

/test ci/centos/mini-e2e/k8s-1.35

ceph-csi-bot · 2026-08-07T16:20:23Z

/test ci/centos/k8s-e2e-external-storage/1.36

ceph-csi-bot · 2026-08-07T16:20:24Z

/test ci/centos/mini-e2e-helm/k8s-1.36

ceph-csi-bot · 2026-08-07T16:20:25Z

/test ci/centos/mini-e2e/k8s-1.36

ceph-csi-bot · 2026-08-07T16:20:25Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2026-08-07T16:20:26Z

/test ci/centos/upgrade-tests-rbd

Madhu-1 · 2026-08-10T04:36:58Z

@Mergifyio refresh

mergify · 2026-08-10T04:37:07Z

refresh

✅ Pull request refreshed

Madhu-1 · 2026-08-10T04:38:02Z

@Mergifyio refresh

mergify · 2026-08-10T04:38:20Z

refresh

✅ Pull request refreshed

mergify · 2026-08-10T04:38:30Z

Deprecation notice: This pull request comes from a fork and was queued with update_method=rebase and update_bot_account impersonation. This capability will be removed on July 1, 2026. After this date, the merge queue will no longer be able to rebase fork pull requests with this configuration. To avoid disruption, switch to update_method=merge in your queue rule.

mergify · 2026-08-10T04:39:30Z

Merge Queue Status

✅ Entered queue — 2026-08-10 04:38 UTC · Rule: default · triggered by merge protections
✅ Checks skipped · PR is already up-to-date
✅ Merged — 2026-08-10 04:39 UTC · at b0daf72f40cfb9655ee2e83dfd13789c1acdbf42 · rebase

This pull request spent 1 minute 12 seconds in the queue, including 3 seconds running CI.

Required conditions to merge

github-review-approved [🛡 GitHub branch protection]
- cleanup: validate unknown parameters in ControllerModifyVolume #6458

nixpanic requested review from a team and gadididi August 5, 2026 11:23

mergify Bot added the cleanup label Aug 5, 2026

nixpanic added component/nfs Issues related to NFS component/nvme-of Issues and PRs related to NVMe-oF. labels Aug 5, 2026

Rakshith-R previously approved these changes Aug 5, 2026

View reviewed changes

black-dragon74 previously approved these changes Aug 5, 2026

View reviewed changes

gadididi reviewed Aug 5, 2026

View reviewed changes

Comment thread internal/nvmeof/controller/controllerserver.go Outdated

iPraveenParihar requested changes Aug 5, 2026

View reviewed changes

Comment thread internal/nvmeof/controller/controllerserver.go Outdated

Comment thread internal/nfs/controller/controllerserver.go Outdated

Madhu-1 reviewed Aug 5, 2026

View reviewed changes

Comment thread internal/nvmeof/controller/controllerserver.go Outdated

nixpanic mentioned this pull request Aug 5, 2026

rbd: return InvalidArgument for unrecognized QoS #6457

Merged

nixpanic force-pushed the ControllerModifyVolume/InvalidArgument branch from 3471981 to 3f7d560 Compare August 5, 2026 16:06

nixpanic force-pushed the ControllerModifyVolume/InvalidArgument branch from 3f7d560 to 218c870 Compare August 5, 2026 18:20

nixpanic requested a review from a team as a code owner August 5, 2026 18:20

nixpanic requested review from Madhu-1, Rakshith-R, black-dragon74, gadididi and iPraveenParihar August 5, 2026 18:21

gadididi reviewed Aug 6, 2026

View reviewed changes

Comment thread internal/nvmeof/controller/controllerserver.go Outdated

Comment thread internal/nvmeof/controller/controllerserver.go Outdated

Comment thread internal/nvmeof/controller/controllerserver.go Outdated

gadididi mentioned this pull request Aug 6, 2026

nvmeof: simplify ControllerCreateVolume() function #6461

Closed

Madhu-1 reviewed Aug 6, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings August 6, 2026 08:29

nixpanic force-pushed the ControllerModifyVolume/InvalidArgument branch from 218c870 to 6eca8d5 Compare August 6, 2026 08:29

Copilot started reviewing on behalf of nixpanic August 6, 2026 08:30 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Comment thread internal/nfs/controller/controllerserver.go

Comment thread internal/nvmeof/controller/controllerserver.go

Comment thread internal/nvmeof/controller/controllerserver.go

gadididi previously approved these changes Aug 6, 2026

View reviewed changes

nixpanic mentioned this pull request Aug 6, 2026

cephfs: support MDS pinning via ControllerModifyVolume #6390

Merged

6 tasks

ceph-csi-bot force-pushed the ControllerModifyVolume/InvalidArgument branch from 5977885 to b0daf72 Compare August 7, 2026 16:19

nixpanic added the ok-to-test Label to trigger E2E tests label Aug 7, 2026

ceph-csi-bot added ci/in-progress/e2e This label acts like a guard and prevents Mergify from adding the `ok-to-test` label again. and removed ok-to-test Label to trigger E2E tests labels Aug 7, 2026

mergify Bot removed the ci/in-progress/e2e This label acts like a guard and prevents Mergify from adding the `ok-to-test` label again. label Aug 7, 2026

gadididi approved these changes Aug 9, 2026

View reviewed changes

Madhu-1 approved these changes Aug 10, 2026

View reviewed changes

Madhu-1 added ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures labels Aug 10, 2026

mergify Bot added the queued label Aug 10, 2026

mergify Bot merged commit 6bc6df3 into ceph:devel Aug 10, 2026
52 checks passed

mergify Bot removed the queued label Aug 10, 2026

Uh oh!

Conversation

nixpanic commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

iPraveenParihar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Madhu-1 Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

nixpanic Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

nixpanic Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gadididi left a comment

Choose a reason for hiding this comment

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

ceph-csi-bot commented Aug 7, 2026

Uh oh!

Madhu-1 commented Aug 10, 2026

Uh oh!

mergify Bot commented Aug 10, 2026

✅ Pull request refreshed

Uh oh!

Madhu-1 commented Aug 10, 2026

Uh oh!

mergify Bot commented Aug 10, 2026

✅ Pull request refreshed

Uh oh!

mergify Bot commented Aug 10, 2026

Uh oh!

mergify Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Queue Status

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

nixpanic commented Aug 5, 2026 •

edited

Loading

mergify Bot commented Aug 10, 2026 •

edited

Loading