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

Add granular control of SELinux labels for host mounts #19839

Merged
merged 14 commits into from
Feb 5, 2024
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
3 changes: 3 additions & 0 deletions .changelog/19839.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
client/volumes: Add a mount volume level option for selinux tags on volumes
```
6 changes: 6 additions & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,15 +426,21 @@ type VolumeMount struct {
Destination *string `hcl:"destination,optional"`
ReadOnly *bool `mapstructure:"read_only" hcl:"read_only,optional"`
PropagationMode *string `mapstructure:"propagation_mode" hcl:"propagation_mode,optional"`
SELinuxLabel *string `mapstructure:"selinux_label" hcl:"selinux_label,optional"`
}

func (vm *VolumeMount) Canonicalize() {
if vm.PropagationMode == nil {
vm.PropagationMode = pointerOf(VolumeMountPropagationPrivate)
}

if vm.ReadOnly == nil {
vm.ReadOnly = pointerOf(false)
}

if vm.SELinuxLabel == nil {
vm.SELinuxLabel = pointerOf("")
}
}

// TaskGroup is the unit of scheduling.
Expand Down
2 changes: 2 additions & 0 deletions client/allocrunner/taskrunner/volume_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func (h *volumeHook) hostVolumeMountConfigurations(taskMounts []*structs.VolumeM
TaskPath: m.Destination,
Readonly: hostVolume.ReadOnly || req.ReadOnly || m.ReadOnly,
PropagationMode: m.PropagationMode,
SELinuxLabel: m.SELinuxLabel,
}
mounts = append(mounts, mcfg)
}
Expand Down Expand Up @@ -188,6 +189,7 @@ func (h *volumeHook) prepareCSIVolumes(req *interfaces.TaskPrestartRequest, volu
TaskPath: m.Destination,
Readonly: request.ReadOnly || m.ReadOnly,
PropagationMode: m.PropagationMode,
SELinuxLabel: m.SELinuxLabel,
}
mounts = append(mounts, mcfg)
}
Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1314,6 +1314,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
Destination: *mount.Destination,
ReadOnly: *mount.ReadOnly,
PropagationMode: *mount.PropagationMode,
SELinuxLabel: *mount.SELinuxLabel,
})
}
}
Expand Down
11 changes: 4 additions & 7 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -7928,15 +7928,12 @@ func (t *Task) Validate(jobType string, tg *TaskGroup) error {

// Validation for volumes
for idx, vm := range t.VolumeMounts {
if !MountPropagationModeIsValid(vm.PropagationMode) {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Volume Mount (%d) has an invalid propagation mode: \"%s\"", idx, vm.PropagationMode))
if _, ok := tg.Volumes[vm.Volume]; !ok {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Volume Mount (%d) references undefined volume %s", idx, vm.Volume))
}

// Validate the task does not reference undefined volume mounts
if vm.Volume == "" {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Volume Mount (%d) references an empty volume", idx))
} else if _, ok := tg.Volumes[vm.Volume]; !ok {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Volume Mount (%d) references undefined volume %s", idx, vm.Volume))
if err := vm.Validate(); err != nil {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Volume Mount (%d) is invalid: \"%w\"", idx, err))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example of how this error looks like? I'm not sure why we do it in some many parts of the code, but appending directly to mErr.Errors usually results in a bad error hierarchy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is actually called in the task group validation test, it should not be a problem

}
}

Expand Down
20 changes: 19 additions & 1 deletion nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,7 @@ func TestTaskGroup_Validate(t *testing.T) {
},
},
expErr: []string{
`Volume Mount (0) references an empty volume`,
`Volume Mount (0) references undefined volume`,
`Volume Mount (0) references undefined volume foob`,
},
jobType: JobTypeService,
Expand Down Expand Up @@ -2043,6 +2043,24 @@ func TestTask_Validate(t *testing.T) {
t.Fatalf("err: %s", err)
}

tg.Volumes = map[string]*VolumeRequest{
"foo": {
Name: "foo",
},
}

task.VolumeMounts = []*VolumeMount{
{
Volume: "blah",
},
}

err = task.Validate(JobTypeBatch, tg)
requireErrors(t, err,
"Volume Mount (0) references undefined volume blah",
)
task.VolumeMounts = nil

task.Constraints = append(task.Constraints,
&Constraint{
Operand: ConstraintDistinctHosts,
Expand Down
61 changes: 61 additions & 0 deletions nomad/structs/volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package structs

import (
"errors"
"testing"

"github.com/hashicorp/nomad/ci"
Expand Down Expand Up @@ -168,3 +169,63 @@ func TestVolumeMount_Equal(t *testing.T) {
Apply: func(vm *VolumeMount) { vm.PropagationMode = "mode2" },
}})
}

func TestVolumeMount_Validate(t *testing.T) {
ci.Parallel(t)

testCases := []struct {
name string
expectedErr error
volMount *VolumeMount
}{
{
name: "valid volume mount",
volMount: &VolumeMount{
Volume: "vol",
},
expectedErr: nil,
},
{
name: "empty volume reference",
volMount: &VolumeMount{
Volume: "",
},
expectedErr: errVolMountEmptyVol,
},
{
name: "invalid propagation mode",
volMount: &VolumeMount{
Volume: "vol",
PropagationMode: "very invalid propagation mode",
},
expectedErr: errVolMountInvalidPropagationMode,
},
{
name: "invalid selinux label",
volMount: &VolumeMount{
Volume: "vol",
PropagationMode: VolumeMountPropagationPrivate,
SELinuxLabel: "very invalid selinux label",
},
expectedErr: errVolMountInvalidSELinuxLabel,
},
{
name: "full valid volume mont",
volMount: &VolumeMount{
Volume: "vol",
PropagationMode: VolumeMountPropagationPrivate,
SELinuxLabel: SELinuxPrivateVolume,
},
expectedErr: nil,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := tc.volMount.Validate()
if !errors.Is(err, tc.expectedErr) {
t.Fatalf("expected error %v, got %v", tc.expectedErr, err)
}
Comment on lines +225 to +228
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be shortened to something like must.ErrorIs(t, tc.expectedErr, err)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must.ErrorIs doesn't work here, probably something with the unwarpping of the errors

})
}
}
59 changes: 49 additions & 10 deletions nomad/structs/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,20 @@ import (

const (
VolumeTypeHost = "host"
)

const (
VolumeMountPropagationPrivate = "private"
VolumeMountPropagationHostToTask = "host-to-task"
VolumeMountPropagationBidirectional = "bidirectional"

SELinuxSharedVolume = "z"
SELinuxPrivateVolume = "Z"
)

func MountPropagationModeIsValid(propagationMode string) bool {
switch propagationMode {
case "", VolumeMountPropagationPrivate, VolumeMountPropagationHostToTask, VolumeMountPropagationBidirectional:
return true
default:
return false
}
}
var (
errVolMountInvalidPropagationMode = fmt.Errorf("volume mount has an invalid propagation mode")
errVolMountInvalidSELinuxLabel = fmt.Errorf("volume mount has an invalid SELinux label")
errVolMountEmptyVol = fmt.Errorf("volume mount references an empty volume")
)

// ClientHostVolumeConfig is used to configure access to host paths on a Nomad Client
type ClientHostVolumeConfig struct {
Expand Down Expand Up @@ -250,6 +248,7 @@ type VolumeMount struct {
Destination string
ReadOnly bool
PropagationMode string
SELinuxLabel string
}

func (v *VolumeMount) Equal(o *VolumeMount) bool {
Expand All @@ -265,7 +264,10 @@ func (v *VolumeMount) Equal(o *VolumeMount) bool {
return false
case v.PropagationMode != o.PropagationMode:
return false
case v.SELinuxLabel != o.SELinuxLabel:
return false
}

return true
}

Expand All @@ -279,6 +281,43 @@ func (v *VolumeMount) Copy() *VolumeMount {
return nv
}

func (v *VolumeMount) Validate() error {
var mErr *multierror.Error

// Validate the task does not reference undefined volume mounts
if v.Volume == "" {
mErr = multierror.Append(mErr, errVolMountEmptyVol)
}

if !v.MountPropagationModeIsValid() {
mErr = multierror.Append(mErr, fmt.Errorf("%w: %q", errVolMountInvalidPropagationMode, v.PropagationMode))
}

if !v.SELinuxLabelIsValid() {
mErr = multierror.Append(mErr, fmt.Errorf("%w: \"%s\"", errVolMountInvalidSELinuxLabel, v.SELinuxLabel))
}

return mErr.ErrorOrNil()
}

func (v *VolumeMount) MountPropagationModeIsValid() bool {
switch v.PropagationMode {
case "", VolumeMountPropagationPrivate, VolumeMountPropagationHostToTask, VolumeMountPropagationBidirectional:
return true
default:
return false
}
}

func (v *VolumeMount) SELinuxLabelIsValid() bool {
switch v.SELinuxLabel {
case "", SELinuxSharedVolume, SELinuxPrivateVolume:
return true
default:
return false
}
}

func CopySliceVolumeMount(s []*VolumeMount) []*VolumeMount {
l := len(s)
if l == 0 {
Expand Down
4 changes: 3 additions & 1 deletion plugins/drivers/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,13 +444,15 @@ type MountConfig struct {
HostPath string
Readonly bool
PropagationMode string
SELinuxLabel string
}

func (m *MountConfig) IsEqual(o *MountConfig) bool {
return m.TaskPath == o.TaskPath &&
m.HostPath == o.HostPath &&
m.Readonly == o.Readonly &&
m.PropagationMode == o.PropagationMode
m.PropagationMode == o.PropagationMode &&
m.SELinuxLabel == o.SELinuxLabel
}

func (m *MountConfig) Copy() *MountConfig {
Expand Down
Loading
Loading