Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

csi: validate mount options during volume registration #9044

Merged
merged 2 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
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
12 changes: 11 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,16 @@ func (v *CSIVolume) Validate() error {
if v.AttachmentMode == "" {
errs = append(errs, "missing attachment mode")
}
if v.AttachmentMode == CSIVolumeAttachmentModeBlockDevice {
if v.MountOptions != nil {
tgross marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down