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: configurable enforcement of container naming rules #18219

Closed
wants to merge 2 commits into from

Conversation

theanalyst
Copy link
Member

@theanalyst theanalyst commented Oct 10, 2017

This is a patchset addressing two issues

  • rgw: allow buckets created with / to be useable

Previously we didn't restrict "/" in bucket names created via swift api
though swift client itself had a restriction, and cases like misconfigured
keystone urls would cause swift client also to prepend a
"/" before the bucket name. The problem is that create bucket succeeds such
a call and rgw_get_bucket_instance_info() and related functions would fail
because we replace the first encountered "/" with a ":" assuming that this
was a(n empty) tenant name. Make these buckets useable/ deleteable by
making sure that when we retreive the key back we replace the beginning /
with a colon as this was used in storing the bucket info. A non backwards
compatible cleaner change would be to make rgw_bucket_instance_key_to_oid not
replace slashes when 0 sized tenants were encountered. python-swiftclient/swift cli are
still broken when uploading objects etc. as they do not prepend the / in
PUT object requests whereas do in create bucket requests. However this
patch makes it possible to use/delete these created buckets when using the
rest api.

Note that however there is still an edge case which cannot be addressed,
which is, currently an object upload request of the form PUT /bucket/foo
would end up with another bucket with the name /bucket/foo being created.
With this patch however we ensure that a PUT /bucket/foo is indeed
considered as an object request, however this introduces the problem where
previously a bucket was created with such a name which would no longer be
accessible. There is no way to address this since object names are allowed
to contain slashes and we are now just allowing the case where a prefix
slash at bucket name is allowed (since we hadn't restricted that before), a
slash in the middle would end up with the part after being considered as an
object request, which is the convention.

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

  • rgw: add a configurable for enforcing swift bucket name validation

Swift containers are declared in the upstream documentation as 1-256 bytes
of UTF-8, and the only forbidden character is '/'.

We did not previously enforce the no-slash rule.

Adding a configurable rgw_swift_enforce_bucket_names which validates
according to swift container naming restrictions (which currently forbids
"/" in container names), also dropping the 0xff check that we previously
had since check_utf8 should already catch that

Making this as a configurable with the default that we enforce the swift
naming rules of no slash, the configurable is useful in cases where when
previously created buckets with slashes exists and need to be used

See-Also:
https://docs.openstack.org/developer/swift/api/object_api_v1_overview.html
Fixes: http://tracker.ceph.com/issues/19264

edit: formatting fixes

theanalyst and others added 2 commits October 10, 2017 14:33
Previously we didn't restrict "/" in bucket names created via swift api
though swift client itself had a restriction, and cases like
misconfigured keystone urls would cause swift client also to prepend a
"/" before the bucket name. The problem is that create bucket succeeds
such a call and rgw_get_bucket_instance_info() and related functions
would fail because we replace the first encountered "/" with a ":"
assuming that this was a(n empty) tenant name. Make these buckets
useable/ deleteable by making sure that when we retreive the key back we
replace the beginning / with a colon as this was used in storing the
bucket info. A non backwards compatible change would be to make
rgw_bucket_instance_key_to_oid not replace slashes when 0 sized tenants
were encountered. Swift clients are still broken when uploading objects
etc. as they do not prepend the / in PUT object requests whereas do in
create bucket requests. However this patch makes it possible to
use/delete these created buckets if using a rest api.

Note that however there is still an edge case which cannot be addressed,
which is, currently an object upload request of the form PUT /bucket/foo
would end up with another bucket with the name /bucket/foo being
created. With this patch however we ensure that a PUT /bucket/foo is
indeed considered as an object request, however this introduces the
problem where previously a bucket was created with such a name which
would no longer be accessible. There is no way to address this since
object names are allowed to contain slashes and we are now just allowing
the case where a prefix slash at bucket name is allowed (since we hadn't
restricted that before), a slash in the middle would end up with the
part after being considered as an object request, which is the convention.

Fixes: http://tracker.ceph.com/issues/21679
Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
Swift containers are declared in the upstream documentation as 1-256
bytes of UTF-8, and the only forbidden character is '/'.

We did not previously enforce the no-slash rule.

Adding a configurable `rgw_swift_enforce_bucket_names` which validates
according to swift container naming restrictions (which currently
forbids "/" in container names), also dropping the 0xff check that we
previously had since check_utf8 should already catch that

Making this as a configurable with the default that we enforce the swift
naming rules of no slash, the configurable is useful in cases where
when previously created buckets with slashes exists and need to be used

See-Also: https://docs.openstack.org/developer/swift/api/object_api_v1_overview.html
Fixes: http://tracker.ceph.com/issues/19264

Signed-off-by: Robin H. Johnson <robin.johnson@dreamhost.com>
Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
@theanalyst
Copy link
Member Author

@rzarzynski @yehudasa I'm open to any other approaches if they can fix this as well

@mattbenjamin
Copy link
Contributor

jenkins retest this please

@stale
Copy link

stale bot commented Nov 3, 2018

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue 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 Nov 3, 2018
@stale stale bot removed the stale label Nov 3, 2018
@mattbenjamin
Copy link
Contributor

@mdw-at-linuxbox can you review this?

@stale
Copy link

stale bot commented Jan 2, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue 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 Jan 2, 2019
@stale
Copy link

stale bot commented Apr 22, 2019

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 Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants