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

Fix route53_info max_items / type being ignored #813

Merged

Conversation

marknet15
Copy link
Contributor

SUMMARY

Currently if max_items is set on the route53_info module then it is ignored meaning all items are returned.

type is also ignored due to an incorrect if statement

It looks like it was a regression introduced here:
ansible/ansible@6075536#diff-23a0c9250633162d50c3f06442b7a552a5ae0659a24dd01a328c0e165e473616

The tests have been updated to reflect a check on max_items and type

Fixes: #529

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

route53_info

ADDITIONAL INFORMATION

The problem with max_items being ignored is resolved by adding PaginationConfig and adding MaxItems to that instead.

The problem with type being ignored is resolved by fixing an if statement.

Boto3 docs: https://boto3.amazonaws.com/v1/documentation/api/1.18.7/reference/services/route53.html#Route53.Paginator.ListResourceRecordSets

Update tests and add soft warning for boto3 version requirement

remove boto requirement change

Fix change made to delegation_set details
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good, but one minor niggle.

plugins/modules/route53_info.py Outdated Show resolved Hide resolved
plugins/modules/route53_info.py Outdated Show resolved Hide resolved
Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just follow @markuman's suggestion!

@markuman
Copy link
Member

recheck

@markuman markuman added the gate label Nov 30, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ansible-zuul ansible-zuul bot merged commit e323f83 into ansible-collections:main Nov 30, 2021
@markuman
Copy link
Member

Thank you @marknet15

@marknet15 marknet15 deleted the fix-route53_info-max_items branch November 30, 2021 18:29
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Aug 2, 2022
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 pull request Aug 2, 2022
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 bot pushed a commit that referenced this pull request Aug 2, 2022
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)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Aug 2, 2022
[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 pull request Aug 2, 2022
[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>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
)

Fix route53_info max_items / type being ignored

SUMMARY
Currently if max_items is set on the route53_info module then it is ignored meaning all items are returned.
type is also ignored due to an incorrect if statement
It looks like it was a regression introduced here:
ansible/ansible@6075536#diff-23a0c9250633162d50c3f06442b7a552a5ae0659a24dd01a328c0e165e473616
The tests have been updated to reflect a check on max_items and type
Fixes: ansible-collections#529
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
The problem with max_items being ignored is resolved by adding PaginationConfig and adding MaxItems to that instead.
The problem with type being ignored is resolved by fixing an if statement.
Boto3 docs: https://boto3.amazonaws.com/v1/documentation/api/1.18.7/reference/services/route53.html#Route53.Paginator.ListResourceRecordSets

Reviewed-by: Mark Chappell <None>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: None <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@e323f83
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

community.aws.route53_info query:record_sets - type and max_items ignored on all 2.10 versions
4 participants