Merge lp:~gaurav-gangalwar/swift/object_acl into lp:~hudson-openstack/swift/trunk

Proposed by Gaurav Gangalwar
Status: Rejected
Rejected by: John Dickinson
Proposed branch: lp:~gaurav-gangalwar/swift/object_acl
Merge into: lp:~hudson-openstack/swift/trunk
Diff against target: 595 lines (+249/-64)
6 files modified
etc/proxy-server.conf-sample (+3/-0)
swift/obj/server.py (+1/-1)
swift/proxy/server.py (+190/-48)
test/functional/sample.conf (+1/-0)
test/functionalnosetests/swift_testing.py (+2/-0)
test/functionalnosetests/test_object.py (+52/-15)
To merge this branch: bzr merge lp:~gaurav-gangalwar/swift/object_acl
Reviewer Review Type Date Requested Status
John Dickinson Disapprove
Review via email: mp+70003@code.launchpad.net

Description of the change

Having container level acl will enforce those acl on all the objects in that container.
I think we should have acl support at object level, to provide more fine grain acl scheme.
Here is what i am proposing.

Permission When granted on a Container When granted on an object

READ Allows to list the objects in the Allows to read the object
         container and read container data and its metadata
  metadata.

WRITE Allows to list create, overwrite, Allows to write the object
  delete any object in the container metadata.
  and write the container metadata.

To post a comment you must log in.
Revision history for this message
Gaurav Gangalwar (gaurav-gangalwar) wrote :

When granted on a Container:->
READ Allows to list the objects in the
         container and read container
  metadata.

WRITE Allows to list, create, overwrite,
  delete any object in the container
  and write the container metadata.

When granted on an object:->
READ Allows to read the object data and its metadata

WRITE Allows to write the object metadata.

Revision history for this message
John Dickinson (notmyname) wrote :

I haven't reviewed all of the code yet, but I notice that there aren't any tests provided. This code will need unit tests before it can be accepted.

review: Needs Fixing
Revision history for this message
Gaurav Gangalwar (gaurav-gangalwar) wrote :

Yes i am working on unit test cases, will submit it soon.

Regards,
Gaurav

On Mon, Aug 1, 2011 at 7:13 PM, John Dickinson <<email address hidden>
> wrote:

> Review: Needs Fixing
> I haven't reviewed all of the code yet, but I notice that there aren't any
> tests provided. This code will need unit tests before it can be accepted.
> --
> https://code.launchpad.net/~gaurav-gangalwar/swift/object_acl/+merge/70003<https://code.launchpad.net/%7Egaurav-gangalwar/swift/object_acl/+merge/70003>
> You are the owner of lp:~gaurav-gangalwar/swift/object_acl.
>

336. By <email address hidden>

Object acl support fixes.

Revision history for this message
Gaurav Gangalwar (gaurav-gangalwar) wrote :

I made some fixes and updated functional tests.
There are some failures in existing unit tests, still figuring them out, will update soon.

Revision history for this message
John Dickinson (notmyname) wrote :

In large clusters, it seems that checking the object info could blow through memcache pretty quickly. Can you add the ability to turn this feature on or off with a config variable?

Revision history for this message
Gaurav Gangalwar (gaurav-gangalwar) wrote :

Ok, i am thinking about these options,

==>Give object-acl as option.
==>Give memcache for object-acl as option.
==>Reduce the expiry time for objects acl.

Which one is more preferrable?

Also i have one doubt, do we recommend mutiple proxy servers in large
clusters for load balancing?
If that is the case, do we recommend to use a common memcahe server for all
the proxies?

Regards,
Gaurav

On Fri, Aug 5, 2011 at 6:57 PM, John Dickinson <email address hidden> wrote:

> In large clusters, it seems that checking the object info could blow
> through memcache pretty quickly. Can you add the ability to turn this
> feature on or off with a config variable?
> --
> https://code.launchpad.net/~gaurav-gangalwar/swift/object_acl/+merge/70003<https://code.launchpad.net/%7Egaurav-gangalwar/swift/object_acl/+merge/70003>
> You are the owner of lp:~gaurav-gangalwar/swift/object_acl.
>

Revision history for this message
John Dickinson (notmyname) wrote :

Yes, large clusters have many proxy servers and all share the same memcached pool.

In large clusters (with billions of objects), checking for the object info for every object as its requested fills up the cache. What should probably happen is that large deployments should be able to turn this off so that the object info is never even looked at (and therefore never cached).

337. By <email address hidden>

Make object_acl feature configurable.

Revision history for this message
Gaurav Gangalwar (gaurav-gangalwar) wrote :

Change to make object_acl configurable submitted.

Revision history for this message
gholt (gholt) wrote :

The object acl seems to override the container acl; so I could grant Susan write access to my container and then she could create objects that I could not delete or overwrite, even though I have write access to the container. Also, I could seemingly take away Susan's access to a container, but she might be able to access objects in the container still.

