route53 - Refactor to use boto3

[pjrm]

• Refactor route53 module to use boto3 instead of boto

SUMMARY

• Migrate boto to boto3
• Update requirements in documentation

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

route53 - Refactor to use boto3

[ansibullbot]

cc @jillr @jimbydamonk @s-hertel @tremble @wimnat
click here for bot help

[tremble]

Thanks for taking the time to migrate this module to boto3. That the integration tests pass is certainly a good start.

I've not had chance to run through most of the code, but there are a number of helpers which should make things easier for you.
See also: https://docs.ansible.com/ansible/devel/dev_guide/platforms/aws_guidelines.html

In general:

• use module.fail_json_aws(e, msg="Some message") rather than module.fail_json(msg=e)
• use is_boto3_error_message / is_boto3_error_code rather than trying to catch the API specific exceptions
• we also have a 'retry' decorator which is helpful on busy accounts as it will automatically retry on things like API ratelimiting exceptions https://docs.ansible.com/ansible/devel/dev_guide/platforms/aws_guidelines.html#api-throttling-rate-limiting-and-pagination (Looks like I need to check for a couple of old examples that still try to use e.response['Error'])

[pjrm]

Hi @tremble,

Thank you very much for the help and quick reply.

I have performed the changes based on the guide and your suggestions.

[tremble]

I'd like to see the paginated queries split into their own small functions, just doing the pagination, and we can use scrub_non_params rather than our own function. But other than that I think this PR is ready to roll. Thanks for working on this.

[tremble]

You mentioned being interested in helping with community.aws in general. Not as part of this PR, but a separate PR we should probably deprecate the 'get' command here.

You'll notice that in tests/sanity/ignore-2.11.txt there's an entry

plugins/modules/route53.py validate-modules:parameter-state-invalid-choice

https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_best_practices.html#scoping-your-module-s

Do not add get, list or info state options to an existing module - create a new _info or _facts module

'get' commands are generally better handled by a dedicated _info module. I believe route53_info can be used to do this so if you're willing to raise a follow up PR it would be appreciated

[tremble]

Thanks for you work on this. Should be available in community.aws:1.4.0

[eRadical]

Just re-downloaded community.aws:1.4.0 and it complains that

    "msg": "The config profile (companyAccount-dev) could not be found"

While the following is ok:

$ aws configure get region --profile companyAccount-dev
eu-west-1

[eRadical]

@tremble is there anyway I can help you to debug this?

[Having route53 work with profiles will be a huge plus as I'll delete a huge chunk of ansible code that does sts_assume_role saves the result in a variable... etc... ]

[tremble]

@eRadical, WFM...

(ansible-dev) [17:21:16+0100] ~/vcs/ansible 
[✔ ansible] $ ansible-playbook test.yml 
[DEPRECATION WARNING]: Ansible will require Python 3.8 or newer on the controller starting with Ansible 2.12. Current version: 3.6.8 (default, Aug 18 2020, 08:33:21) [GCC 8.3.1 20191121 (Red Hat 8.3.1-5)]. This
 feature will be removed from ansible-core in version 2.12. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features under development. This is a rapidly
changing source of code and can become unstable at any point.
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] **************************************************************************************************************************************************************************************************

TASK [community.aws.route53] **************************************************************************************************************************************************************************************
changed: [localhost]

TASK [community.aws.route53] **************************************************************************************************************************************************************************************
changed: [localhost]

PLAY RECAP ********************************************************************************************************************************************************************************************************
localhost                  : ok=2    changed=2    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

(ansible-dev) [17:21:23+0100] ~/vcs/ansible 
[✔ ansible] $ cat test.yml 
- hosts: localhost
  gather_facts: False
  connection: local
  collections:
  - amazon.aws
  - community.aws
  tasks:
  - community.aws.route53:
      zone: example.test
      record: home-sweet-home.example.test
      type: A
      value: 127.0.0.1
      state: present
      profile: myprofile
  - community.aws.route53:
      zone: example.test
      record: home-sweet-home.example.test
      type: A
      value: 127.0.0.1
      state: absent
      profile: myprofile

One key piece is that "profile" is passed directly to the boto3 library, if you don't use "local", then you'll need to make sure that the profile is available wherever the module is actually being executed (note: this will be the Ansible target not the controller.)

[eRadical]

Yup... my bad.

I was inheriting become: true from the playbook/role.
Needless to say that on my machine there is no AWS profile under root.

Thank you @tremble for your time... and thanx @pjrm & @tremble for enhancing community.aws.route53!