Skip to content

Commit

Permalink
csi: validate mount options during volume registration
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tgross committed Oct 7, 2020
1 parent 99b9a8d commit 2782645
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ 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:

* core: Fixed a bug where blocking queries would not include the query's maximum wait time when calculating whether it was safe to retry. [[GH-8921](https://github.com/hashicorp/nomad/issues/8921)]
Expand Down
17 changes: 15 additions & 2 deletions nomad/csi_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -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"
Expand Down
7 changes: 6 additions & 1 deletion nomad/structs/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -578,6 +578,11 @@ func (v *CSIVolume) Validate() error {
if v.AttachmentMode == "" {
errs = append(errs, "missing attachment mode")
}
if v.AttachmentMode == CSIVolumeAttachmentModeBlockDevice {
if v.MountOptions != nil {
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.
Expand Down

0 comments on commit 2782645

Please sign in to comment.