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: fixes for virtual hosting of buckets #11280

Merged
merged 5 commits into from Nov 1, 2016
Merged

rgw: fixes for virtual hosting of buckets #11280

merged 5 commits into from Nov 1, 2016

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Sep 30, 2016

this brings back the fix from @robbat2 that was reverted in #10873, with an additional check that disables the virtual hosting feature if no hostnames were configured. this prevents default configurations (such as teuthology) from treating all requests as virtual hosted buckets

also updated the docs - a preview should eventually be available at http://docs.ceph.com/docs/wip-17440/radosgw/s3/commons/

Copy link
Contributor

@robbat2 robbat2 left a comment

Choose a reason for hiding this comment

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

One comment about the other set variable for staticsites hostnames below.

Also, maybe I'm not seeing it, but we don't actually enforce that any of the entries from the zonegroup hostnames are non-empty; they just seem to be copied from the zonegroup to the set variables.

if (subdomain.empty()
&& (domain.empty() || domain != info.host)
&& !looks_like_ip_address(info.host.c_str())
&& RGWHandler_REST::validate_bucket_name(info.host)) {
&& RGWHandler_REST::validate_bucket_name(info.host) == 0
&& !hostnames_set.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should check hostnames_s3website_set as well. If either of them are non-empty, it should be ok (should be very rare, but want to check anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Robin H. Johnson and others added 5 commits October 3, 2016 14:25
The logic (added in 46aae19) for falling back to just using the hostname as
the possible bucket name contained an accidental inversion, because
RGWHandler_REST::validate_bucket_name returns success as zero.

Backport: jewel
Fixes: http://tracker.ceph.com/issues/17136
Re-Fixes: http://tracker.ceph.com/issues/15975
Signed-off-by: Robin H. Johnson <robin.johnson@dreamhost.com>
if no hostnames are configured, all requests were treated as virtual
hosted buckets. require at least one hostname in hostnames_set to
consider setting in_hosted_domain

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

Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
std::accumulate() was appending hostnames without commas or spaces

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@robbat2
Copy link
Contributor

robbat2 commented Oct 3, 2016

I ran into one related impact from this, that should be clarified & documented.
RGW's legacy behavior differs from S3 behavior, and s3-tests was designed around the RGW legacy behavior: using the path calling format, and assuming that the hostname it was connecting to was also configured as the rgw dns name or zonegroup hostname.

Thus, in some cases, we know get a lot more false-positive s3-tests breakage.

GET /foobar/ HTTP/1.1
Host: non-existing-ceph-buckettest.example.com

AWS-S3 treats this as bucket {{non-existing-ceph-buckettest.example.com}}, and returns:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>NoSuchBucket</Code>
<Message>The specified bucket does not exist</Message>
<BucketName>non-existing-ceph-buckettest.example.com</BucketName>
<RequestId>D1F9F7DCD9D79A11</RequestId>
<HostId>1RGXmTwGt4w2R+BV/xiuJsmJoIwVZsjhwEBH2Qrqh4URbV0zlIvX1+9KczHCCKHnhtV7Mhh7Ps8=</HostId>
</Error>

RGW, prior to my patching, treats this as bucket {{foobar}}, and returns:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>NoSuchBucket</Code>
<BucketName>foobar</BucketName>
<RequestId>tx0000000000000000025fb-0057f2e73e-fa74-default</RequestId>
<HostId>fa74-default-default</HostId>
</Error>

RGW, with patch, now matches AWS-S3:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>NoSuchBucket</Code>
<BucketName>non-existing-ceph-buckettest.example.com</BucketName>
<RequestId>tx0000000000000000c6085-0057f2e721-10bb00c9-default</RequestId>
<HostId>10bb00c9-default-default</HostId>
</Error>

@cbodley cbodley closed this Oct 10, 2016
@cbodley cbodley deleted the wip-17440 branch October 10, 2016 15:08
@cbodley cbodley restored the wip-17440 branch October 13, 2016 18:08
@cbodley
Copy link
Contributor Author

cbodley commented Oct 13, 2016

oops, i was cleaning up branches and nuked this one. reopened

@robbat2
Copy link
Contributor

robbat2 commented Oct 26, 2016

@cbodley @yehudasa what happened to having this merged in v10.2.4? I'm writing documentation for StaticSites, and it depends on having this merged.

@oritwas
Copy link
Member

oritwas commented Oct 28, 2016

@oritwas
Copy link
Member

oritwas commented Oct 31, 2016

jenkins test this please

@oritwas
Copy link
Member

oritwas commented Nov 1, 2016

new run (previous runs had enviorment problems): http://pulpito.ceph.com/owasserm-2016-10-31_18:27:38-rgw-wip-orit-17440---basic-mira/

@oritwas oritwas merged commit 63e86e5 into master Nov 1, 2016
@cbodley cbodley deleted the wip-17440 branch November 1, 2016 13:59
@cbodley
Copy link
Contributor Author

cbodley commented Nov 1, 2016

thanks @oritwas !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants