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

Add support for SQS RawMessageDelievery attribute in subscriptions - issue #193 #640

Merged
merged 1 commit into from
Jul 3, 2022
Merged

Conversation

canidam
Copy link
Contributor

@canidam canidam commented Jul 14, 2021

SUMMARY

Added support to configure RawMessageDelievery option when configuring sqs endpoints.

It use boto3 set_subscription_attributes() to configure changes.
It currently supports only this option, but should be easily extended in the future.

Attributes are expected in the form as in the boto3 docs:
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sns.html#SNS.Client.set_subscription_attributes

Fixes #193

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

updates sns_topic.py to support new functionality

example:

- sns_topic:
    name: '{{ sns_topic_topic_name }}'
    display_name: My new topic name
    subscriptions:
    - endpoint: "{{ sqs_arn }}"
      protocol: sqs
      attributes:
        RawMessageDelivery: true

@ansibullbot
Copy link

@tremble
Copy link
Contributor

tremble commented Aug 1, 2021

recheck

@canidam
Copy link
Contributor Author

canidam commented Aug 21, 2021

@alinabuzachis @jillr can you review the PR please?

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.

Thanks for taking the time to submit this PR.

Please remove the .ansible-test directory, and Docker and Pipfile files. If you believe they should be added in general then open a separate PR, however they look like they're re-inventing ansible-test

@canidam canidam requested a review from tremble August 26, 2021 09:11
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.

Thanks for taking the time to submit this PR. A couple of small issues.

plugins/modules/sns_topic.py Outdated Show resolved Hide resolved
plugins/modules/sns_topic.py Outdated Show resolved Hide resolved
plugins/modules/sns_topic.py Outdated Show resolved Hide resolved
plugins/modules/sns_topic.py Outdated Show resolved Hide resolved
…or RawMessageDelievery (SQS)

Add sqs subscription for testing
@github-actions
Copy link

github-actions bot commented Jul 3, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@tremble
Copy link
Contributor

tremble commented Jul 3, 2022

Sorry it's taken so long to get back to this review. I've rebased this against the latest change sets and I think it's ready to be merged

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 13s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 55s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 11s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 50s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 13m 15s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 46s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 43s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 10s
✔️ ansible-test-splitter SUCCESS in 2m 32s
✔️ integration-community.aws-1 SUCCESS in 7m 30s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble tremble added mergeit Merge the PR (SoftwareFactory) backport-4 PR should be backported to the stable-4 branch labels Jul 3, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 51s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 54s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 11s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 31s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 21s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 8m 59s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 7m 24s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 55s
✔️ ansible-test-splitter SUCCESS in 2m 35s
✔️ integration-community.aws-1 SUCCESS in 6m 05s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 932263d into ansible-collections:main Jul 3, 2022
@patchback
Copy link

patchback bot commented Jul 3, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/932263d415ac7b16f06d4c26f2c960e4ebf3af01/pr-640

Backported as #1302

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jul 3, 2022
…or RawMessageDelievery (SQS) (#640)

Add support for SQS RawMessageDelievery attribute in subscriptions - issue #193

SUMMARY

Added support to configure RawMessageDelievery option when configuring sqs endpoints.
It use boto3 set_subscription_attributes() to configure changes.
It currently supports only this option, but should be easily extended in the future.
Attributes are expected in the form as in the boto3 docs:
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sns.html#SNS.Client.set_subscription_attributes
Fixes #193

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

updates sns_topic.py to support new functionality

example:

- sns_topic:
    name: '{{ sns_topic_topic_name }}'
    display_name: My new topic name
    subscriptions:
    - endpoint: "{{ sqs_arn }}"
      protocol: sqs
      attributes:
        RawMessageDelivery: true

Reviewed-by: Jill R <None>
Reviewed-by: None <None>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Mark Chappell <None>
(cherry picked from commit 932263d)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jul 3, 2022
…or RawMessageDelievery (SQS) (#640) (#1302)

[PR #640/932263d4 backport][stable-4] Add support for SQS RawMessageDelievery attribute in subscriptions - issue #193

This is a backport of PR #640 as merged into main (932263d).
SUMMARY

Added support to configure RawMessageDelievery option when configuring sqs endpoints.
It use boto3 set_subscription_attributes() to configure changes.
It currently supports only this option, but should be easily extended in the future.
Attributes are expected in the form as in the boto3 docs:
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sns.html#SNS.Client.set_subscription_attributes
Fixes #193

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

updates sns_topic.py to support new functionality

example:

- sns_topic:
    name: '{{ sns_topic_topic_name }}'
    display_name: My new topic name
    subscriptions:
    - endpoint: "{{ sqs_arn }}"
      protocol: sqs
      attributes:
        RawMessageDelivery: true

Reviewed-by: Mark Chappell <None>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Feb 3, 2023
sns_topic - Fix Permission Issue for Cross Account Subscriptions

SUMMARY

sns_topic currently fails with the following error if it has any cross account subscriptions:
Couldn't get subscription attributes for subscription arn:aws:sns:us-east-1:123412341234:my-sns-topic-name:555950dc-7c5f-416c-8f8e-e8f38eabfa54: An error occurred (AuthorizationError) when calling the GetSubscriptionAttributes operation: Not authorized to access this subscription

This happens, for example, when a Lambda function in account A is subscribed to an SNS topic in account B, as described here.
I believe this was caused by #640.
I am not sure how to write a test for this specific situation as it would require multiple AWS accounts.

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

sns_topic
ADDITIONAL INFORMATION



- community.aws.sns_topic:
    name: my-sns-topic-in-account-123412341234
    subscriptions:
      - endpoint: "arn:aws:lambda:us-east-1:567856785678:function:my-lambda-function-in-account-567856785678"
        protocol: lambda
    state: present

Reviewed-by: Mark Chappell <None>
patchback bot pushed a commit that referenced this pull request Feb 3, 2023
sns_topic - Fix Permission Issue for Cross Account Subscriptions

SUMMARY

sns_topic currently fails with the following error if it has any cross account subscriptions:
Couldn't get subscription attributes for subscription arn:aws:sns:us-east-1:123412341234:my-sns-topic-name:555950dc-7c5f-416c-8f8e-e8f38eabfa54: An error occurred (AuthorizationError) when calling the GetSubscriptionAttributes operation: Not authorized to access this subscription

This happens, for example, when a Lambda function in account A is subscribed to an SNS topic in account B, as described here.
I believe this was caused by #640.
I am not sure how to write a test for this specific situation as it would require multiple AWS accounts.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

sns_topic
ADDITIONAL INFORMATION

- community.aws.sns_topic:
    name: my-sns-topic-in-account-123412341234
    subscriptions:
      - endpoint: "arn:aws:lambda:us-east-1:567856785678:function:my-lambda-function-in-account-567856785678"
        protocol: lambda
    state: present

Reviewed-by: Mark Chappell <None>
(cherry picked from commit de21c4b)
patchback bot pushed a commit that referenced this pull request Feb 3, 2023
sns_topic - Fix Permission Issue for Cross Account Subscriptions

SUMMARY

sns_topic currently fails with the following error if it has any cross account subscriptions:
Couldn't get subscription attributes for subscription arn:aws:sns:us-east-1:123412341234:my-sns-topic-name:555950dc-7c5f-416c-8f8e-e8f38eabfa54: An error occurred (AuthorizationError) when calling the GetSubscriptionAttributes operation: Not authorized to access this subscription

This happens, for example, when a Lambda function in account A is subscribed to an SNS topic in account B, as described here.
I believe this was caused by #640.
I am not sure how to write a test for this specific situation as it would require multiple AWS accounts.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

sns_topic
ADDITIONAL INFORMATION

- community.aws.sns_topic:
    name: my-sns-topic-in-account-123412341234
    subscriptions:
      - endpoint: "arn:aws:lambda:us-east-1:567856785678:function:my-lambda-function-in-account-567856785678"
        protocol: lambda
    state: present

Reviewed-by: Mark Chappell <None>
(cherry picked from commit de21c4b)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Feb 3, 2023
…) (#1700)

[PR #1418/de21c4bd backport][stable-4] sns_topic - Fix Permission Issue for Cross Account Subscriptions

This is a backport of PR #1418 as merged into main (de21c4b).
SUMMARY

sns_topic currently fails with the following error if it has any cross account subscriptions:
Couldn't get subscription attributes for subscription arn:aws:sns:us-east-1:123412341234:my-sns-topic-name:555950dc-7c5f-416c-8f8e-e8f38eabfa54: An error occurred (AuthorizationError) when calling the GetSubscriptionAttributes operation: Not authorized to access this subscription

This happens, for example, when a Lambda function in account A is subscribed to an SNS topic in account B, as described here.
I believe this was caused by #640.
I am not sure how to write a test for this specific situation as it would require multiple AWS accounts.

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

sns_topic
ADDITIONAL INFORMATION



- community.aws.sns_topic:
    name: my-sns-topic-in-account-123412341234
    subscriptions:
      - endpoint: "arn:aws:lambda:us-east-1:567856785678:function:my-lambda-function-in-account-567856785678"
        protocol: lambda
    state: present

Reviewed-by: Mark Chappell <None>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Feb 3, 2023
…) (#1701)

[PR #1418/de21c4bd backport][stable-5] sns_topic - Fix Permission Issue for Cross Account Subscriptions

This is a backport of PR #1418 as merged into main (de21c4b).
SUMMARY

sns_topic currently fails with the following error if it has any cross account subscriptions:
Couldn't get subscription attributes for subscription arn:aws:sns:us-east-1:123412341234:my-sns-topic-name:555950dc-7c5f-416c-8f8e-e8f38eabfa54: An error occurred (AuthorizationError) when calling the GetSubscriptionAttributes operation: Not authorized to access this subscription

This happens, for example, when a Lambda function in account A is subscribed to an SNS topic in account B, as described here.
I believe this was caused by #640.
I am not sure how to write a test for this specific situation as it would require multiple AWS accounts.

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

sns_topic
ADDITIONAL INFORMATION



- community.aws.sns_topic:
    name: my-sns-topic-in-account-123412341234
    subscriptions:
      - endpoint: "arn:aws:lambda:us-east-1:567856785678:function:my-lambda-function-in-account-567856785678"
        protocol: lambda
    state: present

Reviewed-by: Mark Chappell <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4 PR should be backported to the stable-4 branch community_review feature This issue/PR relates to a feature request integration tests/integration mergeit Merge the PR (SoftwareFactory) module module new_contributor Help guide this first time contributor plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sns_topic - subscriptions section lacks setting the raw message delivery option
5 participants