Merge lp:~gaurav-gangalwar/swift/object_acl into lp:~hudson-openstack/swift/trunk
- object_acl
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
John Dickinson | Disapprove | ||
Review via email: mp+70003@code.launchpad.net |
Commit message
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.
Gaurav Gangalwar (gaurav-gangalwar) wrote : | # |
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.
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:/
> You are the owner of lp:~gaurav-gangalwar/swift/object_acl.
>
- 336. By <email address hidden>
-
Object acl support fixes.
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.
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?
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:/
> You are the owner of lp:~gaurav-gangalwar/swift/object_acl.
>
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.
Gaurav Gangalwar (gaurav-gangalwar) wrote : | # |
Change to make object_acl configurable submitted.
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.
Gaurav Gangalwar (gaurav-gangalwar) wrote : | # |
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...
gholt (gholt) 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?
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...
Gaurav Gangalwar (gaurav-gangalwar) wrote : | # |
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...
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.
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://
I'm disapproving this merge only because it needs to be made through gerrit now.
Preview Diff
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): |
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.