Skip to content

Commit

Permalink
dynamic host volumes: ACL policies (#24356)
Browse files Browse the repository at this point in the history 
This changeset implements the ACLs required for dynamic host volumes RPCs:
* `host-volume-write` is a coarse-grained policy that implies all operations.
* `host-volume-register` is the highest fine-grained privilege because it
  potentially bypasses quotas.
* `host-volume-create` is implicitly granted by `host-volume-register`
* `host-volume-delete` is implicitly granted only by `host-volume-write`
* `host-volume-read` is implicitly granted by `policy = "read"`,

These are namespaced operations, so the testing here is predominantly around
parsing and granting of implicit capabilities rather than the well-tested
`AllowNamespaceOperation` method.

This changeset does not include any changes to the `host_volumes` policy which
we'll need for claiming volumes on job submit. That'll be covered in a later PR.

Ref: https://hashicorp.atlassian.net/browse/NET-11549
  • Loading branch information
@tgross
tgross committed Nov 5, 2024
1 parent a8b84a6 commit 3a39b6b
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 18 deletions.
4 changes: 4 additions & 0 deletions acl/acl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,12 @@ func TestACLManagement(t *testing.T) {
// Check default namespace rights
must.True(t, acl.AllowNamespaceOperation("default", NamespaceCapabilityListJobs))
must.True(t, acl.AllowNamespaceOperation("default", NamespaceCapabilitySubmitJob))
must.True(t, acl.AllowNamespaceOperation("default", NamespaceCapabilityHostVolumeCreate))
must.True(t, acl.AllowNamespace("default"))

// Check non-specified namespace
must.True(t, acl.AllowNamespaceOperation("foo", NamespaceCapabilityListJobs))
must.True(t, acl.AllowNamespaceOperation("foo", NamespaceCapabilityHostVolumeCreate))
must.True(t, acl.AllowNamespace("foo"))

// Check node pool rights.
Expand Down Expand Up @@ -155,9 +157,11 @@ func TestACLMerge(t *testing.T) {
// Check default namespace rights
must.True(t, acl.AllowNamespaceOperation("default", NamespaceCapabilityListJobs))
must.False(t, acl.AllowNamespaceOperation("default", NamespaceCapabilitySubmitJob))
must.False(t, acl.AllowNamespaceOperation("default", NamespaceCapabilityHostVolumeRegister))

// Check non-specified namespace
must.False(t, acl.AllowNamespaceOperation("foo", NamespaceCapabilityListJobs))
must.False(t, acl.AllowNamespaceOperation("foo", NamespaceCapabilityHostVolumeCreate))

// Check rights in the node pool specified in policies.
must.True(t, acl.AllowNodePoolOperation("my-pool", NodePoolCapabilityRead))
Expand Down
38 changes: 37 additions & 1 deletion acl/policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ const (
NamespaceCapabilityCSIReadVolume = "csi-read-volume"
NamespaceCapabilityCSIListVolume = "csi-list-volume"
NamespaceCapabilityCSIMountVolume = "csi-mount-volume"
NamespaceCapabilityHostVolumeCreate = "host-volume-create"
NamespaceCapabilityHostVolumeRegister = "host-volume-register"
NamespaceCapabilityHostVolumeRead = "host-volume-read"
NamespaceCapabilityHostVolumeWrite = "host-volume-write"
NamespaceCapabilityHostVolumeDelete = "host-volume-delete"
NamespaceCapabilityListScalingPolicies = "list-scaling-policies"
NamespaceCapabilityReadScalingPolicy = "read-scaling-policy"
NamespaceCapabilityReadJobScaling = "read-job-scaling"
Expand Down Expand Up @@ -207,7 +212,7 @@ func isNamespaceCapabilityValid(cap string) bool {
NamespaceCapabilityReadFS, NamespaceCapabilityAllocLifecycle,
NamespaceCapabilityAllocExec, NamespaceCapabilityAllocNodeExec,
NamespaceCapabilityCSIReadVolume, NamespaceCapabilityCSIWriteVolume, NamespaceCapabilityCSIListVolume, NamespaceCapabilityCSIMountVolume, NamespaceCapabilityCSIRegisterPlugin,
NamespaceCapabilityListScalingPolicies, NamespaceCapabilityReadScalingPolicy, NamespaceCapabilityReadJobScaling, NamespaceCapabilityScaleJob:
NamespaceCapabilityListScalingPolicies, NamespaceCapabilityReadScalingPolicy, NamespaceCapabilityReadJobScaling, NamespaceCapabilityScaleJob, NamespaceCapabilityHostVolumeCreate, NamespaceCapabilityHostVolumeRegister, NamespaceCapabilityHostVolumeWrite, NamespaceCapabilityHostVolumeRead:
return true
// Separate the enterprise-only capabilities
case NamespaceCapabilitySentinelOverride, NamespaceCapabilitySubmitRecommendation:
Expand Down Expand Up @@ -241,6 +246,7 @@ func expandNamespacePolicy(policy string) []string {
NamespaceCapabilityReadJobScaling,
NamespaceCapabilityListScalingPolicies,
NamespaceCapabilityReadScalingPolicy,
NamespaceCapabilityHostVolumeRead,
}

write := make([]string, len(read))
Expand All @@ -257,6 +263,7 @@ func expandNamespacePolicy(policy string) []string {
NamespaceCapabilityCSIMountVolume,
NamespaceCapabilityCSIWriteVolume,
NamespaceCapabilitySubmitRecommendation,
NamespaceCapabilityHostVolumeCreate,
}...)

switch policy {
Expand All @@ -278,6 +285,32 @@ func expandNamespacePolicy(policy string) []string {
}
}

// expandNamespaceCapabilities adds extra capabilities implied by fine-grained
// capabilities.
func expandNamespaceCapabilities(ns *NamespacePolicy) {
extraCaps := []string{}
for _, cap := range ns.Capabilities {
switch cap {
case NamespaceCapabilityHostVolumeWrite:
extraCaps = append(extraCaps,
NamespaceCapabilityHostVolumeRegister,
NamespaceCapabilityHostVolumeCreate,
NamespaceCapabilityHostVolumeDelete,
NamespaceCapabilityHostVolumeRead)
case NamespaceCapabilityHostVolumeRegister:
extraCaps = append(extraCaps,
NamespaceCapabilityHostVolumeCreate,
NamespaceCapabilityHostVolumeRead)
case NamespaceCapabilityHostVolumeCreate:
extraCaps = append(extraCaps, NamespaceCapabilityHostVolumeRead)
}
}

// These may end up being duplicated, but they'll get deduplicated in NewACL
// when inserted into the radix tree.
ns.Capabilities = append(ns.Capabilities, extraCaps...)
}

func isNodePoolCapabilityValid(cap string) bool {
switch cap {
case NodePoolCapabilityDelete, NodePoolCapabilityRead, NodePoolCapabilityWrite,
Expand Down Expand Up @@ -388,6 +421,9 @@ func Parse(rules string) (*Policy, error) {
ns.Capabilities = append(ns.Capabilities, extraCap...)
}

// Expand implicit capabilities
expandNamespaceCapabilities(ns)

if ns.Variables != nil {
if len(ns.Variables.Paths) == 0 {
return nil, fmt.Errorf("Invalid variable policy: no variable paths in namespace %s", ns.Name)
Expand Down
86 changes: 69 additions & 17 deletions acl/policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package acl

import (
"fmt"
"strings"
"testing"

"github.com/hashicorp/nomad/ci"
Expand All @@ -17,9 +16,9 @@ func TestParse(t *testing.T) {
ci.Parallel(t)

type tcase struct {
Raw string
ErrStr string
Expect *Policy
Raw string
ExpectErr string
Expect *Policy
}
tcases := []tcase{
{
Expand All @@ -43,6 +42,7 @@ func TestParse(t *testing.T) {
NamespaceCapabilityReadJobScaling,
NamespaceCapabilityListScalingPolicies,
NamespaceCapabilityReadScalingPolicy,
NamespaceCapabilityHostVolumeRead,
},
},
},
Expand Down Expand Up @@ -118,6 +118,7 @@ func TestParse(t *testing.T) {
NamespaceCapabilityReadJobScaling,
NamespaceCapabilityListScalingPolicies,
NamespaceCapabilityReadScalingPolicy,
NamespaceCapabilityHostVolumeRead,
},
},
{
Expand All @@ -132,6 +133,7 @@ func TestParse(t *testing.T) {
NamespaceCapabilityReadJobScaling,
NamespaceCapabilityListScalingPolicies,
NamespaceCapabilityReadScalingPolicy,
NamespaceCapabilityHostVolumeRead,
NamespaceCapabilityScaleJob,
NamespaceCapabilitySubmitJob,
NamespaceCapabilityDispatchJob,
Expand All @@ -142,6 +144,8 @@ func TestParse(t *testing.T) {
NamespaceCapabilityCSIMountVolume,
NamespaceCapabilityCSIWriteVolume,
NamespaceCapabilitySubmitRecommendation,
NamespaceCapabilityHostVolumeCreate,
NamespaceCapabilityHostVolumeRead,
},
},
{
Expand Down Expand Up @@ -338,6 +342,7 @@ func TestParse(t *testing.T) {
NamespaceCapabilityReadJobScaling,
NamespaceCapabilityListScalingPolicies,
NamespaceCapabilityReadScalingPolicy,
NamespaceCapabilityHostVolumeRead,
},
},
{
Expand All @@ -352,6 +357,7 @@ func TestParse(t *testing.T) {
NamespaceCapabilityReadJobScaling,
NamespaceCapabilityListScalingPolicies,
NamespaceCapabilityReadScalingPolicy,
NamespaceCapabilityHostVolumeRead,
NamespaceCapabilityScaleJob,
NamespaceCapabilitySubmitJob,
NamespaceCapabilityDispatchJob,
Expand All @@ -362,6 +368,8 @@ func TestParse(t *testing.T) {
NamespaceCapabilityCSIMountVolume,
NamespaceCapabilityCSIWriteVolume,
NamespaceCapabilitySubmitRecommendation,
NamespaceCapabilityHostVolumeCreate,
NamespaceCapabilityHostVolumeRead,
},
},
{
Expand Down Expand Up @@ -638,6 +646,54 @@ func TestParse(t *testing.T) {
},
},
},
{
`
namespace "default" {
capabilities = ["host-volume-register"]
}
namespace "other" {
capabilities = ["host-volume-create"]
}
namespace "foo" {
capabilities = ["host-volume-write"]
}
`,
"",
&Policy{
Namespaces: []*NamespacePolicy{
{
Name: "default",
Policy: "",
Capabilities: []string{
NamespaceCapabilityHostVolumeRegister,
NamespaceCapabilityHostVolumeCreate,
NamespaceCapabilityHostVolumeRead,
},
},
{
Name: "other",
Policy: "",
Capabilities: []string{
NamespaceCapabilityHostVolumeCreate,
NamespaceCapabilityHostVolumeRead,
},
},
{
Name: "foo",
Policy: "",
Capabilities: []string{
NamespaceCapabilityHostVolumeWrite,
NamespaceCapabilityHostVolumeRegister,
NamespaceCapabilityHostVolumeCreate,
NamespaceCapabilityHostVolumeDelete,
NamespaceCapabilityHostVolumeRead,
},
},
},
},
},
{
`
node_pool "pool-read-only" {
Expand Down Expand Up @@ -878,22 +934,18 @@ func TestParse(t *testing.T) {
}

for idx, tc := range tcases {
t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
t.Run(fmt.Sprintf("%02d", idx), func(t *testing.T) {
p, err := Parse(tc.Raw)
if err != nil {
if tc.ErrStr == "" {
t.Fatalf("Unexpected err: %v", err)
}
if !strings.Contains(err.Error(), tc.ErrStr) {
t.Fatalf("Unexpected err: %v", err)
}
return
if tc.ExpectErr == "" {
must.NoError(t, err)
} else {
must.ErrorContains(t, err, tc.ExpectErr)
}
if err == nil && tc.ErrStr != "" {
t.Fatalf("Missing expected err")

if tc.Expect != nil {
tc.Expect.Raw = tc.Raw
must.Eq(t, tc.Expect, p)
}
tc.Expect.Raw = tc.Raw
assert.EqualValues(t, tc.Expect, p)
})
}
}
Expand Down

0 comments on commit 3a39b6b

Please sign in to comment.