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.

Sign up for GitHub

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

Jump to bottom

route53_info max_items parameter does not work with queries that use non-paginated APIs #1383

Closed
1 task done
mcoot opened this issue Aug 2, 2022 · 3 comments
Closed
1 task done

route53_info max_items parameter does not work with queries that use non-paginated APIs #1383

mcoot opened this issue Aug 2, 2022 · 3 comments
Labels
bug This issue/PR relates to a bug module module needs_triage plugins plugin (any type) python3

Comments

@mcoot
Copy link
Contributor

mcoot commented Aug 2, 2022

Summary

Attempting to use the route53_info module with particular parameters causes an error coming from boto3:

Parameter validation failed:
Unknown parameter in input: "PaginationConfig", must be one of: DNSName, HostedZoneId, MaxItems

In particular, this occurs using the query: hosted_zone parameter with hosted_zone_method: list_by_name or query: reusable_delegation_set without specifying a delegation_set_id.

We noticed this when updating Ansible (and community.aws) versions. I believe this was introduced in version 3.0.0 (and was not present in 2.6.1) and that it was caused by this PR: #813 which was addressing #529.

In particular, this PR replaced all occurrences of setting the MaxItems parameter in route53_info with creating and passing a PaginationConfig object. However the PaginationConfig is associated with a boto3 paginator, and not all boto3 calls in the module use a paginator. For those that do not, namely the two listed above, PaginationConfig is not a valid parameter, hence the parameter validation failure.

That these commands take a MaxItems parameter directly can be seen in the Boto3 docs:

The implementations in route53_info of list_hosted_zones_by_name and reusable_delegation_set_details show that they are currently being passed a PaginationConfig parameter if max_items is specified despite not using a paginator.

I am raising a PR along with this report to fix the issue by setting MaxItems directly for these plays instead of the incorrect PaginationConfig. I am sorry if the format here is a little informal - I encountered this in an internal work repository which I can't easily share outputs from. I hope the explanation here is clear.

Issue Type

Bug Report

Component Name

route53_info

Ansible Version

$ ansible --version
ansible [core 2.13.2]
  config file = None
  configured module search path = ['/Users/jspearritt/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.9/site-packages/ansible
  ansible collection location = /Users/jspearritt/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.9.13 (main, May 24 2022, 21:28:44) [Clang 13.0.0 (clang-1300.0.29.30)]
  jinja version = 3.1.2
  libyaml = True

Collection Versions

AWS SDK versions

$ pip show boto boto3 botocore
Name: boto
Version: 2.49.0
Summary: Amazon Web Services Library
Home-page: https://github.com/boto/boto/
Author: Mitch Garnaat
Author-email: mitch@garnaat.com
License: MIT
Location: /usr/local/lib/python3.9/site-packages
Requires: 
Required-by: 
---
Name: boto3
Version: 1.24.43
Summary: The AWS SDK for Python
Home-page: https://github.com/boto/boto3
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /usr/local/lib/python3.9/site-packages
Requires: botocore, jmespath, s3transfer
Required-by: 
---
Name: botocore
Version: 1.27.43
Summary: Low-level, data-driven core of boto 3.
Home-page: https://github.com/boto/botocore
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /usr/local/lib/python3.9/site-packages
Requires: jmespath, python-dateutil, urllib3
Required-by: boto3, s3transfer

Configuration

No response

OS / Environment

Tested running plays under Ubuntu 20.04 and MacOS Mojave

Steps to Reproduce

- name: "Find the hosted zone ID of foobar.net"
  route53_facts:
    query: hosted_zone
    hosted_zone_method: list_by_name
    dns_name: "foobar.net."
    max_items: 1
  register: foobar_details

Expected Results

The task should successfully retrieve the hosted zone by name, returning a maximum of one item.

Actual Results

The play fails with an error coming from boto3:

Parameter validation failed:
Unknown parameter in input: "PaginationConfig", must be one of: DNSName, HostedZoneId, MaxItems

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibullbot
Copy link

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibullbot
Copy link

cc @Etherdaemon @jillr @markuman @s-hertel @tremble
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module needs_triage plugins plugin (any type) python3 labels Aug 2, 2022
@mcoot mcoot mentioned this issue Aug 2, 2022
Merged
softwarefactory-project-zuul bot pushed a commit that referenced this issue Aug 2, 2022
@mcoot
route53_info - fix max_items when not paginating (#1384)
569fff4 
route53_info - fix max_items when not paginating

SUMMARY
As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically:

query: hosted_zone parameter with hosted_zone_method: list_by_name
query: reusable_delegation_set without specifying a delegation_set_id

I believe this is a regression introduced in #813
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method.
Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator.

Reviewed-by: Mark Chappell <None>
patchback bot pushed a commit that referenced this issue Aug 2, 2022
@mcoot @patchback
route53_info - fix max_items when not paginating (#1384)
2fd7a4c 
route53_info - fix max_items when not paginating

SUMMARY
As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically:

query: hosted_zone parameter with hosted_zone_method: list_by_name
query: reusable_delegation_set without specifying a delegation_set_id

I believe this is a regression introduced in #813
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method.
Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator.

Reviewed-by: Mark Chappell <None>
(cherry picked from commit 569fff4)
@patchback patchback bot mentioned this issue Aug 2, 2022
Merged
patchback bot pushed a commit that referenced this issue Aug 2, 2022
@mcoot @patchback
route53_info - fix max_items when not paginating (#1384)
8f9470f 
route53_info - fix max_items when not paginating

SUMMARY
As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically:

query: hosted_zone parameter with hosted_zone_method: list_by_name
query: reusable_delegation_set without specifying a delegation_set_id

I believe this is a regression introduced in #813
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method.
Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator.

Reviewed-by: Mark Chappell <None>
(cherry picked from commit 569fff4)
@patchback patchback bot mentioned this issue Aug 2, 2022
Merged
@tremble
Copy link
Contributor

tremble commented Aug 2, 2022

fixed by #1384

@tremble tremble closed this as completed Aug 2, 2022
softwarefactory-project-zuul bot pushed a commit that referenced this issue Aug 2, 2022
@patchback
route53_info - fix max_items when not paginating (#1384) (#1387)
68d9a4d 
[PR #1384/569fff4c backport][stable-3] route53_info - fix max_items when not paginating

This is a backport of PR #1384 as merged into main (569fff4).
SUMMARY
As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically:

query: hosted_zone parameter with hosted_zone_method: list_by_name
query: reusable_delegation_set without specifying a delegation_set_id

I believe this is a regression introduced in #813
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method.
Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator.

Reviewed-by: Mark Chappell <None>
softwarefactory-project-zuul bot pushed a commit that referenced this issue Aug 2, 2022
@patchback
route53_info - fix max_items when not paginating (#1384) (#1388)
17773b6 
[PR #1384/569fff4c backport][stable-4] route53_info - fix max_items when not paginating

This is a backport of PR #1384 as merged into main (569fff4).
SUMMARY
As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically:

query: hosted_zone parameter with hosted_zone_method: list_by_name
query: reusable_delegation_set without specifying a delegation_set_id

I believe this is a regression introduced in #813
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method.
Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator.

Reviewed-by: Mark Chappell <None>
@renovate renovate bot mentioned this issue Aug 8, 2022
Merged
1 task
abikouo pushed a commit to abikouo/community.aws that referenced this issue Oct 24, 2023
@mcoot @goneri
route53_info - fix max_items when not paginating (ansible-collections…
a868d64 
…#1384)

route53_info - fix max_items when not paginating

SUMMARY
As reported in ansible-collections#1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically:

query: hosted_zone parameter with hosted_zone_method: list_by_name
query: reusable_delegation_set without specifying a delegation_set_id

I believe this is a regression introduced in ansible-collections#813
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method.
Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator.

Reviewed-by: Mark Chappell <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@569fff4
abikouo added a commit to abikouo/community.aws that referenced this issue Oct 24, 2023
@abikouo
fix idempotency issue when creating existing key pair with key materi…
b769b6a 
…al (ansible-collections#1383)

ec2_key - fix idempotency issue when updating existing key pair

SUMMARY

when running module with state=present on existing key pair with the same key material the module raises the following issue
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: TypeError: ansible_collections.amazon.aws.plugins.module_utils.modules.AnsibleAWSModule.exit_json() argument after ** must be a mapping, not NoneType
fatal: [localhost]: FAILED! => {"changed": false, "module_stderr": "Traceback (most recent call last):\n  File \"/home/aubin/.ansible/tmp/ansible-tmp-1676994806.9488113-2761194-77181612241045/AnsiballZ_ec2_key.py\", line 100, in <module>\n    _ansiballz_main()\n  File \"/home/aubin/.ansible/tmp/ansible-tmp-1676994806.9488113-2761194-77181612241045/AnsiballZ_ec2_key.py\", line 92, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/aubin/.ansible/tmp/ansible-tmp-1676994806.9488113-2761194-77181612241045/AnsiballZ_ec2_key.py\", line 40, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.amazon.aws.plugins.modules.ec2_key', init_globals=dict(_module_fqn='ansible_collections.amazon.aws.plugins.modules.ec2_key', _modlib_path=modlib_path),\n  File \"/usr/lib64/python3.9/runpy.py\", line 225, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib64/python3.9/runpy.py\", line 97, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib64/python3.9/runpy.py\", line 87, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_amazon.aws.ec2_key_payload_tghhfma0/ansible_amazon.aws.ec2_key_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_key.py\", line 389, in <module>\n  File \"/tmp/ansible_amazon.aws.ec2_key_payload_tghhfma0/ansible_amazon.aws.ec2_key_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_key.py\", line 385, in main\nTypeError: ansible_collections.amazon.aws.plugins.module_utils.modules.AnsibleAWSModule.exit_json() argument after ** must be a mapping, not NoneType\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

This pull request aims to fix this issue

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

ec2_key

Reviewed-by: Mark Chappell
Reviewed-by: Bikouo Aubin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This issue/PR relates to a bug module module needs_triage plugins plugin (any type) python3
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tremble @mcoot @ansibullbot