From 615995bc2bb0848673c16d4a52b1f383b7d905ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20M=C3=A9nab=C3=A9?=
 <francois.menabe@gmail.com>
Date: Wed, 20 Apr 2022 11:03:33 +0200
Subject: [PATCH] s3_lifecycle: support value '0' for transition_days (#1077)

s3_lifecycle: support value '0' for transition_days

SUMMARY
s3_lifecycle module does not support value 0 for transition_days parameter.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
A lifecycle rule with 0 as transition_days allows to create a rule that will move objects with a delta of just few hours (if set to 1, objects will be moved with a delta of 1d + few hours).
When the value 0 is set, the value is stripped from the query and we get an error that is hard to correlate with an "invalid"  value on this parameter (which is valid as 0 is an integer):
- name: Create s3 buckets lifecycle rules
  s3_lifecycle:
    ec2_url: "FIXME"
    region: "FIXME"
    aws_access_key: "FIXME"
    aws_secret_key: "FIXME"
    name: "FIXME"
    rule_id: "onezone"
    transitions:
    - transition_days: 0
      storage_class: "ONEZONE_IA"
    state: present
    status: enabled

fatal: [localhost]: FAILED! => {
    "boto3_version": "1.21.3",
    "botocore_version": "1.24.19",
    "changed": false,
    "error": {
        "code": "MalformedXML",
        "message": "Extra element Transition in interleave"
    },
    "lifecycle_configuration": {
        "Rules": [
            {
                "Filter": {
                    "Prefix": ""
                },
                "ID": "onezone",
                "Status": "Enabled",
                "Transitions": [
                    {
                        "StorageClass": "ONEZONE_IA"
                    }
                ]
            }
        ]
    },
    ...
MSG:

An error occurred (MalformedXML) when calling the PutBucketLifecycleConfiguration operation: Extra element Transition in interleave

This is because transition.get('transition_days') returns 0 which is considered as False on a condition (this patch just check if the value is defined; e.g. is not None).

Reviewed-by: Mark Chappell <None>
Reviewed-by: Markus Bergholz <git@osuv.de>
(cherry picked from commit 4f989bdd5578373f97ce31008f50a15c825af5ef)
---
 ...1077-s3_lifecycle-transition-days-zero.yml |  2 ++
 plugins/modules/s3_lifecycle.py               |  4 ++--
 .../targets/s3_lifecycle/tasks/main.yml       | 24 +++++++++++++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/1077-s3_lifecycle-transition-days-zero.yml

diff --git a/changelogs/fragments/1077-s3_lifecycle-transition-days-zero.yml b/changelogs/fragments/1077-s3_lifecycle-transition-days-zero.yml
new file mode 100644
index 00000000000..cdef5072d6b
--- /dev/null
+++ b/changelogs/fragments/1077-s3_lifecycle-transition-days-zero.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - s3_lifecycle - add support of value *0* for ``transition_days`` (https://github.com/ansible-collections/community.aws/pull/1077).
diff --git a/plugins/modules/s3_lifecycle.py b/plugins/modules/s3_lifecycle.py
index c12ce6b0897..3f0bd784cef 100644
--- a/plugins/modules/s3_lifecycle.py
+++ b/plugins/modules/s3_lifecycle.py
@@ -314,7 +314,7 @@ def build_rule(client, module):
             t_out = dict()
             if transition.get('transition_date'):
                 t_out['Date'] = transition['transition_date']
-            elif transition.get('transition_days'):
+            elif transition.get('transition_days') is not None:
                 t_out['Days'] = transition['transition_days']
             if transition.get('storage_class'):
                 t_out['StorageClass'] = transition['storage_class'].upper()
@@ -596,7 +596,7 @@ def main():
                                  'noncurrent_version_transition_days',
                                  'noncurrent_version_transitions')
         for param in required_when_present:
-            if module.params.get(param):
+            if module.params.get(param) is None:
                 break
         else:
             msg = "one of the following is required when 'state' is 'present': %s" % ', '.join(required_when_present)
diff --git a/tests/integration/targets/s3_lifecycle/tasks/main.yml b/tests/integration/targets/s3_lifecycle/tasks/main.yml
index 566a8381c19..67871545bda 100644
--- a/tests/integration/targets/s3_lifecycle/tasks/main.yml
+++ b/tests/integration/targets/s3_lifecycle/tasks/main.yml
@@ -180,6 +180,30 @@
         prefix: /something
       register: output
 
+    - assert:
+        that:
+          - output is not changed
+  # ============================================================
+    - name: Create a lifecycle policy, with glacier and transition_days to 0
+      s3_lifecycle:
+        name: '{{ bucket_name }}'
+        transition_days: 0
+        storage_class: glacier
+        prefix: /something
+      register: output
+
+    - assert:
+        that:
+          - output is changed
+  # ============================================================
+    - name: Create a lifecycle policy, with glacier and transition_days to 0 (idempotency)
+      s3_lifecycle:
+        name: '{{ bucket_name }}'
+        transition_days: 0
+        storage_class: glacier
+        prefix: /something
+      register: output
+
     - assert:
         that:
           - output is not changed