Merge pull request #2454 from mikegrima/s3lifecycle

Made fixes to the S3 Lifecycle mocks to be more consistent with the API

moto/s3/models.py

@@ -329,7 +329,8 @@ class FakeGrant(BaseModel):
 
 class FakeAcl(BaseModel):
 
-    def __init__(self, grants=[]):
+    def __init__(self, grants=None):
+        grants = grants or []
         self.grants = grants
 
     @property
@@ -396,7 +397,7 @@ class FakeTag(BaseModel):
 class LifecycleFilter(BaseModel):
 
     def __init__(self, prefix=None, tag=None, and_filter=None):
-        self.prefix = prefix or ''
+        self.prefix = prefix
         self.tag = tag
         self.and_filter = and_filter
 
@@ -404,7 +405,7 @@ class LifecycleFilter(BaseModel):
 class LifecycleAndFilter(BaseModel):
 
     def __init__(self, prefix=None, tags=None):
-        self.prefix = prefix or ''
+        self.prefix = prefix
         self.tags = tags
 
 
@@ -478,6 +479,8 @@ class FakeBucket(BaseModel):
         self.logging = {}
         self.notification_configuration = None
         self.accelerate_configuration = None
+        self.payer = 'BucketOwner'
+        self.creation_date = datetime.datetime.utcnow()
 
     @property
     def location(self):
@@ -494,6 +497,11 @@ class FakeBucket(BaseModel):
             expiration = rule.get('Expiration')
             transition = rule.get('Transition')
 
+            try:
+                top_level_prefix = rule['Prefix'] or ''  # If it's `None` the set to the empty string
+            except KeyError:
+                top_level_prefix = None
+
             nve_noncurrent_days = None
             if rule.get('NoncurrentVersionExpiration') is not None:
                 if rule["NoncurrentVersionExpiration"].get('NoncurrentDays') is None:
@@ -528,13 +536,22 @@ class FakeBucket(BaseModel):
             if rule.get("Filter"):
                 # Can't have both `Filter` and `Prefix` (need to check for the presence of the key):
                 try:
+                    # 'Prefix' cannot be outside of a Filter:
                     if rule["Prefix"] or not rule["Prefix"]:
                         raise MalformedXML()
                 except KeyError:
                     pass
 
+                filters = 0
+                try:
+                    prefix_filter = rule['Filter']['Prefix'] or ''  # If it's `None` the set to the empty string
+                    filters += 1
+                except KeyError:
+                    prefix_filter = None
+
                 and_filter = None
                 if rule["Filter"].get("And"):
+                    filters += 1
                     and_tags = []
                     if rule["Filter"]["And"].get("Tag"):
                         if not isinstance(rule["Filter"]["And"]["Tag"], list):
@@ -543,17 +560,34 @@ class FakeBucket(BaseModel):
                         for t in rule["Filter"]["And"]["Tag"]:
                             and_tags.append(FakeTag(t["Key"], t.get("Value", '')))
 
-                    and_filter = LifecycleAndFilter(prefix=rule["Filter"]["And"]["Prefix"], tags=and_tags)
+                    try:
+                        and_prefix = rule["Filter"]["And"]["Prefix"] or ''  # If it's `None` then set to the empty string
+                    except KeyError:
+                        and_prefix = None
+
+                    and_filter = LifecycleAndFilter(prefix=and_prefix, tags=and_tags)
 
                 filter_tag = None
                 if rule["Filter"].get("Tag"):
+                    filters += 1
                     filter_tag = FakeTag(rule["Filter"]["Tag"]["Key"], rule["Filter"]["Tag"].get("Value", ''))
 
-                lc_filter = LifecycleFilter(prefix=rule["Filter"]["Prefix"], tag=filter_tag, and_filter=and_filter)
+                # Can't have more than 1 filter:
+                if filters > 1:
+                    raise MalformedXML()
+
+                lc_filter = LifecycleFilter(prefix=prefix_filter, tag=filter_tag, and_filter=and_filter)
+
+            # If no top level prefix and no filter is present, then this is invalid:
+            if top_level_prefix is None:
+                try:
+                    rule['Filter']
+                except KeyError:
+                    raise MalformedXML()
 
             self.rules.append(LifecycleRule(
                 id=rule.get('ID'),
-                prefix=rule.get('Prefix'),
+                prefix=top_level_prefix,
                 lc_filter=lc_filter,
                 status=rule['Status'],
                 expiration_days=expiration.get('Days') if expiration else None,

moto/s3/responses.py

@@ -1310,7 +1310,7 @@ S3_ALL_BUCKETS = """<ListAllMyBucketsResult xmlns="http://s3.amazonaws.com/doc/2
     {% for bucket in buckets %}
       <Bucket>
         <Name>{{ bucket.name }}</Name>
-        <CreationDate>2006-02-03T16:45:09.000Z</CreationDate>
+        <CreationDate>{{ bucket.creation_date }}</CreationDate>
       </Bucket>
     {% endfor %}
  </Buckets>
@@ -1416,7 +1416,9 @@ S3_BUCKET_LIFECYCLE_CONFIGURATION = """<?xml version="1.0" encoding="UTF-8"?>
         <ID>{{ rule.id }}</ID>
         {% if rule.filter %}
         <Filter>
+            {% if rule.filter.prefix != None %}
             <Prefix>{{ rule.filter.prefix }}</Prefix>
+            {% endif %}
             {% if rule.filter.tag %}
             <Tag>
                 <Key>{{ rule.filter.tag.key }}</Key>
@@ -1425,7 +1427,9 @@ S3_BUCKET_LIFECYCLE_CONFIGURATION = """<?xml version="1.0" encoding="UTF-8"?>
             {% endif %}
             {% if rule.filter.and_filter %}
             <And>
+                {% if rule.filter.and_filter.prefix != None %}
                 <Prefix>{{ rule.filter.and_filter.prefix }}</Prefix>
+                {% endif %}
                 {% for tag in rule.filter.and_filter.tags %}
                 <Tag>
                     <Key>{{ tag.key }}</Key>
@@ -1436,7 +1440,9 @@ S3_BUCKET_LIFECYCLE_CONFIGURATION = """<?xml version="1.0" encoding="UTF-8"?>
             {% endif %}
         </Filter>
         {% else %}
-        <Prefix>{{ rule.prefix if rule.prefix != None }}</Prefix>
+            {% if rule.prefix != None %}
+            <Prefix>{{ rule.prefix }}</Prefix>
+            {% endif %}
         {% endif %}
         <Status>{{ rule.status }}</Status>
         {% if rule.storage_class %}

tests/test_s3/test_s3_lifecycle.py

@@ -59,15 +59,43 @@ def test_lifecycle_with_filters():
     with assert_raises(KeyError):
         assert result["Rules"][0]["Prefix"]
 
-    # With a tag:
+    # Without any prefixes and an empty filter (this is by default a prefix for the whole bucket):
-    lfc["Rules"][0]["Filter"]["Tag"] = {
+    lfc = {
-        "Key": "mytag",
+        "Rules": [
-        "Value": "mytagvalue"
+            {
+                "Expiration": {
+                    "Days": 7
+                },
+                "ID": "wholebucket",
+                "Filter": {},
+                "Status": "Enabled"
+            }
+        ]
     }
     client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
     result = client.get_bucket_lifecycle_configuration(Bucket="bucket")
     assert len(result["Rules"]) == 1
-    assert result["Rules"][0]["Filter"]["Prefix"] == ''
+    with assert_raises(KeyError):
+        assert result["Rules"][0]["Prefix"]
+
+    # If we remove the filter -- and don't specify a Prefix, then this is bad:
+    lfc['Rules'][0].pop('Filter')
+    with assert_raises(ClientError) as err:
+        client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
+    assert err.exception.response["Error"]["Code"] == "MalformedXML"
+
+    # With a tag:
+    lfc["Rules"][0]["Filter"] = {
+        'Tag': {
+            "Key": "mytag",
+            "Value": "mytagvalue"
+        }
+    }
+    client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
+    result = client.get_bucket_lifecycle_configuration(Bucket="bucket")
+    assert len(result["Rules"]) == 1
+    with assert_raises(KeyError):
+        assert result["Rules"][0]["Filter"]['Prefix']
     assert not result["Rules"][0]["Filter"].get("And")
     assert result["Rules"][0]["Filter"]["Tag"]["Key"] == "mytag"
     assert result["Rules"][0]["Filter"]["Tag"]["Value"] == "mytagvalue"
@@ -75,25 +103,25 @@ def test_lifecycle_with_filters():
         assert result["Rules"][0]["Prefix"]
 
     # With And (single tag):
-    lfc["Rules"][0]["Filter"]["And"] = {
+    lfc["Rules"][0]["Filter"] = {
-        "Prefix": "some/prefix",
+        "And": {
-        "Tags": [
+            "Prefix": "some/prefix",
-            {
+            "Tags": [
-                "Key": "mytag",
+                {
-                "Value": "mytagvalue"
+                    "Key": "mytag",
-            }
+                    "Value": "mytagvalue"
-        ]
+                }
+            ]
+        }
     }
     client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
     result = client.get_bucket_lifecycle_configuration(Bucket="bucket")
     assert len(result["Rules"]) == 1
-    assert result["Rules"][0]["Filter"]["Prefix"] == ""
+    assert not result["Rules"][0]["Filter"].get("Prefix")
     assert result["Rules"][0]["Filter"]["And"]["Prefix"] == "some/prefix"
     assert len(result["Rules"][0]["Filter"]["And"]["Tags"]) == 1
     assert result["Rules"][0]["Filter"]["And"]["Tags"][0]["Key"] == "mytag"
     assert result["Rules"][0]["Filter"]["And"]["Tags"][0]["Value"] == "mytagvalue"
-    assert result["Rules"][0]["Filter"]["Tag"]["Key"] == "mytag"
-    assert result["Rules"][0]["Filter"]["Tag"]["Value"] == "mytagvalue"
     with assert_raises(KeyError):
         assert result["Rules"][0]["Prefix"]
 
@@ -114,17 +142,39 @@ def test_lifecycle_with_filters():
     client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
     result = client.get_bucket_lifecycle_configuration(Bucket="bucket")
     assert len(result["Rules"]) == 1
-    assert result["Rules"][0]["Filter"]["Prefix"] == ""
+    assert not result["Rules"][0]["Filter"].get("Prefix")
     assert result["Rules"][0]["Filter"]["And"]["Prefix"] == "some/prefix"
     assert len(result["Rules"][0]["Filter"]["And"]["Tags"]) == 2
     assert result["Rules"][0]["Filter"]["And"]["Tags"][0]["Key"] == "mytag"
     assert result["Rules"][0]["Filter"]["And"]["Tags"][0]["Value"] == "mytagvalue"
-    assert result["Rules"][0]["Filter"]["Tag"]["Key"] == "mytag"
-    assert result["Rules"][0]["Filter"]["Tag"]["Value"] == "mytagvalue"
     assert result["Rules"][0]["Filter"]["And"]["Tags"][1]["Key"] == "mytag2"
     assert result["Rules"][0]["Filter"]["And"]["Tags"][1]["Value"] == "mytagvalue2"
-    assert result["Rules"][0]["Filter"]["Tag"]["Key"] == "mytag"
+    with assert_raises(KeyError):
-    assert result["Rules"][0]["Filter"]["Tag"]["Value"] == "mytagvalue"
+        assert result["Rules"][0]["Prefix"]
+
+    # And filter without Prefix but multiple Tags:
+    lfc["Rules"][0]["Filter"]["And"] = {
+        "Tags": [
+            {
+                "Key": "mytag",
+                "Value": "mytagvalue"
+            },
+            {
+                "Key": "mytag2",
+                "Value": "mytagvalue2"
+            }
+        ]
+    }
+    client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
+    result = client.get_bucket_lifecycle_configuration(Bucket="bucket")
+    assert len(result["Rules"]) == 1
+    with assert_raises(KeyError):
+        assert result["Rules"][0]["Filter"]["And"]["Prefix"]
+    assert len(result["Rules"][0]["Filter"]["And"]["Tags"]) == 2
+    assert result["Rules"][0]["Filter"]["And"]["Tags"][0]["Key"] == "mytag"
+    assert result["Rules"][0]["Filter"]["And"]["Tags"][0]["Value"] == "mytagvalue"
+    assert result["Rules"][0]["Filter"]["And"]["Tags"][1]["Key"] == "mytag2"
+    assert result["Rules"][0]["Filter"]["And"]["Tags"][1]["Value"] == "mytagvalue2"
     with assert_raises(KeyError):
         assert result["Rules"][0]["Prefix"]
 
@@ -146,6 +196,73 @@ def test_lifecycle_with_filters():
     assert not result["Rules"][0].get("Filter")
     assert result["Rules"][0]["Prefix"] == "some/path"
 
+    # Can't have Tag, Prefix, and And in a filter:
+    del lfc['Rules'][0]['Prefix']
+    lfc["Rules"][0]["Filter"] = {
+        "Prefix": "some/prefix",
+        "Tag": {
+                "Key": "mytag",
+                "Value": "mytagvalue"
+        }
+    }
+    with assert_raises(ClientError) as err:
+        client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
+    assert err.exception.response["Error"]["Code"] == "MalformedXML"
+
+    lfc["Rules"][0]["Filter"] = {
+        "Tag": {
+            "Key": "mytag",
+            "Value": "mytagvalue"
+        },
+        "And": {
+            "Prefix": "some/prefix",
+            "Tags": [
+                {
+                    "Key": "mytag",
+                    "Value": "mytagvalue"
+                },
+                {
+                    "Key": "mytag2",
+                    "Value": "mytagvalue2"
+                }
+            ]
+        }
+    }
+    with assert_raises(ClientError) as err:
+        client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
+    assert err.exception.response["Error"]["Code"] == "MalformedXML"
+
+    # Make sure multiple rules work:
+    lfc = {
+        "Rules": [
+            {
+                "Expiration": {
+                    "Days": 7
+                },
+                "ID": "wholebucket",
+                "Filter": {
+                    "Prefix": ""
+                },
+                "Status": "Enabled"
+            },
+            {
+                "Expiration": {
+                    "Days": 10
+                },
+                "ID": "Tags",
+                "Filter": {
+                    "Tag": {'Key': 'somekey', 'Value': 'somevalue'}
+                },
+                "Status": "Enabled"
+            }
+        ]
+    }
+    client.put_bucket_lifecycle_configuration(Bucket="bucket", LifecycleConfiguration=lfc)
+    result = client.get_bucket_lifecycle_configuration(Bucket="bucket")['Rules']
+    assert len(result) == 2
+    assert result[0]['ID'] == 'wholebucket'
+    assert result[1]['ID'] == 'Tags'
+
 
 @mock_s3
 def test_lifecycle_with_eodm():