Skip to content

Conversation

@lorenzo-merici
Copy link
Collaborator

@lorenzo-merici lorenzo-merici commented Apr 9, 2025

Support cross-region and cross-accounts deployments for Cloud Logs integration

Changes:

  • Stackset for cross accounts bucket
  • Stackset for cross accounts topic
  • Update documentation

This allows the module to work correctly when the S3 bucket and SNS topic are in different regions

@lorenzo-merici lorenzo-merici marked this pull request as ready for review April 10, 2025 14:36
@lorenzo-merici lorenzo-merici requested review from a team as code owners April 10, 2025 14:36
@matteopasa matteopasa requested a review from Copilot April 10, 2025 15:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

modules/log_ingestion.s3.cft.yaml:148

  • [nitpick] Consider updating 'CloudlogsS3Access' to 'CloudLogsS3Access' for consistent capitalization across the template.
              - Sid: "CloudlogsS3Access"

Copy link
Contributor

@ravinadhruve10 ravinadhruve10 left a comment

Choose a reason for hiding this comment

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

Mainly comments around using soon-to-be-deprecated parameter.

default: Bucket Account ID
TopicAccountId:
default: SNS Topic Account ID
OrganizationalUnitIds:
Copy link
Contributor

Choose a reason for hiding this comment

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

With include/exclude support for AWS, we want to avoid using this parameter since it will be deprecated soon anyway.
Could you please update this to use the new parameters instead as this? we should start adding org install support to cloud-logs using the new params.

Copy link
Contributor

Choose a reason for hiding this comment

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

Approving this for now. This support for all new installs with new include/exclude based parameters can be added in a separate PR.

default: Bucket Account ID
TopicAccountId:
default: SNS Topic Account ID
OrganizationalUnitIds:
Copy link
Contributor

Choose a reason for hiding this comment

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

Approving this for now. This support for all new installs with new include/exclude based parameters can be added in a separate PR.

@lorenzo-merici lorenzo-merici merged commit d5733e5 into main Apr 14, 2025
3 checks passed
@lorenzo-merici lorenzo-merici deleted the feat-cross-account-cloud-logs branch April 14, 2025 08:47
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.

5 participants