Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw: Add subuser to RemoteApplier for Keystone #33210

Closed
wants to merge 2 commits into from

Conversation

clwluvw
Copy link
Contributor

@clwluvw clwluvw commented Feb 11, 2020

@clwluvw clwluvw force-pushed the remoteapplier-subuser branch 4 times, most recently from 9aa8e4d to 23c2f80 Compare February 11, 2020 17:09
@clwluvw clwluvw force-pushed the remoteapplier-subuser branch 8 times, most recently from a0ffbb4 to d72dfb5 Compare February 12, 2020 00:22
@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 17, 2020

@pritha-srivastava
Copy link
Contributor

When can a remote user have subusers?

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 18, 2020

@pritha-srivastava In case of using project in Keystone we can have multiple users in project so we can have user for project in radosgw and users in project can be subuser for better management on access and stats usage.

@pritha-srivastava
Copy link
Contributor

@pritha-srivastava In case of using project in Keystone we can have multiple users in project so we can have user for project in radosgw and users in project can be subuser for better management on access and stats usage.

Does Openstack Keystone allow creation of such users? Are you talking about Swift use cases?

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 18, 2020

@pritha-srivastava We can have both S3 and swift subuser. And we can have users in project for Keystone.

@pritha-srivastava
Copy link
Contributor

@pritha-srivastava We can have both S3 and swift subuser. And we can have users in project for Keystone.

In case of Remote users, subusers will be created (according to the code you added) only when the incoming user has subusers, so can an incoming Keystone user have subusers?

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 18, 2020

@pritha-srivastava No users in Keystone can't have subusers but projects in Keystone has subusers.

@pritha-srivastava
Copy link
Contributor

@pritha-srivastava No users in Keystone can't have subusers but projects in Keystone has subusers.

Ok

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 18, 2020

@pritha-srivastava Are you agree with these changes?

@pritha-srivastava
Copy link
Contributor

@pritha-srivastava Are you agree with these changes?

How about rgw::auth::RemoteApplier::is_identity() method - doesn't it need checks for subusers?

@clwluvw clwluvw force-pushed the remoteapplier-subuser branch 2 times, most recently from 8ba231a to 2d56aa5 Compare February 18, 2020 19:01
@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 18, 2020

@pritha-srivastava Yes you are right. Done.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
@pritha-srivastava
Copy link
Contributor

looks good to me.

@pritha-srivastava Thanks. Any plan to be merged?

What are the permissions of a subuser with respect to the project? Are we going to give FULL_CONTROL to the subusers also?

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 20, 2020

What are the permissions of a subuser with respect to the project? Are we going to give FULL_CONTROL to the subusers also?

@pritha-srivastava I have added AuthInfo perm_mask to subuser. So we will set permission that AuthInfo have to subuser. Sorry for this mistake.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 20, 2020

jenkins test make check

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 21, 2020

@pritha-srivastava Are you okay with this now?

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 25, 2020

Ping @pritha-srivastava :)

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 25, 2020

Ping @mdw-at-linuxbox

@clwluvw clwluvw requested a review from cbodley February 25, 2020 20:58
@@ -129,6 +129,7 @@ TokenEngine::get_creds_info(const TokenEngine::token_envelope_t& token,
rgw_user(token.get_project_id()),
/* User's display name (aka real name). */
token.get_project_name(),
token.get_user_name(),
Copy link
Contributor

Choose a reason for hiding this comment

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

How about the perm_mask here? It returns RGW_PERM_FULL_CONTROL here, will the subuser also have RGW_PERM_FULL_CONTROL? @cbodley @mattbenjamin : Radoslaw has left a comment that 'Keystone doesn't support RGW's subuser concept' (see below) Any idea why?

Copy link
Contributor Author

@clwluvw clwluvw Feb 26, 2020

Choose a reason for hiding this comment

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

Yes because in Keystone response we don't have any field that represent permission for users we give RGW_PERM_FULL_CONTROL to all keystone subusers and here we set the AuthInfo perm_mask to RGWSubUser perm_mask.

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 28, 2020

@cbodley @mattbenjamin Would you mind to review this please?

@mattbenjamin
Copy link
Contributor

@clwluvw pritha expressed concern that this PR defines "subuser" for keystone when keystone does not have subusers. Could someone fill in the rationale for this change specifically? What does it fix?

@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 28, 2020

@mattbenjamin Keystone doesn't specifically has subuser but you can have project in Keystone and users in that project can be subuser here. So we can have this structure in radosgw to support subuser.

@mattbenjamin
Copy link
Contributor

@mdw-at-linuxbox can you weigh in here?

@clwluvw
Copy link
Contributor Author

clwluvw commented Mar 8, 2020

@mattbenjamin Any alternative can weigh in here? :)

@clwluvw
Copy link
Contributor Author

clwluvw commented Mar 9, 2020

@mattbenjamin Sorry for mentioning you again but can we weight up this? I think this could be really good feature that support radosgw subuser with external auth like Keystone. subuser in radosgw is really wonderful feature for user managing and it will so amazing to have it through Keystone auth, too.
Thanks :)

@clwluvw
Copy link
Contributor Author

clwluvw commented Mar 10, 2020

@mattbenjamin We can also use domain as a user instead of project but I see this sentence in project description that matches to radosgw subuser concept I think.

A project is a group of zero or more users.

I think if we use project as a user we can have user management like keystone do. Do you have any idea on this work? :)

@clwluvw
Copy link
Contributor Author

clwluvw commented Mar 13, 2020

Ping @mattbenjamin :)

@clwluvw
Copy link
Contributor Author

clwluvw commented Mar 16, 2020

Ping @mdw-at-linuxbox

@clwluvw
Copy link
Contributor Author

clwluvw commented Mar 26, 2020

@mattbenjamin It's about 1 month we have requested @mdw-at-linuxbox to review this PR. Can we get help from someone else?

@stale
Copy link

stale bot commented May 25, 2020

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label May 25, 2020
@adamemerson adamemerson removed their request for review June 16, 2020 17:59
@stale stale bot removed the stale label Jun 16, 2020
@stale
Copy link

stale bot commented Aug 15, 2020

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Aug 15, 2020
@stale
Copy link

stale bot commented Nov 16, 2020

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@stale stale bot closed this Nov 16, 2020
@frittentheke frittentheke mentioned this pull request Feb 14, 2022
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants