csi: validate mount options during volume registration (#9044)

Volumes using attachment mode `file-system` use the CSI filesystem API when
they're mounted, and can be passed mount options. But `block-device` mode
volumes don't have this option. When RPCs are made to plugins, we are silently
dropping the mount options we don't expect to see, but this results in a poor
operator experience when the mount options aren't honored. This changeset
makes passing mount options to a `block-device` volume a validation error.

CHANGELOG.md

@@ -14,6 +14,7 @@ IMPROVEMENTS:
 
 __BACKWARDS INCOMPATIBILITIES:__
  * core: null characters are prohibited in region, datacenter, job name/ID, task group name, and task name [[GH-9020](https://github.com/hashicorp/nomad/issues/9020)]
+ * csi: registering a CSI volume with a `block-device` attachment mode and `mount_options` now returns a validation error, instead of silently dropping the `mount_options`. [[GH-9044](https://github.com/hashicorp/nomad/issues/9044)]
  * driver/docker: Tasks are now issued SIGTERM instead of SIGINT when stopping [[GH-8932](https://github.com/hashicorp/nomad/issues/8932)]
 
 BUG FIXES:

nomad/csi_endpoint_test.go

@@ -141,8 +141,12 @@ func TestCSIVolumeEndpoint_Register(t *testing.T) {
 		Namespace:      "notTheNamespace",
 		PluginID:       "minnie",
 		AccessMode:     structs.CSIVolumeAccessModeMultiNodeReader,
-		AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
-		Secrets:        structs.CSISecrets{"mysecret": "secretvalue"},
+		AttachmentMode: structs.CSIVolumeAttachmentModeBlockDevice,
+		MountOptions: &structs.CSIMountOptions{
+			FSType: "ext4", MountFlags: []string{"sensitive"}},
+		Secrets:    structs.CSISecrets{"mysecret": "secretvalue"},
+		Parameters: map[string]string{"myparam": "paramvalue"},
+		Context:    map[string]string{"mycontext": "contextvalue"},
 	}}
 
 	// Create the register request
@@ -155,6 +159,11 @@ func TestCSIVolumeEndpoint_Register(t *testing.T) {
 	}
 	resp1 := &structs.CSIVolumeRegisterResponse{}
 	err := msgpackrpc.CallWithCodec(codec, "CSIVolume.Register", req1, resp1)
+	require.Error(t, err, "expected validation error")
+
+	// Fix the registration so that it passes validation
+	vols[0].AttachmentMode = structs.CSIVolumeAttachmentModeFilesystem
+	err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Register", req1, resp1)
 	require.NoError(t, err)
 	require.NotEqual(t, uint64(0), resp1.Index)
 
@@ -170,6 +179,10 @@ func TestCSIVolumeEndpoint_Register(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, resp1.Index, resp2.Index)
 	require.Equal(t, vols[0].ID, resp2.Volume.ID)
+	require.Equal(t, "csi.CSISecrets(map[mysecret:[REDACTED]])",
+		resp2.Volume.Secrets.String())
+	require.Equal(t, "csi.CSIOptions(FSType: ext4, MountFlags: [REDACTED])",
+		resp2.Volume.MountOptions.String())
 
 	// Registration does not update
 	req1.Volumes[0].PluginID = "adam"

nomad/structs/csi.go

@@ -172,7 +172,7 @@ func (o *CSIMountOptions) Merge(p *CSIMountOptions) {
 	}
 }
 
-// VolumeMountOptions implements the Stringer and GoStringer interfaces to prevent
+// CSIMountOptions implements the Stringer and GoStringer interfaces to prevent
 // accidental leakage of sensitive mount flags via logs.
 var _ fmt.Stringer = &CSIMountOptions{}
 var _ fmt.GoStringer = &CSIMountOptions{}
@@ -578,6 +578,16 @@ func (v *CSIVolume) Validate() error {
 	if v.AttachmentMode == "" {
 		errs = append(errs, "missing attachment mode")
 	}
+	if v.AttachmentMode == CSIVolumeAttachmentModeBlockDevice {
+		if v.MountOptions != nil {
+			if v.MountOptions.FSType != "" {
+				errs = append(errs, "mount options not allowed for block-device")
+			}
+			if v.MountOptions.MountFlags != nil && len(v.MountOptions.MountFlags) != 0 {
+				errs = append(errs, "mount options not allowed for block-device")
+			}
+		}
+	}
 
 	// TODO: Volume Topologies are optional - We should check to see if the plugin
 	//       the volume is being registered with requires them.