[#316] Add Cloudtrail module as an optional module by tung-nimblehq · Pull Request #318 · nimblehq/infrastructure-templates

tung-nimblehq · 2025-10-05T15:41:47Z

Close Enhance security posture and ensure compliance for infrastructure #316

What happened 👀

Created CloudTrail for Management, Insight, and Data events.
Created an S3 bucket for both CloudTrail, separated by prefix.
Created Event Bridge + AWS Chatbox + SNS topic to notify when failed login events happen.
Add some unit tests.

Insight 📝

The Terraform apply will fail since this PR has not been merged yet: [#286] Fix: Terraform plan can't be applied because of VPC endpoint misconfiguration #309

Proof Of Work 📹

Applied correctly

Cloudtrail

S3 bucket

Sns topic

Slack alerts

Does not create Cloudtrail in Blank template.

Screen.Recording.2025-10-05.at.22.27.44.mov

Does not create CloudTrail in the Advanced template with the flag not set.

Screen.Recording.2025-10-05.at.22.29.50.mov

Create CloudTrail in the Advanced template with the flag set.

Screen.Recording.2025-10-05.at.22.29.03.mov

Install CloudTrail into Bank template.

Screen.Recording.2025-10-05.at.22.28.16.mov

src/generators/addons/aws/constants.ts

hoangmirs · 2025-10-08T05:05:38Z

templates/addons/aws/modules/cloudtrail/s3-bucket-cloudtrail/main.tf

+}
+
+# trivy:ignore:AVD-AWS-0089 Access logging not required for CloudTrail log bucket
+resource "aws_s3_bucket" "cloudtrail_logs" {


We are duplicating S3 code in a few modules. Should we adjust the current S3 module, or create a new S3 module that we can use for these purposes? What do you think?

😅 I don’t think so, since this bucket is dedicated to CloudTrail logs, it’s quite different from a general S3 bucket. Adjusting or creating a new S3 module would require adding many new variables to adapt it and easily cause confusion. I think we should keep it under CloudTrail for easier management.

hoangmirs · 2025-10-08T05:06:56Z

templates/addons/aws/modules/cloudtrail/sns/main.tf

+
+# trivy:ignore:AVD-AWS-0136 No encryption for CloudTrail compatibility
+# trivy:ignore:AVD-AWS-0095 No encryption for CloudTrail compatibility
+resource "aws_sns_topic" "cloudtrail" {


Same for SNS, can we create a general SNS module and reuse here?

Same as this one
it’s dedicated to the CloudTrail configuration. I’d prefer to keep it under CloudTrail rather than introduce new modules that can’t work independently.

tung-nimblehq · 2025-10-17T02:00:38Z

@hoangmirs Can you recheck it? 🙏

toby-thanathip · 2026-03-05T03:59:21Z

.github/wiki/Security.md

+- **CloudWatch integration**: Sends logs to CloudWatch for real-time monitoring and alerting
+- **S3 storage**: Stores all CloudTrail logs securely in Amazon S3 with configurable key prefix organization
+- **SNS notifications**: Integrates with SNS topics for immediate alerting on critical events
+- **Insight events**: Captures unusual activity patterns like API call rate and error rate anomalies


Is Insight events overlapping with Comprehensive event logging?

toby-thanathip · 2026-03-05T04:00:13Z

.github/wiki/Security.md

+
+- **Comprehensive event logging**: Captures management events, data events, and insight events based on configuration
+- **Multi-region support**: Can be configured to log events across all AWS regions for complete visibility
+- **CloudWatch integration**: Sends logs to CloudWatch for real-time monitoring and alerting


Preferably put CloudWatch integration together with SNS notifications

→ Group them, as they both relate to alerting/monitoring.

Updated in 54e1427

toby-thanathip · 2026-03-05T07:50:11Z

src/generators/addons/aws/modules/cloudtrail.ts

+    s3_key_prefix              = "cloudtrail"
+    log_retention_days         = var.cloudtrail_log_retention_days
+    s3_ignore_data_bucket_arns = ["arn:aws:s3:::\${module.cloudtrail_s3_bucket.aws_s3_bucket_name}"]
+    cloud_watch_arn            = module.cloudtrail_cloudwatch.aws_cloudwatch_log_group_arn


Should it have cloudtrail_event_type here too? Or, just rely on the default?

I think we can just rely on cloudtrail_event_type default value for the template here. If in case the project want to change it, there is already one var for the project to adjust.

toby-thanathip · 2026-03-05T07:51:23Z

templates/addons/aws/modules/cloudtrail/variables.tf

+variable "log_retention_days" {
+  description = "Number of days to retain CloudTrail logs in CloudWatch Logs."
+  type        = number
+  default     = 90


Hmmmm, I see we defined 365 in the other cloudtrail.ts.

Changed to 30 days in 54e1427

toby-thanathip · 2026-03-05T07:51:55Z

src/generators/addons/aws/modules/cloudtrail.ts

+  variable "cloudtrail_log_retention_days" {
+    description = "The number of days to retain CloudTrail logs in CloudWatch"
+    type        = number
+    default     = 365


Do you think it's too long? Similar to this comment.

Changed to 30 days in 54e1427

toby-thanathip · 2026-03-05T07:52:29Z

src/generators/addons/aws/modules/cloudtrail.ts

+
+const cloudtrailLocalesContent = dedent`
+  ### Begin CloudTrail ###
+  locals {


Similar comment as here.

Replied in https://github.com/nimblehq/infrastructure-templates/pull/317/changes#r2888884910

…and create dedicated locals and data files

tung-nimblehq added this to the 2.4.0 milestone Oct 5, 2025

tung-nimblehq self-assigned this Oct 5, 2025

tung-nimblehq added the type : feature New feature or request label Oct 5, 2025

tung-nimblehq marked this pull request as ready for review October 5, 2025 15:42

tung-nimblehq requested review from andyduong1920, bterone, hoangmirs, liamstevens111, nvminhtue and sanG-github as code owners October 5, 2025 15:42

hoangmirs reviewed Oct 8, 2025

View reviewed changes

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch from b55003b to 73b05d7 Compare October 12, 2025 02:09

tung-nimblehq requested a review from suho as a code owner October 12, 2025 02:09

tung-nimblehq force-pushed the feature/316-add-vpc-cloud-trail-module branch from f600b45 to 5f47bab Compare October 12, 2025 02:37

tung-nimblehq requested a review from hoangmirs October 17, 2025 02:00

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch from 2625247 to 220e3c5 Compare October 17, 2025 03:46

tung-nimblehq force-pushed the feature/316-add-vpc-cloud-trail-module branch from a512c22 to c3b6064 Compare October 17, 2025 03:47

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch from 220e3c5 to c222713 Compare January 10, 2026 04:38

tung-nimblehq force-pushed the feature/316-add-vpc-cloud-trail-module branch 2 times, most recently from 7421b23 to 0a89c65 Compare January 10, 2026 09:43

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch from c222713 to 8f894f4 Compare January 15, 2026 15:55

tung-nimblehq force-pushed the feature/316-add-vpc-cloud-trail-module branch from 0a89c65 to 491687b Compare January 15, 2026 16:09

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch from 8f894f4 to 84a9710 Compare January 16, 2026 14:57

tung-nimblehq force-pushed the feature/316-add-vpc-cloud-trail-module branch from 491687b to 88243e5 Compare January 16, 2026 15:07

toby-thanathip reviewed Mar 5, 2026

View reviewed changes

tung-nimblehq force-pushed the feature/316-add-vpc-cloud-trail-module branch from 88243e5 to 2a604e2 Compare March 6, 2026 06:51

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch from 0d16418 to fe38800 Compare March 6, 2026 07:27

tung-nimblehq added 2 commits March 6, 2026 14:28

[#323]Make s3 module to be generic, and move locals out of main file …

834550b

…and create dedicated locals and data files

[#316] Add Cloudtrail modules

54e1427

tung-nimblehq force-pushed the feature/316-add-vpc-cloud-trail-module branch from 2a604e2 to 54e1427 Compare March 6, 2026 07:28

tung-nimblehq requested a review from toby-thanathip March 6, 2026 07:36

Conversation

tung-nimblehq commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What happened 👀

Insight 📝

Proof Of Work 📹

Applied correctly

Does not create Cloudtrail in Blank template.

Does not create CloudTrail in the Advanced template with the flag not set.

Create CloudTrail in the Advanced template with the flag set.

Install CloudTrail into Bank template.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tung-nimblehq commented Oct 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tung-nimblehq commented Oct 5, 2025 •

edited

Loading