rgw: support admin credentials in S3-related Keystone authentication. #6131
Conversation
rzarzynski
force-pushed
the
wip-admincreds-in-s3-keystone
branch
from
347c45f
to
8ff6e6d
Compare
|
@yehudasa number of lines affected by this PR has been decreased several times. Previous version is available as well. |
|
@rzarzynski great, much better! |
|
Looks good to me, but somewhat overcommented perhaps? There's a place where comment literally says "get auth token from Keystone" and the function name is get_keystone_admin_token(). |
rzarzynski
force-pushed
the
wip-admincreds-in-s3-keystone
branch
from
8ff6e6d
to
a84bd48
Compare
rzarzynski
force-pushed
the
wip-admincreds-in-s3-keystone
branch
from
a84bd48
to
d2150bf
Compare
|
@yehudasa rebased to master and improved spacing according to Matt's comment. Today I've tested the patch using s3-tests, Tempest and manually against radosgw in many different configurations (Keystone/admin token, Keystone/admin credentials, internal RADOS-based auth). Tests showed there is no regression here. IMO the PR is ready to merge. |
| string admin_token_id; | ||
| int r = RGWSwift::get_keystone_admin_token(cct, admin_token_id); | ||
| if (r < 0) { | ||
| dout(2) << "s3 keystone: cannot get token for keystone access" << dendl; |
There was a problem hiding this comment.
@rzarzynski one small nitpick, please change this dout(2) to ldout(cct, 2). Once that's done I'll merge it.
Fixes: ceph#13302 Backport: infernalis, hammer Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rzarzynski
force-pushed
the
wip-admincreds-in-s3-keystone
branch
from
d2150bf
to
136433d
Compare
|
@yehudasa thanks for pointing the dout. Pushed amended commit. |
rgw: support admin credentials in S3-related Keystone authentication. Reviewed-by: Yehuda Sadeh <yehuda@redhat.com>
No description provided.