Skip to content

Add PUT/HEAD/DELETE for identity/v3/OS-INHERIT#2610

Merged
mandre merged 4 commits intogophercloud:masterfrom
nikParasyr:os-inherit
May 15, 2023
Merged

Add PUT/HEAD/DELETE for identity/v3/OS-INHERIT#2610
mandre merged 4 commits intogophercloud:masterfrom
nikParasyr:os-inherit

Conversation

@nikParasyr
Copy link
Copy Markdown
Contributor

@nikParasyr nikParasyr commented Apr 28, 2023

Add PUT/HEAD/DELETE for indentity/v3/OS-INHERIT. Follow the same pattern as normal role assignments for consistency.

This adds PUT/HEAD/DELETE for all combinations of domain/project and user/group.

DOCS
Fixes #2205

Each commit adds one API call, thus it is a bit easier to review by reviewing one commit each time

Add PUT for indentity/v3/OS-INHERIT. Follow the same pattern
as normal role assignments for consistensy and minimalization.
This adds PUT for all combinations of domain/project and user/group.
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 28, 2023

Coverage Status

Coverage: 79.088% (+0.03%) from 79.06% when pulling 6accee0 on nikParasyr:os-inherit into 7137f08 on gophercloud:master.

Add HEAD to identity/v3/OS-INHERIT. These calls can be used
to validate the existance of various inherited roles. Follow
similar conventions as identity/v3/roles for consistency
Add DELETE for identity/v3/OS-INHERIT. These calls can be used
to unassign various inherited roles. Follow similar conventions
as identity/v3/roles for consistency
@nikParasyr nikParasyr changed the title Add PUT for indentity/v3/OS-INHERIT Add PUT/HEAD/DELETE for indentity/v3/OS-INHERIT Apr 28, 2023
@nikParasyr
Copy link
Copy Markdown
Contributor Author

@pierreprinetti and/or @mandre could you check this one?
unit tests failing are not due to my changes but due to bad response status from coveralls: 500. The unit tests pass before that.

I've grouped it into a single PR, but hopefully reviewing commit-by-commit will make it easier.

thanks

@nikParasyr nikParasyr marked this pull request as ready for review April 29, 2023 08:28
mandre
mandre previously approved these changes May 12, 2023
Copy link
Copy Markdown
Contributor

@mandre mandre left a comment

Choose a reason for hiding this comment

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

This looks perfect, apart for a minor typo. Really well done, as usual.

defer th.TeardownHTTP()
HandleAssignSuccessfully(t)

err := osinherit.Assign(client.ServiceClient(), "{role_id}", osinherit.AssignOpts{
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.

Shouldn't we also test known failure cases, such as providing both UserID and GroupID, to ensure the validation works?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

err = osinherit.Unassign(client, role.ID, unassignOpts).ExtractErr()
th.AssertNoErr(t, err)

t.Logf("Successfully unassgined inherited role %s to a user %s on a project %s",
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.

Small typo -> unassigned

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

err = osinherit.Unassign(client, role.ID, unassignOpts).ExtractErr()
th.AssertNoErr(t, err)

t.Logf("Successfully unassgined inherited role %s to a group %s on a project %s",
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.

ditto

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@nikParasyr
Copy link
Copy Markdown
Contributor Author

@mandre Thanks for the review. Added the changes you requested.

@mandre mandre added this to the v1.4.0 milestone May 15, 2023
Copy link
Copy Markdown
Contributor

@mandre mandre left a comment

Choose a reason for hiding this comment

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

Perfect, thanks.

@mandre mandre merged commit 894ad9d into gophercloud:master May 15, 2023
@mandre mandre changed the title Add PUT/HEAD/DELETE for indentity/v3/OS-INHERIT Add PUT/HEAD/DELETE for identity/v3/OS-INHERIT May 25, 2023
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.

Identity V3: support parent project roles inheritance (OS-INHERIT)

3 participants