dynamic host volumes: ACL policies #24356
Conversation
tgross
force-pushed
the
dhv-acl-policies
branch
from
b70924c
to
76c0bbf
Compare
That's a good question. Only Variables and Node Pools have an explicit separate of write vs delete today. While I've been working on the RPCs handlers I've been assuming that |
tgross
force-pushed
the
dhv-acl-policies
branch
from
b79ee7d
to
8f5bb60
Compare
|
I've updated the PR and its description as per the discussion between you, me, and @gulducat. |
| @@ -241,6 +246,7 @@ func expandNamespacePolicy(policy string) []string { | |||
| NamespaceCapabilityReadJobScaling, | |||
| NamespaceCapabilityListScalingPolicies, | |||
| NamespaceCapabilityReadScalingPolicy, | |||
| NamespaceCapabilityHostVolumeRead, | |||
There was a problem hiding this comment.
since there's no addition to write below, policy = "write" does not imply host-volume-write (and its descendants) in contrast to CSI?
There was a problem hiding this comment.
Hm, that's how I wrote it in the RFC, but I'm realizing that I missed that policy = "write" gives csi-write-volume. Once you're using a coarse-grained policy you can't subtract capabilities from it. So we can do that here, but I think we should probably have it only imply host-volume-create/read so that the broad policy doesn't imply delete and the quota-breaking register
There was a problem hiding this comment.
is that what's meant by the ACL policies doc stating
When both the policy short hand and a capabilities list are provided, the capabilities are merged.
I suppose deny would win? but aside from that, a merger would be additive, not subtractive.
aside from losing the simplicity of "write means write", "write means create/read" seems like a reasonable trade-off. 👍
There was a problem hiding this comment.
Yeah deny would win but it's coarse-grained so it flattens everything.
aside from losing the simplicity of "write means write", "write means create/read" seems like a reasonable trade-off.
Yeah, I think it's an example of how some early choices in Nomad's design (like how we did coarse-grained ACLs) were perfectly good choices at the time but make for awkward decisions in a more mature product years later 😁
tgross
force-pushed
the
dhv-acl-policies
branch
from
8f5bb60
to
6a57333
Compare
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
tgross
force-pushed
the
dhv-acl-policies
branch
from
6a57333
to
1a4c153
Compare
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
This changeset implements the ACLs required for dynamic host volumes RPCs:
host-volume-writeis a coarse-grained policy that implies all operations.host-volume-registeris the highest fine-grained privilege because it potentially bypasses quotas.host-volume-createis implicitly granted byhost-volume-registerhost-volume-deleteis implicitly granted only byhost-volume-writehost-volume-readis implicitly granted bypolicy = "read",These are namespaced operations, so the testing here is predominantly around parsing and granting of implicit capabilities rather than the well-tested
AllowNamespaceOperationmethod.This changeset does not include any changes to the
host_volumespolicy which we'll need for claiming volumes on job submit (if any). That'll be covered in a later PR.Ref: https://hashicorp.atlassian.net/browse/NET-11549