Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cloudtrail: Integration tests #61919

Open
wants to merge 1 commit into
base: devel
from

Conversation

@tremble
Copy link
Contributor

commented Sep 6, 2019

SUMMARY

Add integration tests for the cloudtrail module

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/modules/cloud/amazon/cloudtrail.py
hacking/aws_config/testing_policies/security-policy.json
hacking/aws_config/testing_policies/storage-policy.json

ADDITIONAL INFORMATION
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

@tremble tremble force-pushed the tremble:cloudtrail_tests branch from b1258de to ce0ce91 Sep 6, 2019

@tremble tremble changed the title WIP cloudtrail: Initial integration tests cloudtrail: Integration tests Sep 6, 2019

@tremble tremble force-pushed the tremble:cloudtrail_tests branch from ce0ce91 to bc06aab Sep 6, 2019

@ansibot ansibot added core_review and removed WIP labels Sep 6, 2019

@tremble tremble force-pushed the tremble:cloudtrail_tests branch from bc06aab to 331665d Sep 6, 2019

@tremble

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

@shepdelacreme

These tests have also exposed a couple of possible bugs:

  • output.exists returns false when creating
  • Changed reports true when using a KMS alias
  • Tag keys are being lower-cased

As well as some quirks

  • No way to remove SNS Topic
  • No way to remove KMS Encryption key
  • No way to remove CloudWatch settings
  • It'd be worth cleanly failing when you try to enable Multi-Region logging, but disable logging of global events (which AWS rejects)

@samdoran samdoran requested a review from jillr Sep 12, 2019

@samdoran samdoran removed the needs_triage label Sep 12, 2019

@jillr

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

Thanks very much @tremble for the tests - could you please open an issue(s) for those findings?

@tremble

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2019

@jillr Done. I wanted to give @shepdelacreme chance to comment just in case I'd misunderstood how things were intended to function and avoid bumping the Issue list further.

@ansibot ansibot added the has_issue label Sep 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.