clean up Polar file: minor functional changes#1113
Merged
Conversation
Collaborator
Author
|
I initially thought I could remove the "external-authenticator" role, but this is one that we assign via the database. |
69 tasks
plotnick
approved these changes
May 25, 2022
plotnick
left a comment
Contributor
There was a problem hiding this comment.
This change makes the roles across the resource hierarchy much more symmetric, which I think helps make them much more understandable.
| # Permissions granted directly by roles on this resource | ||
| "list_children" if "collaborator"; | ||
| "read" if "collaborator"; | ||
| "list_children" if "viewer"; |
Contributor
There was a problem hiding this comment.
Great. I mentioned the previous asymmetry in a comment on #1110, glad to see it go away.
Comment on lines
+330
to
331
| has_permission(actor: AuthenticatedActor, "modify", _resource: Database) | ||
| if actor = USER_DB_INIT; |
Contributor
There was a problem hiding this comment.
Untested, but should work:
Suggested change
| has_permission(actor: AuthenticatedActor, "modify", _resource: Database) | |
| if actor = USER_DB_INIT; | |
| has_permission(USER_DB_INIT, "modify", _resource: Database); |
Collaborator
Author
There was a problem hiding this comment.
Neat! I'll try that in my follow-up where I'm adding some policy tests.
Collaborator
Author
There was a problem hiding this comment.
Interestingly, that gave me:
thread 'authz::policy_test::test_iam_roles' panicked at 'initializing Oso: loading Polar (Oso) config
Caused by:
Invalid rule: has_permission(USER_DB_INIT, "modify", _resource: Database{}); Must match one of the following rule types:
has_permission(actor: Actor{}, _permission: String{}, resource: Resource{});
Failed to match because: Parameter `USER_DB_INIT` expects a Actor type constraint.
USER_DB_INIT: Actor
at line 331, column 1:
331: has_permission(USER_DB_INIT, "modify", _resource: Database);
^
', nexus/src/authz/context.rs:33:54
but if I make it:
has_permission(USER_DB_INIT: AuthenticatedActor, "modify", _resource: Database);
that seems to work.
leftwo
pushed a commit
that referenced
this pull request
Jan 28, 2024
Crucible changes Remove a superfluous copy during write serialization (#1087) Update to progenitor v0.5.0, pull in required Omicron updates (#1115) Update usdt to v0.5.0 (#1116) Do not panic on reinitialize of a downstairs client. (#1114) Bump (tracing-)opentelemetry(-jaeger) (#1113) Make the Guest -> Upstairs queue fully async (#1086) Switch to per-block ownership (#1107) Handle timeout in the client IO task (#1109) Enforce buffer alignment (#1106) Block size buffers (#1105) New dtrace probes and a counter struct in the Upstairs. (#1104) Implement read decryption offloading (#1089) Remove Arc + Mutex from Buffer (#1094) Comment cleanup and rename of DsState::Repair -> Reconcile (#1102) do not panic the dynamometer for OOB writes (#1101) Allow dsc to start the downstairs in read-only mode. (#1098) Use the omicron-zone-package methods for topo sorting (#1099) Package with topological sorting (#1097) Fix clippy lints in dsc (#1095) Propolis changes: PHD: demote artifact store logs to DEBUG, enable DEBUG on CI (#626) PHD: fix missing newlines in serial.log (#622) PHD: fix run_shell_command with multiline commands (#621) PHD: fix `--artifact-directory` not doing anything (#618) Update h2 dependency Update Crucible (and Omicron) dependencies PHD: refactor guest serial console handling (#615) phd: add basic "migration-from-base" tests + machinery (#609) phd: Ensure min disk size fits read-only parents (#611) phd: automatically fetch `crucible-downstairs` from Buildomat (#604) Mitigate behavior from illumos#16183 PHD: add guest adapter for WS2022 (#607) phd: include error cause chain in failure output (#606) add QEMU pvpanic ISA device (#596) Add crucible-mem backend Make crucible opt parsing more terse in standalone
leftwo
added a commit
that referenced
this pull request
Jan 29, 2024
Crucible changes Remove a superfluous copy during write serialization (#1087) Update to progenitor v0.5.0, pull in required Omicron updates (#1115) Update usdt to v0.5.0 (#1116) Do not panic on reinitialize of a downstairs client. (#1114) Bump (tracing-)opentelemetry(-jaeger) (#1113) Make the Guest -> Upstairs queue fully async (#1086) Switch to per-block ownership (#1107) Handle timeout in the client IO task (#1109) Enforce buffer alignment (#1106) Block size buffers (#1105) New dtrace probes and a counter struct in the Upstairs. (#1104) Implement read decryption offloading (#1089) Remove Arc + Mutex from Buffer (#1094) Comment cleanup and rename of DsState::Repair -> Reconcile (#1102) do not panic the dynamometer for OOB writes (#1101) Allow dsc to start the downstairs in read-only mode. (#1098) Use the omicron-zone-package methods for topo sorting (#1099) Package with topological sorting (#1097) Fix clippy lints in dsc (#1095) Propolis changes: PHD: demote artifact store logs to DEBUG, enable DEBUG on CI (#626) PHD: fix missing newlines in serial.log (#622) PHD: fix run_shell_command with multiline commands (#621) PHD: fix `--artifact-directory` not doing anything (#618) Update h2 dependency Update Crucible (and Omicron) dependencies PHD: refactor guest serial console handling (#615) phd: add basic "migration-from-base" tests + machinery (#609) phd: Ensure min disk size fits read-only parents (#611) phd: automatically fetch `crucible-downstairs` from Buildomat (#604) Mitigate behavior from illumos#16183 PHD: add guest adapter for WS2022 (#607) phd: include error cause chain in failure output (#606) add QEMU pvpanic ISA device (#596) Add crucible-mem backend Make crucible opt parsing more terse in standalone Co-authored-by: Alan Hanson <alan@oxide.computer>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This follows on #1112 but makes some (very small) changes to the policy file:
Databaseshouldn't bother with roles, and more generally none of the synthetic resources should. I think it's confusing to use roles to define the policy for things where we don't actually support assigning those roles.viewerrole. I didn't bother with this initially but at this point it's unnecessarily error-prone not to have it because the definitions ofOrganizationandProjectwere very subtly different from each other. Now, the definitions of Silo, Organization, and Project are basically identical aside from the parent relationship type.