This feature would need to be off by default. If a large cluster operator were to upgrade to this code and not realize they had to add "object_acl = false" to their configuration, they'd have some difficulties. Not only can this have a dramatic impact on memcache usage, but it also will up the number of backend requests.

Why is the "object_size" memcached?

In the object GETorHEAD the acls are retrieved even if 'swift.authorize' not in req.environ. This isn't often used, but was added to allow short-circuiting such extra queries to lower memcache and backend requests when they aren't needed.

The logic for setting req.acl is different than before. It used to be that write access could be granted to a container without granting read access (blind drop box type-thing). In the object GETorHEAD, now write access also grants read access.

With segmented objects, why the change to giving the end user all headers from the backend server?

In the container GETorHEAD, the change to calling container_info before performing the backend container request causes extra requests that aren't needed. The way it was coded before, it just always did the container backend request and returned the response depending on the acl values in the backend response.

The code now requires write access to a container to delete or post to that container, whereas before it just required the swift.authorize method to allow it. This change means a user that has full access to an account but isn't explicitly in the write list for a container can't do anything about it.

Whereas there are some new functional tests, there aren't any new unit tests and many existing unit tests are failing, but I think you were already aware of that part and working on it.

Revision history for this message
Gaurav Gangalwar (gaurav-gangalwar) wrote :
Download full text (3.5 KiB)

On Fri, Aug 12, 2011 at 7:40 PM, gholt <email address hidden> wrote:

> The object acl seems to override the container acl; so I could grant Susan
> write access to my container and then she could create objects that I could
> not delete or overwrite, even though I have write access to the container.
> Also, I could seemingly take away Susan's access to a container, but she
> might be able to access objects in the container still.
>

Write object-acl will only control POST and GETorHEAD on the object, You can
DELETE or PUT objects inside the container if you have write access or owner
of the account.

>
> This feature would need to be off by default. If a large cluster operator
> were to upgrade to this code and not realize they had to add "object_acl =
> false" to their configuration, they'd have some difficulties. Not only can
> this have a dramatic impact on memcache usage, but it also will up the
> number of backend requests.
>

Ok, in large clusters if its enabled then it might lead to excess memcache
usage, but it will not affect any functionality if object-acl is not set.

>
> Why is the "object_size" memcached?
>

object_size is not used for now, will remove it.

>
> In the object GETorHEAD the acls are retrieved even if 'swift.authorize'
> not in req.environ. This isn't often used, but was added to allow
> short-circuiting such extra queries to lower memcache and backend requests
> when they aren't needed.
>

In which case swift.authorize will not be in the req.environ? I thought auth
subsystem will set it for each req.

>
> The logic for setting req.acl is different than before. It used to be that
> write access could be granted to a container without granting read access
> (blind drop box type-thing). In the object GETorHEAD, now write access also
> grants read access.
>

Yes, allowing write and not allowing read is not supposed to be the expected
behaviour, so write acl will have read/write access to the object.

>
> With segmented objects, why the change to giving the end user all headers
> from the backend server?
>

This filtering is done by the backend servers, so it was redundant here,
thats why.

>
> In the container GETorHEAD, the change to calling container_info before
> performing the backend container request causes extra requests that aren't
> needed. The way it was coded before, it just always did the container
> backend request and returned the response depending on the acl values in the
> backend response.
>

In case of unauthorized it will be only one request. I thought of keeping a
single interface for memcache, could we remove memcache updation code from
Container ops?

>
> The code now requires write access to a container to delete or post to that
> container, whereas before it just required the swift.authorize method to
> allow it. This change means a user that has full access to an account but
> isn't explicitly in the write list for a container can't do anything about
> it.
>

No, if you are the owner of account, authorize will not check for any access
list, it will give you all access to the containers and objects.

>
> Whereas there are some new functional tests, there aren't any new unit
> tests and many ex...

Read more...

Revision history for this message
gholt (gholt) wrote :
Download full text (4.2 KiB)

> Write object-acl will only control POST and GETorHEAD on the
> object, You can DELETE or PUT objects inside the container if you
> have write access or owner of the account.

Okay, that's a little weird that POST acts differently; in fact,
POST-as-COPY would act like PUT, so only POST-not-as-COPY would have
the new effect. We've done our best to this point to make
POST-as-COPY and POST-not-as-COPY act the same way. If I have write
access to a container, shouldn't I be able to POST to any object in
that container?

Also, is it true that if my access to a container is taken away I'd
still be able to access any objects I explicitly set ACLs for myself
on within that container?

> Ok, in large clusters if its enabled then it might lead to excess
> memcache usage, but it will not affect any functionality if
> object-acl is not set.

