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: swift: the http referer acl in swift API should be shown #13003

Merged
merged 1 commit into from Jan 30, 2017

Conversation

Jing-Scott
Copy link
Contributor

The container acl about http referer set should be shown in container metadata dump.

Signed-off-by: Jing Wenjun jingwenjun@cmss.chinamobile.com

@Jing-Scott
Copy link
Contributor Author

@rzarzynski Hi
Can you help to review? Thanks.

@Jing-Scott
Copy link
Contributor Author

@mattbenjamin @cbodley @rzarzynski
Hi, all
Could you help to take a look at this. Thanks.
I've test the http referer feature in openstack acl. ceph swift is not compatible very well. I tried to fix them.
Another bug fix is in #13005 .

@rzarzynski rzarzynski self-assigned this Jan 24, 2017
Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

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

Just one minor thing. In overall the patch brings a lot of improvement for Swift API's referrer handling. Big thanks, @Jing-Scott!


if (perm != 0) {
if(url_spec[0] == '.')
url_spec = "*" + url_spec;
Copy link
Contributor

Choose a reason for hiding this comment

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

Swift handles this a bit different:

$ curl -i "${publicURL}/cont-for-acls" -X POST -H "X-Auth-Token: ${token}" -H "X-Container-Read: .r:*.example.com"
HTTP/1.1 204 No Content
Content-Length: 0
Content-Type: text/html; charset=UTF-8
X-Trans-Id: tx65885b8aa26d4c21bf582-0058878fc9
Date: Tue, 24 Jan 2017 17:32:57 GMT

$ curl -i "${publicURL}/cont-for-acls" -X HEAD -H "X-Auth-Token: ${token}"
HTTP/1.1 204 No Content
Content-Length: 0
X-Container-Object-Count: 0
Accept-Ranges: bytes
X-Timestamp: 1484935150.21775
X-Container-Read: .r:.example.com
X-Container-Bytes-Used: 0
Content-Type: text/plain; charset=utf-8
X-Trans-Id: tx8be1cb3d36544cd385a91-0058878fd0
Date: Tue, 24 Jan 2017 17:33:04 GMT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rzarzynski
yeah, you are right. I've updated it, pls check it. Thanks a lot!

The container acl about http referer set should be shown in container metadata dump.

Fixes: http://tracker.ceph.com/issues/18665
Signed-off-by: Jing Wenjun <jingwenjun@cmss.chinamobile.com>
id = SWIFT_GROUP_ALL_USERS;
} else {
url_spec = grant.get_referer();
if (url_spec.empty())
Copy link
Contributor

Choose a reason for hiding this comment

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

May we have braces around this if as well?

url_spec = grant.get_referer();
if (url_spec.empty())
continue;
id = (perm !=0) ? ".r:" + url_spec : ".r:-" + url_spec;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missed space before 0.

@rzarzynski
Copy link
Contributor

@Jing-Scott: looks good to me, thanks! Those comments above are only about formatting, nothing more. I'm taking the change for testing.

For the sake of backporting could you please squash the commits, create a ticket on RadosGW's tracker and put the Fixes tag into the commit message? I think we want to have the fix also in Kraken and Jewel.

@Jing-Scott Jing-Scott force-pushed the dev/swift-container-acl-info branch 2 times, most recently from ce49fb4 to 3860dda Compare January 25, 2017 13:24
@Jing-Scott
Copy link
Contributor Author

Hi, @rzarzynski
I've squashed them and created the bug in tracker which is included in my squashed commit message.
Thanks a lot for your check! ^_^

Fixes: http://tracker.ceph.com/issues/18665

Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@rzarzynski
Copy link
Contributor

jenkins test this please

@rzarzynski
Copy link
Contributor

The Teuthology run looks good to me. There is one but unrelated failure. Merging.

@rzarzynski rzarzynski merged commit 034a6f9 into ceph:master Jan 30, 2017
@Jing-Scott Jing-Scott deleted the dev/swift-container-acl-info branch February 2, 2017 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants