Skip to content

IAM: support assigning roles on Projects#1042

Merged
davepacheco merged 2 commits into
mainfrom
role-assignments
May 9, 2022
Merged

IAM: support assigning roles on Projects#1042
davepacheco merged 2 commits into
mainfrom
role-assignments

Conversation

@davepacheco

@davepacheco davepacheco commented May 9, 2022

Copy link
Copy Markdown
Collaborator

This is a pretty straightforward extension of #1002 that adds support for assigning roles on Projects.

@davepacheco davepacheco requested a review from plotnick May 9, 2022 17:18
@davepacheco davepacheco mentioned this pull request May 9, 2022
69 tasks

@plotnick plotnick left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great. Nice job setting up the datastore layer so that this just worked with no modifications. As in pool, the tricky bit is not just getting this shot in, but setting up the next one, too.

opctx: &OpContext,
organization_name: &Name,
) -> LookupResult<shared::Policy<OrganizationRoles>> {
) -> LookupResult<shared::Policy<authz::OrganizationRoles>> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is definitely clearer with the explicit authz package prefix.

Comment thread openapi/nexus.json
]
},
"ProjectRolesPolicy": {
"description": "Client view of a [`Policy`], which describes how this resource may be accessed\n\nNote that the Policy only describes access granted explicitly for this resource. The policies of parent resources can also cause a user to have access to this resource.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really nice description, should help avoid some end-user questions.

let org_name = "test-org";
create_organization(client, org_name).await;
let org_url = format!("/organizations/{}", org_name);
do_test::<_, views::Organization>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I'd prefer an explicit type for T here (and below on line 47) rather than a wildcard. Not a big deal, just might save some brain cycles when reading.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Will do!

@davepacheco davepacheco enabled auto-merge (squash) May 9, 2022 17:59
@davepacheco davepacheco merged commit f5a24d1 into main May 9, 2022
@davepacheco davepacheco deleted the role-assignments branch May 9, 2022 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants