Integration tests for s3_logging (#63257)

* s3_logging: (integration tests) updated AWS policy

* s3_logging: fix sanity test issues

* s3_logging: Integration tests

* Add pauses to cope with evenual consistency

* Mark s3_logging tests as 'unsupported' for now due to testing instability

hacking/aws_config/testing_policies/storage-policy.json

@@ -6,24 +6,27 @@
             "Action": [
                 "s3:CreateBucket",
                 "s3:Delete*",
+                "s3:GetBucketAcl",
+                "s3:GetBucketLogging",
+                "s3:GetBucketNotification",
                 "s3:GetBucketPolicy",
                 "s3:GetBucketRequestPayment",
                 "s3:GetBucketTagging",
                 "s3:GetBucketVersioning",
                 "s3:GetEncryptionConfiguration",
                 "s3:GetObject",
-                "s3:GetBucketNotification",
                 "s3:HeadBucket",
                 "s3:List*",
                 "s3:PutBucketAcl",
+                "s3:PutBucketLogging",
+                "s3:PutBucketNotification",
                 "s3:PutBucketPolicy",
                 "s3:PutBucketRequestPayment",
                 "s3:PutBucketTagging",
                 "s3:PutBucketVersioning",
                 "s3:PutEncryptionConfiguration",
                 "s3:PutObject",
-                "s3:PutObjectAcl",
-                "s3:PutBucketNotification"
+                "s3:PutObjectAcl"
             ],
             "Effect": "Allow",
             "Resource": [

lib/ansible/modules/cloud/amazon/s3_logging.py

@@ -24,18 +24,22 @@ options:
     description:
       - "Name of the s3 bucket."
     required: true
+    type: str
   state:
     description:
       - "Enable or disable logging."
     default: present
     choices: [ 'present', 'absent' ]
+    type: str
   target_bucket:
     description:
       - "The bucket to log to. Required when state=present."
+    type: str
   target_prefix:
     description:
       - "The prefix that should be prepended to the generated log files written to the target_bucket."
     default: ""
+    type: str
 extends_documentation_fragment:
     - aws
     - ec2

test/integration/targets/s3_logging/aliases

@@ -0,0 +1,4 @@
+cloud/aws
+#shippable/aws/group1
+# when running tests we saw an ~20% failure rate
+unsupported

test/integration/targets/s3_logging/defaults/main.yml

@@ -0,0 +1,4 @@
+---
+test_bucket: '{{ resource_prefix }}-testbucket'
+log_bucket_1: '{{ resource_prefix }}-logs-1'
+log_bucket_2: '{{ resource_prefix }}-logs-2'

test/integration/targets/s3_logging/tasks/main.yml

@@ -0,0 +1,203 @@
+---
+# Integration tests for s3_logging
+#
+# Notes:
+# - s3_logging doesn't support check_mode and the only output is 'changed'
+# - During initial testing we hit issues with boto reporting
+#   "You must give the log-delivery group WRITE and READ_ACP permissions
+#   to the target bucket"
+#   a long term solution might be to port s3_logging to AnsibleAWSModule
+#   so we can add retries
+#
+- module_defaults:
+    group/aws:
+      aws_access_key: '{{ aws_access_key | default(omit) }}'
+      aws_secret_key: '{{ aws_secret_key | default(omit) }}'
+      security_token: '{{ security_token | default(omit) }}'
+      region: '{{ aws_region | default(omit) }}'
+  block:
+
+  # ============================================================
+
+  - name: Try to enable logging without providing target_bucket
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+    register: result
+    ignore_errors: yes
+
+  - assert:
+      that:
+      - result is failed
+
+  # ============================================================
+  - name: Create simple s3_bucket to be logged
+    s3_bucket:
+      state: present
+      name: '{{ test_bucket }}'
+    register: output
+
+  - assert:
+      that:
+      - output is changed
+      - output.name == test_bucket
+
+  - name: Create simple s3_bucket as target for logs
+    s3_bucket:
+      state: present
+      name: '{{ log_bucket_1 }}'
+    register: output
+
+  - assert:
+      that:
+      - output is changed
+      - output.name == log_bucket_1
+
+  - name: Create simple s3_bucket as second target for logs
+    s3_bucket:
+      state: present
+      name: '{{ log_bucket_2 }}'
+    register: output
+
+  - assert:
+      that:
+      - output is changed
+      - output.name == log_bucket_2
+
+# ============================================================
+
+  - name: Enable logging
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_1 }}'
+    register: result
+
+  - assert:
+      that:
+      - result is changed
+
+  - name: Enable logging idempotency
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_1 }}'
+    register: result
+
+  - assert:
+      that:
+      - result is not changed
+
+# ============================================================
+
+  - name: Change logging bucket
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_2 }}'
+    register: result
+
+  - assert:
+      that:
+      - result is changed
+
+  - name: Change logging bucket idempotency
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_2 }}'
+    register: result
+
+  - assert:
+      that:
+      - result is not changed
+
+# ============================================================
+
+  - name: Change logging prefix
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_2 }}'
+      target_prefix: '/{{ resource_prefix }}/'
+    register: result
+
+  - assert:
+      that:
+      - result is changed
+
+  - name: Change logging prefix idempotency
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_2 }}'
+      target_prefix: '/{{ resource_prefix }}/'
+    register: result
+
+  - assert:
+      that:
+      - result is not changed
+
+# ============================================================
+
+  - name: Remove logging prefix
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_2 }}'
+    register: result
+
+  - assert:
+      that:
+      - result is changed
+
+  - name: Remove logging prefix idempotency
+    s3_logging:
+      state: present
+      name: '{{ test_bucket }}'
+      target_bucket: '{{ log_bucket_2 }}'
+    register: result
+
+  - assert:
+      that:
+      - result is not changed
+
+# ============================================================
+
+  - name: Disable logging
+    s3_logging:
+      state: absent
+      name: '{{ test_bucket }}'
+    register: result
+
+  - assert:
+      that:
+      - result is changed
+
+  - name: Disable logging idempotency
+    s3_logging:
+      state: absent
+      name: '{{ test_bucket }}'
+    register: result
+
+  - assert:
+      that:
+      - result is not changed
+
+# ============================================================
+  always:
+  - name: Delete bucket being logged
+    s3_bucket:
+      name: '{{ test_bucket }}'
+      state: absent
+    ignore_errors: yes
+  - name: Delete first bucket containing logs
+    s3_bucket:
+      name: '{{ log_bucket_1 }}'
+      state: absent
+    ignore_errors: yes
+  - name: Delete second bucket containing logs
+    s3_bucket:
+      name: '{{ log_bucket_2 }}'
+      state: absent
+    ignore_errors: yes

test/sanity/ignore.txt

@@ -1051,7 +1051,6 @@ lib/ansible/modules/cloud/amazon/s3_bucket.py validate-modules:doc-missing-type
 lib/ansible/modules/cloud/amazon/s3_lifecycle.py validate-modules:undocumented-parameter
 lib/ansible/modules/cloud/amazon/s3_lifecycle.py validate-modules:parameter-type-not-in-doc
 lib/ansible/modules/cloud/amazon/s3_lifecycle.py validate-modules:doc-missing-type
-lib/ansible/modules/cloud/amazon/s3_logging.py validate-modules:doc-missing-type
 lib/ansible/modules/cloud/amazon/s3_sync.py future-import-boilerplate
 lib/ansible/modules/cloud/amazon/s3_sync.py metaclass-boilerplate
 lib/ansible/modules/cloud/amazon/s3_sync.py pylint:blacklisted-name