Add s3_bucket_notification module

[aljazkosir]

SUMMARY

PR to introduce lambda_bucket_event module.

ISSUE TYPE

• New Module Pull Request

COMPONENT NAME

• lib/ansible/modules/cloud/amazon/lambda_bucket_event.py
• test/units/modules/cloud/amazon/test_lambda_bucket_event.py

ADDITIONAL INFORMATION

This module allows the management of AWS Lambda function bucket event mappings via the Ansible framework. Use module lambda to manage the lambda function itself, lambda_alias to manage function aliases and lambda_policy to modify lambda permissions.

Example:

---
# Example that creates a lambda event notification for a bucket
- hosts: localhost
  gather_facts: no
  tasks:
  - name: Process jpg image
    s3_bucket_notification:
      state: present
      event_name: on_jpg_add_or_remove
      bucket_name: test-bucket
      function_name: arn:aws:lambda:us-east-2:525810310200:function:test-lambda
      events: ["s3:ObjectCreated:*", "s3:ObjectRemoved:*"]
      prefix: images/
      suffix: .jpg

[ansibot]

@aljazkosir, just so you are aware we have a dedicated Working Group for aws.
You can find other people interested in this in #ansible-aws on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

[aljazkosir]

bot_status

[ansibot]

Components

lib/ansible/modules/cloud/amazon/lambda_bucket_event.py
support: community
maintainers:

test/units/modules/cloud/amazon/test_lambda_bucket_event.py
support: core
maintainers:

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

[aljazkosir]

ready_for_review

[willthames]

This module seems the wrong approach - I'd prefer that this was just a parameter in the s3_bucket module

[ansibot]

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/lambda_bucket_event.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

The test ansible-test sanity --test metaclass-boilerplate [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/lambda_bucket_event.py:0:0: missing: __metaclass__ = type

click here for bot help

[aljazkosir]

@willthames I updated the arguments and added test that checks that order of events doesn't matter

[willthames]

Can we just call this argument lambda_alias without the alias (just thinking about future proofing for other notification targets)

[willthames]

Can we just call this argument lambda_version without the alias (just thinking about future proofing for other notification targets)

[willthames]

Need to make the test suite name match the module name. I don't mind if that's adding aws_ to the module name or removing aws_ from the test suite name

[aljazkosir]

Removing _aws from the test suite name seems better to me.

[willthames]

I've tested this locally, everything looks great. Good to merge from my point of view

[jillr]

Overall this looks good, but I'm concerned about the 41MB function.zip. We have to carry this around in the repo (and ansible is already fairly hefty), and I'm getting timeouts on the "register lambda" test some of the time. This will likely lead to these tests being unstable in CI. It would be better to use a simple hello world style script like test/integration/targets/aws_lambda/files/mini_lambda.py or test/integration/targets/lambda_policy/files/mini_http_lambda.py.

[jillr]

I'd also personally prefer it if the function were in-repo as a regular file, then zipped in the test, from a review and accounting perspective. It makes it much easier to track and ensure that nothing is sneaking into the repo via a binary.

[mattclay]

We should not include a file of this size in the repo. Use a very tiny lambda function and create the zip file during the test. If that's not possible, then we can put the zip file in our S3 bucket for CI -- but lets avoid that if possible.

[mattclay]

Ideally the lambda would be a singe python file with no requirements outside the standard library, then no zip would be needed. Keeping it python vs another language makes it easier to review since most contributors will be familiar with python.

[aljazkosir]

@jillr @mattclay Of course, I totally forgot about that zip, sorry about that. It is now zipped during the test without any extra requirements, so it should be fairly small.
ready_for_review

Dismissing out-of-date review.

[aljazkosir]

I changed the authors, so the right people will be pinged.

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/s3_bucket_notification.py:0:0: E305 DOCUMENTATION.author: Invalid author for dictionary value @ data['author']. Got ['XLAB Steampunk d.o.o. (@xlab-si/steampunk)']

click here for bot help

[ansibot]

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/s3_bucket_notification.py:29:0: SyntaxError: Non-ASCII character '\xc5' in file /root/ansible/lib/ansible/modules/cloud/amazon/s3_bucket_notification.py on line 30, but no encoding declared; see http://www.python.org/peps/pep-0263.html for details

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/s3_bucket_notification.py:29:0: SyntaxError: Non-ASCII character '\xc5' in file /root/ansible/lib/ansible/modules/cloud/amazon/s3_bucket_notification.py on line 30, but no encoding declared; see http://python.org/dev/peps/pep-0263/ for details

click here for bot help

[jillr]

Thanks @aljazkosir that looks much better, appreciate all your work on this module!