Update resource as per new policy #449
Conversation
|
@balamurugana Could you review this? Also related to #448 |
|
This fixes #448 |
| @@ -132,7 +132,7 @@ def _get_in_use_policy(statements, bucket_name, prefix=''): | |||
| resource = s['Resource'] | |||
| actions = set(_get_action(s)) | |||
| if (resource != object_resource and | |||
| resource.startswith(resource_prefix)): | |||
| resource[0].startswith(resource_prefix)): | |||
There was a problem hiding this comment.
- As per Amazon AWS dynamic JSON,
Resourcecan be a string or a list of string. Assuming as a list would break logically. The right fix would be type checking like
resource = s['resource']
if isinstance(resource, basestring):
resource = [resource]- We shouldn't assume
resource[0]instead it could match in any item in the list.
There was a problem hiding this comment.
@balamurugana Thanks. Fixed it.
| @@ -66,7 +81,7 @@ def _new_bucket_statement(policy, bucket_name, prefix=''): | |||
| if policy == Policy.NONE: | |||
| return [] | |||
|
|
|||
| bucket_resource = _AWS_RESOURCE_PREFIX + bucket_name | |||
| bucket_resource = [_AWS_RESOURCE_PREFIX + bucket_name] | |||
There was a problem hiding this comment.
Why do you want bucket_resource as list?
There was a problem hiding this comment.
I thought resource has to be a list. Didn't know, it can either be list or string.
| @@ -129,10 +148,10 @@ def _get_in_use_policy(statements, bucket_name, prefix=''): | |||
| Policy.WRITE_ONLY: False} | |||
|
|
|||
| for s in statements: | |||
| resource = s['Resource'] | |||
| resource = _get_resource(s) | |||
There was a problem hiding this comment.
if s['Resource'] would raise an exception which denotes an wrong JSON. This approach swallows the error.
There was a problem hiding this comment.
Removed default value. Thanks.
| @@ -119,6 +134,10 @@ def _new_statements(policy, bucket_name, prefix=''): | |||
| _new_object_statement(policy, bucket_name, prefix)) | |||
|
|
|||
|
|
|||
| def resource_startswith(prefix, resources): | |||
| return any([resource.startswith(prefix) for resource in resources]) | |||
There was a problem hiding this comment.
is it good idea to use filter() function something like
return filter(lambda r: r.startswith(prefix), resources)I am not sure which one is better.
|
LGTM |
| @@ -119,6 +131,10 @@ def _new_statements(policy, bucket_name, prefix=''): | |||
| _new_object_statement(policy, bucket_name, prefix)) | |||
|
|
|||
|
|
|||
| def resource_startswith(prefix, resources): | |||
| return list(filter(lambda r: r.startswith(prefix), resources)) | |||
There was a problem hiding this comment.
- filter() always returns list by default. wrapping filter() return is unnecessary.
- rename resource_startswith() to _filter_resources() and add a comment what this function does
There was a problem hiding this comment.
In [51]: bool(filter(lambda r: r.startswith('c'), ['aa', 'bb']))
Out[51]: True
In python 3, filter returns filter object and its bool is always true. So we have to convert to list.
Documented _filter_resources. Thanks.
There was a problem hiding this comment.
👍 filter() returns an iterable in py3.
|
Can you make a new release? |
|
@ChillarAnand will do soon. |
minio is returning resource as a list