It should not affect functionality, true (it does currently, but
that's discussed elsewhere). But extra memcache queries and extra
object HEADs for no reason is bad when we're already doing thousands
of requests per second.

Along the same vein, with this code the first GET or HEAD of an
object will do a cache_get + HEAD + cache_set + GET|HEAD and a
second request would do a cache_get + GET|HEAD. That should be able
to be reduced to a GET|HEAD + cache_set and the ACL checked on that
response and thrown away if access is denied.

> In which case swift.authorize will not be in the req.environ? I
> thought auth subsystem will set it for each req.

The auth subsystem does not need to set swift.authorize to authorize
a request, it only needs to set swift.authorize if it wants the
proxy code to execute that callback to check access. If
swift.authorize does not exist, access is granted.

As an extreme example, you can actually run with no auth middleware
and all requests will be granted. As a less extreme example, auth
middleware may clear swift.authorize when it has enough information
up front to know the request should be granted.

Honestly, I'm not too worried about this since no one uses it, but
it might become important to someone someday and there didn't seem
to be a good reason to drop the behavior.

> Yes, allowing write and not allowing read is not supposed to be
> the expected behaviour, so write acl will have read/write access
> to the object.

I'm not sure what you mean by "not supposed to be the expected
behaviour". It was the expected behavior before.

> This filtering is done by the backend servers, so it was redundant
> here, thats why.

It is redundant, but is a safety mechanism to ensure we don't
accidentally return headers we didn't mean to. It wastes very little
CPU time, so I'm not sure why we'd want to change what was already
there.

>> In the container GETorHEAD, the change to calling container_info
>> before performing the backend container request causes extra
>> requests that aren't needed.

> In case of unauthorized it will be only one request. I thought of
> keeping a single interface for memcache, could we remove memcache
> updation code from Container ops?

What I meant was that the previous code for container GETorHEAD did
the container request, sent the ACL part to memcache, checked the
ACL a...

Read more...

Revision history for this message
Gaurav Gangalwar (gaurav-gangalwar) wrote :
Download full text (6.3 KiB)

On Tue, Aug 16, 2011 at 7:49 PM, gholt <email address hidden> wrote:

> > Write object-acl will only control POST and GETorHEAD on the
> > object, You can DELETE or PUT objects inside the container if you
> > have write access or owner of the account.
>
> Okay, that's a little weird that POST acts differently; in fact,
> POST-as-COPY would act like PUT, so only POST-not-as-COPY would have
> the new effect. We've done our best to this point to make
> POST-as-COPY and POST-not-as-COPY act the same way. If I have write
> access to a container, shouldn't I be able to POST to any object in
> that container?
>
Write access to container will allow POST-as-COPY but not POST-not-as-COPY.
The reason for this is,
In general PUT operation on objects affect the container metadata, so it
should have write access to the container, but POST operations on object
doesn't affect the container, so it should require only write access to the
object since it will only affect object metadata. In POST-as-COPY its
equivalent to PUT.
We can allow PUT which will only overwrite the existing the object (not
creating new object inside container) just by checking write permission on
object, no need to check container write acl for this, so this way
POST-as-COPY and POST-not-as-COPY will work the same way.

>
> Also, is it true that if my access to a container is taken away I'd
> still be able to access any objects I explicitly set ACLs for myself
> on within that container?
>

Yes you can do GETorHEAD/POST but won't be able to PUT/DELETE on those
objects.

>
> > Ok, in large clusters if its enabled then it might lead to excess
> > memcache usage, but it will not affect any functionality if
> > object-acl is not set.
>
> It should not affect functionality, true (it does currently, but
> that's discussed elsewhere). But extra memcache queries and extra
> object HEADs for no reason is bad when we're already doing thousands
> of requests per second.
>

If object_acl is disabled, object_info will return the acl for container, so
it will work the same way as it was earlier.

>
> Along the same vein, with this code the first GET or HEAD of an
> object will do a cache_get + HEAD + cache_set + GET|HEAD and a
> second request would do a cache_get + GET|HEAD. That should be able
> to be reduced to a GET|HEAD + cache_set and the ACL checked on that
> response and thrown away if access is denied.
>

Ok i will optimize this code to do minimal requests.

>
> > In which case swift.authorize will not be in the req.environ? I
> > thought auth subsystem will set it for each req.
>
> The auth subsystem does not need to set swift.authorize to authorize
> a request, it only needs to set swift.authorize if it wants the
> proxy code to execute that callback to check access. If
> swift.authorize does not exist, access is granted.
>
> As an extreme example, you can actually run with no auth middleware
> and all requests will be granted. As a less extreme example, auth
> middleware may clear swift.authorize when it has enough information
> up front to know the request should be granted.
>
> Honestly, I'm not too worried about this since no one uses it, but
> it might become important to someone some...

Read more...

Revision history for this message
gholt (gholt) wrote :

The write and no read thing was from someone wanting to use a container as an "upload here" type thing. Where others could drop files that would then be approved and moved to official locations later. In that use case, you don't want the uploaders to be able to see the unapproved files.

The more I think about it, such decisions like "does write give read?" "does container access override object access?" etc. should be made by the auth middleware, not the proxy. The proxy should just provide the information. In other words, the proxy should send up the read and write acls for objects and containers and let the auth middleware figure out what to do.

Revision history for this message
John Dickinson (notmyname) wrote :

(Just today) swift has moved to using github for code hosting and gerrit for code reviews. Instructions on setting it up are at http://wiki.openstack.org/GerritWorkflow. Please resubmit your patch there.

I'm disapproving this merge only because it needs to be made through gerrit now.

review: Disapprove

Unmerged revisions

337. By <email address hidden>

Make object_acl feature configurable.

336. By <email address hidden>

Object acl support fixes.

335. By <email address hidden>

Object acl support1.

334. By <email address hidden>

Object acl support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'etc/proxy-server.conf-sample'
2--- etc/proxy-server.conf-sample 2011-06-16 22:05:03 +0000
3+++ etc/proxy-server.conf-sample 2011-08-07 06:38:25 +0000
4@@ -48,6 +48,9 @@
5 # If set to 'true' authorized accounts that do not yet exist within the Swift
6 # cluster will be automatically created.
7 # account_autocreate = false
8+# If set to false this will disable the object_acl feature, in large cluster
9+# enviornment with limited memcache, it is recommended to disable object_acl
10+# object_acl = true
11
12 [filter:tempauth]
13 use = egg:swift#tempauth
14
15=== modified file 'swift/obj/server.py'
16--- swift/obj/server.py 2011-08-01 16:08:55 +0000
17+++ swift/obj/server.py 2011-08-07 06:38:25 +0000
18@@ -363,7 +363,7 @@
19 self.slow = int(conf.get('slow', 0))
20 self.bytes_per_sync = int(conf.get('mb_per_sync', 512)) * 1024 * 1024
21 default_allowed_headers = 'content-encoding, x-object-manifest, ' \
22- 'content-disposition'
23+ 'content-disposition, x-object-read, x-object-write'
24 self.allowed_headers = set(i.strip().lower() for i in \
25 conf.get('allowed_headers', \
26 default_allowed_headers).split(',') if i.strip() and \
27
28=== modified file 'swift/proxy/server.py'
29--- swift/proxy/server.py 2011-07-14 18:43:08 +0000
30+++ swift/proxy/server.py 2011-08-07 06:38:25 +0000
31@@ -105,6 +105,8 @@
32 def get_container_memcache_key(account, container):
33 return 'container/%s/%s' % (account, container)
34
35+def get_object_memcache_key(account, container, obj):
36+ return 'object/%s/%s/%s' % (account, container, obj)
37
38 class SegmentedIterable(object):
39 """
40@@ -493,6 +495,97 @@
41 return partition, nodes, read_acl, write_acl, sync_key
42 return None, None, None, None, None
43
44+ def object_info(self, account, container, obj):
45+ """
46+ Get object information and thusly verify object existance.
47+ This will also make a call to container_info to verify that the
48+ container exists.
49+
50+ :param account: account name for the container
51+ :param container: container name for the object
52+ :param object: object name to look up.
53+ :returns: tuple of (object partition, object nodes, object
54+ read acl, object write acl) or (None, None, None, None) if
55+ the object does not exist
56+ """
57+ partition, nodes = self.app.object_ring.get_nodes(
58+ account, container, obj)
59+
60+ _junk, cnodes, read_acl, write_acl, _junk = \
61+ self.container_info(account, container)
62+ if not cnodes:
63+ return None, None, None, None
64+
65+ if not self.app.object_acl:
66+ return partition, nodes, read_acl, write_acl
67+
68+ path = '/%s/%s/%s' % (account, container, obj)
69+ if self.app.memcache:
70+ cache_key = get_object_memcache_key(account, container, obj)
71+ cache_value = self.app.memcache.get(cache_key)
72+
73+ if isinstance(cache_value, dict):
74+ status = cache_value['status']
75+ read_acl = cache_value['read_acl']
76+ write_acl = cache_value['write_acl']
77+ if status == 200:
78+ return partition, nodes, read_acl, write_acl
79+ elif status == 404:
80+ return None, None, None, None
81+
82+ result_code = 0
83+ read_acl = None
84+ write_acl = None
85+ obj_size = None
86+ attempts_left = self.app.object_ring.replica_count
87+ headers = {'x-trans-id': self.trans_id}
88+ for node in nodes:
89+ try:
90+ with ConnectionTimeout(self.app.conn_timeout):
91+ conn = http_connect(node['ip'], node['port'],
92+ node['device'], partition, 'HEAD', path, headers)
93+ with Timeout(self.app.node_timeout):
94+ resp = conn.getresponse()
95+ body = resp.read()
96+ if 200 <= resp.status <= 299:
97+ result_code = 200
98+ read_acl = resp.getheader('x-object-read')
99+ write_acl = resp.getheader('x-object-write')
100+ obj_size = \
101+ resp.getheader('Content-Length')
102+ break
103+ elif resp.status == 404:
104+ if result_code == 0:
105+ result_code = 404
106+ elif result_code != 404:
107+ result_code = -1
108+ elif resp.status == 507:
109+ self.error_limit(node)
110+ continue
111+ else:
112+ result_code = -1
113+ attempts_left -= 1
114+ if attempts_left <= 0:
115+ break
116+ except (Exception, TimeoutError):
117+ self.exception_occurred(node, _('Object'),
118+ _('Trying to get object info for %s') % path)
119+ if self.app.memcache and result_code in (200, 404):
120+ if result_code == 200:
121+ cache_timeout = self.app.recheck_container_existence
122+ else:
123+ cache_timeout = self.app.recheck_container_existence * 0.1
124+ self.app.memcache.set(cache_key,
125+ {'status': result_code,
126+ 'read_acl': read_acl,
127+ 'write_acl': write_acl,
128+ 'object_size': obj_size},
129+ timeout=cache_timeout)
130+ if result_code == 200:
131+ return partition, nodes, read_acl, write_acl
132+ return None, None, None, None
133+
134+
135 def iter_nodes(self, partition, nodes, ring):
136 """
137 Node iterator that will first iterate over the normal nodes for a
138@@ -584,6 +677,19 @@
139 resp.status = '503 Internal Server Error'
140 return resp
141
142+ def clean_acls(self, req):
143+ if 'swift.clean_acl' in req.environ:
144+ for header in ('x-container-read', 'x-container-write',
145+ 'x-object-read', 'x-object-write'):
146+ if header in req.headers:
147+ try:
148+ req.headers[header] = \
149+ req.environ['swift.clean_acl'](header,
150+ req.headers[header])
151+ except ValueError, err:
152+ return HTTPBadRequest(request=req, body=str(err))
153+ return None
154+
155 @public
156 def GET(self, req):
157 """Handler for HTTP GET requests."""
158@@ -728,18 +834,26 @@
159
160 def GETorHEAD(self, req):
161 """Handle HTTP GET or HEAD requests."""
162+ partition, nodes, read_acl, write_acl = \
163+ self.object_info(self.account_name, self.container_name, \
164+ self.object_name)
165+ if not nodes:
166+ return HTTPNotFound(request=req)
167 if 'swift.authorize' in req.environ:
168- req.acl = \
169- self.container_info(self.account_name, self.container_name)[2]
170+ if read_acl and write_acl:
171+ req.acl = read_acl + ',' + write_acl
172+ else:
173+ req.acl = read_acl or write_acl
174+
175 aresp = req.environ['swift.authorize'](req)
176 if aresp:
177 return aresp
178- partition, nodes = self.app.object_ring.get_nodes(
179- self.account_name, self.container_name, self.object_name)
180+
181 shuffle(nodes)
182 resp = self.GETorHEAD_base(req, _('Object'), partition,
183 self.iter_nodes(partition, nodes, self.app.object_ring),
184 req.path_info, self.app.object_ring.replica_count)
185+
186 # If we get a 416 Requested Range Not Satisfiable we have to check if
187 # we were actually requesting a manifest object and then redo the range
188 # request on the whole object.
189@@ -754,11 +868,24 @@
190 resp = resp2
191 req.range = req_range
192
193+ if not req.environ.get('swift_owner', False):
194+ for key in ('x-object-read', 'x-object-write'):
195+ if key in resp.headers:
196+ del resp.headers[key]
197+
198 if 'x-object-manifest' in resp.headers:
199 lcontainer, lprefix = \
200 resp.headers['x-object-manifest'].split('/', 1)
201- lpartition, lnodes = self.app.container_ring.get_nodes(
202- self.account_name, lcontainer)
203+ lpartition, lnodes, lread_acl, lwrite_acl, _junk = \
204+ self.container_info(self.account_name, lcontainer)
205+ if lread_acl and lwrite_acl:
206+ req.acl = lread_acl + ',' + lwrite_acl
207+ else:
208+ req.acl = lread_acl or lwrite_acl
209+ if 'swift.authorize' in req.environ:
210+ aresp = req.environ['swift.authorize'](req)
211+ if aresp:
212+ return aresp
213 marker = ''
214 listing = []
215 while True:
216@@ -774,11 +901,6 @@
217 lresp.headers['X-Object-Manifest'] = \
218 resp.headers['x-object-manifest']
219 return lresp
220- if 'swift.authorize' in req.environ:
221- req.acl = lresp.headers.get('x-container-read')
222- aresp = req.environ['swift.authorize'](req)
223- if aresp:
224- return aresp
225 sublisting = json.loads(lresp.body)
226 if not sublisting:
227 break
228@@ -805,13 +927,6 @@
229 raise Exception(_('Object manifest GET could not '
230 'continue listing: %s %s') %
231 (req.path, lreq.path))
232- if 'swift.authorize' in req.environ:
233- req.acl = lresp.headers.get('x-container-read')
234- aresp = req.environ['swift.authorize'](req)
235- if aresp:
236- raise Exception(_('Object manifest GET could '
237- 'not continue listing: %s %s') %
238- (req.path, aresp))
239 sublisting = json.loads(lresp.body)
240 if not sublisting:
241 break
242@@ -823,8 +938,7 @@
243 'X-Object-Manifest': resp.headers['x-object-manifest'],
244 'Content-Type': resp.content_type}
245 for key, value in resp.headers.iteritems():
246- if key.lower().startswith('x-object-meta-'):
247- headers[key] = value
248+ headers[key] = value
249 resp = Response(headers=headers, request=req,
250 conditional_response=True)
251 if req.method == 'HEAD':
252@@ -865,8 +979,7 @@
253 'Content-Length': content_length,
254 'ETag': etag}
255 for key, value in resp.headers.iteritems():
256- if key.lower().startswith('x-object-meta-'):
257- headers[key] = value
258+ headers[key] = value
259 resp = Response(headers=headers, request=req,
260 conditional_response=True)
261 resp.app_iter = SegmentedIterable(self, lcontainer, listing,
262@@ -874,7 +987,7 @@
263 resp.content_length = content_length
264 resp.last_modified = last_modified
265 resp.headers['accept-ranges'] = 'bytes'
266-
267+
268 return resp
269
270 @public
271@@ -893,6 +1006,10 @@
272 @delay_denial
273 def POST(self, req):
274 """HTTP POST request handler."""
275+ error_response = \
276+ self.clean_acls(req) or check_metadata(req, 'object')
277+ if error_response:
278+ return error_response
279 if self.app.object_post_as_copy:
280 req.method = 'PUT'
281 req.path_info = '/%s/%s/%s' % (self.account_name,
282@@ -912,17 +1029,21 @@
283 error_response = check_metadata(req, 'object')
284 if error_response:
285 return error_response
286- container_partition, containers, _junk, req.acl, _junk = \
287+ container_partition, containers, _junk, _junk, _junk = \
288 self.container_info(self.account_name, self.container_name,
289 account_autocreate=self.app.account_autocreate)
290+ partition, nodes, _junk, req.acl = \
291+ self.object_info(self.account_name, self.container_name,\
292+ self.object_name)
293+ if not nodes:
294+ return HTTPNotFound(request=req)
295 if 'swift.authorize' in req.environ:
296 aresp = req.environ['swift.authorize'](req)
297 if aresp:
298 return aresp
299 if not containers:
300 return HTTPNotFound(request=req)
301- partition, nodes = self.app.object_ring.get_nodes(
302- self.account_name, self.container_name, self.object_name)
303+
304 req.headers['X-Timestamp'] = normalize_timestamp(time.time())
305 headers = []
306 for container in containers:
307@@ -931,6 +1052,11 @@
308 nheaders['X-Container-Partition'] = container_partition
309 nheaders['X-Container-Device'] = container['device']
310 headers.append(nheaders)
311+ if self.app.memcache:
312+ cache_key = get_object_memcache_key(self.account_name,
313+ self.container_name,
314+ self.object_name)
315+ self.app.memcache.delete(cache_key)
316 return self.make_requests(req, self.app.object_ring,
317 partition, 'POST', req.path_info, headers)
318
319@@ -970,6 +1096,10 @@
320 @delay_denial
321 def PUT(self, req):
322 """HTTP PUT request handler."""
323+ error_response = \
324+ self.clean_acls(req) or check_metadata(req, 'object')
325+ if error_response:
326+ return error_response
327 (container_partition, containers, _junk, req.acl,
328 req.environ['swift_sync_key']) = \
329 self.container_info(self.account_name, self.container_name,
330@@ -982,6 +1112,7 @@
331 return HTTPNotFound(request=req)
332 partition, nodes = self.app.object_ring.get_nodes(
333 self.account_name, self.container_name, self.object_name)
334+
335 # Used by container sync feature
336 if 'x-timestamp' in req.headers:
337 try:
338@@ -1176,6 +1307,11 @@
339 # reset the bytes, since the user didn't actually send anything
340 req.bytes_transferred = 0
341 resp.last_modified = float(req.headers['X-Timestamp'])
342+ if self.app.memcache:
343+ cache_key = get_object_memcache_key(self.account_name,
344+ self.container_name,
345+ self.object_name)
346+ self.app.memcache.delete(cache_key)
347 return resp
348
349 @public
350@@ -1211,6 +1347,11 @@
351 nheaders['X-Container-Partition'] = container_partition
352 nheaders['X-Container-Device'] = container['device']
353 headers.append(nheaders)
354+ if self.app.memcache:
355+ cache_key = get_object_memcache_key(self.account_name,
356+ self.container_name,
357+ self.object_name)
358+ self.app.memcache.delete(cache_key)
359 return self.make_requests(req, self.app.object_ring,
360 partition, 'DELETE', req.path_info, headers)
361
362@@ -1257,24 +1398,22 @@
363 self.account_name = unquote(account_name)
364 self.container_name = unquote(container_name)
365
366- def clean_acls(self, req):
367- if 'swift.clean_acl' in req.environ:
368- for header in ('x-container-read', 'x-container-write'):
369- if header in req.headers:
370- try:
371- req.headers[header] = \
372- req.environ['swift.clean_acl'](header,
373- req.headers[header])
374- except ValueError, err:
375- return HTTPBadRequest(request=req, body=str(err))
376- return None
377-
378 def GETorHEAD(self, req):
379 """Handler for HTTP GET/HEAD requests."""
380- if not self.account_info(self.account_name)[1]:
381+ part, nodes, read_acl, write_acl, _junk= \
382+ self.container_info(self.account_name, self.container_name)
383+ if 'swift.authorize' in req.environ:
384+ if read_acl and write_acl:
385+ req.acl = read_acl + ',' + write_acl
386+ else:
387+ req.acl = read_acl or write_acl
388+
389+ aresp = req.environ['swift.authorize'](req)
390+ if aresp:
391+ return aresp
392+ if not nodes:
393 return HTTPNotFound(request=req)
394- part, nodes = self.app.container_ring.get_nodes(
395- self.account_name, self.container_name)
396+
397 shuffle(nodes)
398 resp = self.GETorHEAD_base(req, _('Container'), part, nodes,
399 req.path_info, self.app.container_ring.replica_count)
400@@ -1291,11 +1430,6 @@
401 'container_size': resp.headers.get('x-container-object-count')},
402 timeout=self.app.recheck_container_existence)
403
404- if 'swift.authorize' in req.environ:
405- req.acl = resp.headers.get('x-container-read')
406- aresp = req.environ['swift.authorize'](req)
407- if aresp:
408- return aresp
409 if not req.environ.get('swift_owner', False):
410 for key in ('x-container-read', 'x-container-write',
411 'x-container-sync-key', 'x-container-sync-to'):
412@@ -1352,6 +1486,7 @@
413 container_partition, 'PUT', req.path_info, headers)
414
415 @public
416+ @delay_denial
417 def POST(self, req):
418 """HTTP POST request handler."""
419 error_response = \
420@@ -1362,8 +1497,14 @@
421 autocreate=self.app.account_autocreate)
422 if not accounts:
423 return HTTPNotFound(request=req)
424- container_partition, containers = self.app.container_ring.get_nodes(
425- self.account_name, self.container_name)
426+ container_partition, containers, _junk, req.acl, _junk =\
427+ self.container_info(self.account_name, self.container_name)
428+ if not containers:
429+ return HTTPNotFound(request=req)
430+ if 'swift.authorize' in req.environ:
431+ aresp = req.environ['swift.authorize'](req)
432+ if aresp:
433+ return aresp
434 headers = {'X-Timestamp': normalize_timestamp(time.time()),
435 'x-trans-id': self.trans_id}
436 headers.update(value for value in req.headers.iteritems()
437@@ -1534,6 +1675,7 @@
438 self.put_queue_depth = int(conf.get('put_queue_depth', 10))
439 self.object_chunk_size = int(conf.get('object_chunk_size', 65536))
440 self.client_chunk_size = int(conf.get('client_chunk_size', 65536))
441+ self.object_acl = conf.get('object_acl', 'true').lower() in TRUE_VALUES
442 self.log_headers = conf.get('log_headers', 'no').lower() in TRUE_VALUES
443 self.error_suppression_interval = \
444 int(conf.get('error_suppression_interval', 60))
445
446=== modified file 'test/functional/sample.conf'
447--- test/functional/sample.conf 2011-03-14 02:56:37 +0000
448+++ test/functional/sample.conf 2011-08-07 06:38:25 +0000
449@@ -4,6 +4,7 @@
450 auth_port = 8080
451 auth_ssl = no
452 auth_prefix = /auth/
453+object_acl = true
454
455 # Primary functional test account (needs admin access to the account)
456 account = test
457
458=== modified file 'test/functionalnosetests/swift_testing.py'
459--- test/functionalnosetests/swift_testing.py 2011-02-24 22:21:14 +0000
460+++ test/functionalnosetests/swift_testing.py 2011-08-07 06:38:25 +0000
461@@ -17,6 +17,8 @@
462 swift_test_auth = os.environ.get('SWIFT_TEST_AUTH')
463 swift_test_user = [os.environ.get('SWIFT_TEST_USER'), None, None]
464 swift_test_key = [os.environ.get('SWIFT_TEST_KEY'), None, None]
465+object_acl = conf.get('object_acl', 'true').lower() in ('yes', 'true', 'on',
466+'1')
467
468 if conf:
469 swift_test_auth = 'http'
470
471=== modified file 'test/functionalnosetests/test_object.py'
472--- test/functionalnosetests/test_object.py 2011-07-22 19:58:22 +0000
473+++ test/functionalnosetests/test_object.py 2011-08-07 06:38:25 +0000
474@@ -7,7 +7,8 @@
475 from swift.common.constraints import MAX_META_COUNT, MAX_META_NAME_LENGTH, \
476 MAX_META_OVERALL_SIZE, MAX_META_VALUE_LENGTH
477
478-from swift_testing import check_response, retry, skip, skip3, swift_test_user
479+from swift_testing import check_response, retry, skip, skip3, swift_test_user, \
480+object_acl
481
482
483 class TestObject(unittest.TestCase):
484@@ -158,24 +159,39 @@
485 self.assert_(str(err).startswith('No result after '))
486
487 def post(url, token, parsed, conn):
488- conn.request('POST', parsed.path + '/' + self.container, '',
489+ if object_acl:
490+ conn.request('POST', parsed.path + '/' + self.container + '/' + self.obj, '',
491+ {'X-Auth-Token': token,
492+ 'X-Object-Read': '.r:*'})
493+ else:
494+ conn.request('POST', parsed.path + '/' + self.container, '',
495 {'X-Auth-Token': token,
496 'X-Container-Read': '.r:*'})
497 return check_response(conn)
498 resp = retry(post)
499 resp.read()
500- self.assertEquals(resp.status, 204)
501+ if object_acl:
502+ self.assertEquals(resp.status, 202)
503+ else:
504+ self.assertEquals(resp.status, 204)
505 resp = retry(get)
506 resp.read()
507 self.assertEquals(resp.status, 200)
508
509 def post(url, token, parsed, conn):
510- conn.request('POST', parsed.path + '/' + self.container, '',
511- {'X-Auth-Token': token, 'X-Container-Read': ''})
512+ if object_acl:
513+ conn.request('POST', parsed.path + '/' + self.container + '/' + self.obj, '',
514+ {'X-Auth-Token': token, 'X-Object-Read': ''})
515+ else:
516+ conn.request('POST', parsed.path + '/' + self.container, '',
517+ {'X-Auth-Token': token, 'X-Container-Read': ''})
518 return check_response(conn)
519 resp = retry(post)
520 resp.read()
521- self.assertEquals(resp.status, 204)
522+ if object_acl:
523+ self.assertEquals(resp.status, 202)
524+ else:
525+ self.assertEquals(resp.status, 204)
526 try:
527 resp = retry(get)
528 raise Exception('Should not have been able to GET')
529@@ -227,7 +243,9 @@
530 # verify third account can write "obj1" to shared container
531 def put(url, token, parsed, conn):
532 conn.request('PUT', '%s/%s/%s' % (parsed.path, shared_container,
533- 'obj1'), 'test', {'X-Auth-Token': token})
534+ 'obj1'), 'test', {'X-Auth-Token': token,
535+ 'X-Object-Read': swift_test_user[2],
536+ 'X-Object-Write': swift_test_user[2]})
537 return check_response(conn)
538 resp = retry(put, use_account=3)
539 resp.read()
540@@ -401,13 +419,21 @@
541
542 # Grant access to the third account
543 def post(url, token, parsed, conn):
544- conn.request('POST', '%s/%s' % (parsed.path, self.container),
545- '', {'X-Auth-Token': token, 'X-Container-Read':
546- swift_test_user[2]})
547+ if object_acl:
548+ conn.request('POST', '%s/%s/manifest' % (parsed.path, self.container),
549+ '', {'X-Auth-Token': token, 'X-Object-Read':
550+ swift_test_user[2]})
551+ else:
552+ conn.request('POST', '%s/%s' % (parsed.path, self.container),
553+ '', {'X-Auth-Token': token, 'X-Container-Read':
554+ swift_test_user[2]})
555 return check_response(conn)
556 resp = retry(post)
557 resp.read()
558- self.assertEquals(resp.status, 204)
559+ if object_acl:
560+ self.assertEquals(resp.status, 202)
561+ else:
562+ self.assertEquals(resp.status, 204)
563
564 # The third account should be able to get the manifest now
565 def get(url, token, parsed, conn):
566@@ -475,14 +501,25 @@
567 self.assertEquals(resp.status, 403)
568
569 # Grant access to the third account
570+ # If its be post as copy for object, then it will copy all the
571+ # segments in one single object, and also remove the 'manifest'
572+ # header, so it will no more be a manifest object.
573 def post(url, token, parsed, conn):
574- conn.request('POST', '%s/%s' % (parsed.path, acontainer),
575- '', {'X-Auth-Token': token, 'X-Container-Read':
576- swift_test_user[2]})
577+ if object_acl:
578+ conn.request('POST', '%s/%s/manifest' % (parsed.path, self.container),
579+ '', {'X-Auth-Token': token, 'X-Object-Read':
580+ swift_test_user[2]})
581+ else:
582+ conn.request('POST', '%s/%s' % (parsed.path, acontainer),
583+ '', {'X-Auth-Token': token, 'X-Container-Read':
584+ swift_test_user[2]})
585 return check_response(conn)
586 resp = retry(post)
587 resp.read()
588- self.assertEquals(resp.status, 204)
589+ if object_acl:
590+ self.assertEquals(resp.status, 202)
591+ else:
592+ self.assertEquals(resp.status, 204)
593
594 # The third account should be able to get the manifest now
595 def get(url, token, parsed, conn):