Refactor s3_bucket_notifications to support SNS / SQS #940
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansibullbot
added
community_review
feature
|
Build failed.
|
|
Build failed.
|
There was a problem hiding this comment.
@marknet15 Thank you. Can you please add a changelog fragment as well?
marknet15
force-pushed
the
extend-s3-notifications
branch
from
2a27278
to
4eb0ce5
Compare
marknet15
force-pushed
the
extend-s3-notifications
branch
from
4eb0ce5
to
25db6b0
Compare
alinabuzachis
approved these changes
Mar 11, 2022
There was a problem hiding this comment.
@marknet15 Thank you. I only left one small suggestion, other than that LGTM!
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
marknet15
force-pushed
the
extend-s3-notifications
branch
from
b30351b
to
ebb4abf
Compare
alinabuzachis
approved these changes
Mar 24, 2022
alinabuzachis
added
the
backport-3
markuman
approved these changes
Mar 28, 2022
Backport to stable-3: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply dd89ed1 on top of patchback/backports/stable-3/dd89ed152413dad7b1d0a84049a9f647221eedb6/pr-940 Backporting merged PR #940 into main
🤖 @patchback |
marknet15
added a commit
to marknet15/community.aws
that referenced
this pull request
Mar 28, 2022
…tions#940) Refactor s3_bucket_notifications to support SNS / SQS SUMMARY Refactor s3_bucket_notifications to extend module to support the extra targets of SNS and SQS along with the currently supported Lambda functions. Summary of changes: Refactor module to support SNS/SQS targets along with current Lambda function support. Fix check mode coverage Update integration tests to more comprehensive cover functionality. Update documentation in sns_topic and sqs_queue modules to add policy setting example. Fixes: ansible-collections#140 ISSUE TYPE Feature Pull Request COMPONENT NAME s3_bucket_notifications ADDITIONAL INFORMATION https://boto3.amazonaws.com/v1/documentation/api/1.16.0/reference/services/s3.html#S3.Client.put_bucket_notification_configuration Reviewed-by: Alina Buzachis <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Markus Bergholz <git@osuv.de> (cherry picked from commit dd89ed1)
marknet15
added a commit
to marknet15/community.aws
that referenced
this pull request
Mar 29, 2022
…tions#940) Refactor s3_bucket_notifications to support SNS / SQS SUMMARY Refactor s3_bucket_notifications to extend module to support the extra targets of SNS and SQS along with the currently supported Lambda functions. Summary of changes: Refactor module to support SNS/SQS targets along with current Lambda function support. Fix check mode coverage Update integration tests to more comprehensive cover functionality. Update documentation in sns_topic and sqs_queue modules to add policy setting example. Fixes: ansible-collections#140 ISSUE TYPE Feature Pull Request COMPONENT NAME s3_bucket_notifications ADDITIONAL INFORMATION https://boto3.amazonaws.com/v1/documentation/api/1.16.0/reference/services/s3.html#S3.Client.put_bucket_notification_configuration Reviewed-by: Alina Buzachis <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Markus Bergholz <git@osuv.de>
marknet15
added a commit
to marknet15/community.aws
that referenced
this pull request
Mar 29, 2022
…tions#940) Refactor s3_bucket_notifications to support SNS / SQS SUMMARY Refactor s3_bucket_notifications to extend module to support the extra targets of SNS and SQS along with the currently supported Lambda functions. Summary of changes: Refactor module to support SNS/SQS targets along with current Lambda function support. Fix check mode coverage Update integration tests to more comprehensive cover functionality. Update documentation in sns_topic and sqs_queue modules to add policy setting example. Fixes: ansible-collections#140 ISSUE TYPE Feature Pull Request COMPONENT NAME s3_bucket_notifications ADDITIONAL INFORMATION https://boto3.amazonaws.com/v1/documentation/api/1.16.0/reference/services/s3.html#S3.Client.put_bucket_notification_configuration Reviewed-by: Alina Buzachis <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Markus Bergholz <git@osuv.de>
softwarefactory-project-zuul bot
pushed a commit
that referenced
this pull request
Mar 29, 2022
Backport: Refactor s3_bucket_notifications to support SNS / SQS (#940) - stable-3 SUMMARY Backport #940 manually to resolve conflict ISSUE TYPE Feature Pull Request COMPONENT NAME s3_bucket_notification ADDITIONAL INFORMATION Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/community.aws
that referenced
this pull request
Oct 24, 2023
More work on ELBv2 module_utils SUMMARY Refactors LB creation and makes sure that ip_address_type is set on creation (bug found when working on fixing NLB tests) Fixes bug with DefaultAction comparisons Extends tests for _prune_ForwardConfig Extends tests for _prune_secrets Fixes KeyError bug uncovered when extending tests for _prune_secrets ISSUE TYPE Bugfix Pull Request COMPONENT NAME plugins/module_utils/elbv2.py ADDITIONAL INFORMATION Fixes: ansible-collections#604 See also: ansible-collections#1365 Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/community.aws
that referenced
this pull request
Oct 24, 2023
elbv2: respect UseExistingClientSecret SUMMARY Since amazon.aws 5.0.0, elb_application_lb runs into an exception, when using Type: authenticate-oidc in a rule, even when UseExistingClientSecret: True parameter is given. That works as expected with amazon.aws 4.x.x. The logic gets broken in ansible-collections#940 Basically AWS won't return both, UseExistingClientSecret and ClientSecret. But when requesting against boto3, both parameters are mutually exclusive! When the user set UseExistingClientSecret: True, the ClientSecret must be removed for the request. When the user does not set UseExistingClientSecret or set it to False, the UseExistingClientSecret must be included in the request. While diving deeper, I've noticed a basic change detection problem for default values, that are not requested, but AWS will return them. I've summerized it in ansible-collections#1284 However, this PR does not target ansible-collections#1284, it just fixes the exception and restores the functionality and hotfix the change-detection only for Type: authenticate-oidc. origin PR description The error was: botocore.errorfactory.InvalidLoadBalancerActionException: An error occurred (InvalidLoadBalancerAction) when calling the ModifyRule operation: You must either specify a client secret or set UseExistingClientSecret to true UseExistingClientSecret is not respected anymore since a.a 5 Introduced in ansible-collections#940 Furthermore, AWS returns also Scope and SessionTimeout parameters that are filled with default values if not requested. 'Scope': 'openid', 'SessionTimeout': 604800, That make the module always returns a change, if they are not requested. This fix does not break backwards compatibility, because the values are already set by aws, when not requested yet. ISSUE TYPE Bugfix Pull Request COMPONENT NAME plugins/module_utils/elbv2.yml ADDITIONAL INFORMATION - Conditions: - Field: host-header Values: - some.tld - Field: path-pattern Values: - "/admin/*" Actions: - Type: authenticate-oidc Order: 1 AuthenticateOidcConfig: Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0 AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo ClientId: fasgd-235463-fsgd-243 ClientSecret: "{{ lookup('onepassword', 'some cool secret', vault='some important vault') }}" SessionCookieName: AWSELBAuthSessionCookie OnUnauthenticatedRequest: authenticate UseExistingClientSecret: True - TargetGroupName: "{{ some_tg }}" Type: forward Order: 2 Reviewed-by: Alina Buzachis Reviewed-by: Mark Chappell
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Refactor
s3_bucket_notificationsto extend module to support the extra targets of SNS and SQS along with the currently supported Lambda functions.Summary of changes:
sns_topicandsqs_queuemodules to add policy setting example.Fixes: #140
ISSUE TYPE
COMPONENT NAME
s3_bucket_notifications
ADDITIONAL INFORMATION
https://boto3.amazonaws.com/v1/documentation/api/1.16.0/reference/services/s3.html#S3.Client.put_bucket_notification_configuration