[#316] Add VPC flow log module as an optional module by tung-nimblehq · Pull Request #317 · nimblehq/infrastructure-templates

tung-nimblehq · 2025-10-05T07:04:10Z

Close Enhance security posture and ensure compliance for infrastructure #316

What happened 👀

Add vpc-flow-log module
Upgrade Terraform to the latest version.
Add enabledSecurityFeatures flag to make vpc-flow-log module an optional module.
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

Add a Glue table for Athena to query flow logs. Since Athena charges based on queries: https://www.amazonaws.cn/en/athena/pricing/

Proof Of Work 📹

Apply on AWS correctly

Logs

S3 bucket

Athena Query

Example query:
99825aeb-13ee-4801-87c3-5c512f619a50.csv

Create VPC flow log in the Advanced template with flag set true.

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

Does not create VPC flow log in Blank template.

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

Does not create VPC flow log in the Advanced template with the flag not set.

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

templates/addons/aws/modules/vpc_flow_log/main.tf

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

@hoangmirs Can you recheck it? 🙏

hoangmirs · 2025-10-17T03:35:48Z

Could you help rebase against the develop branch again?

tung-nimblehq · 2025-10-17T03:47:25Z

Could you help rebase against the develop branch again?

Rebased 🙏

…and create dedicated locals and data files

toby-thanathip

@tung-nimblehq Just wondering if VPC Flow Log is something we've used in other projects?

If not, do you think it's essential for the B2B project? 🙏

Just a concern if we plan to add VPC Flow Log, it would be an additional cost for the project. 💭

toby-thanathip · 2026-03-05T05:14:40Z

.github/wiki/Security.md

+1. Run the infrastructure generator
+2. Select "Complete infrastructure (VPC + ECR + RDS + S3 + FARGATE + Cloudwatch + Security groups + ALB)" when prompted for infrastructure type
+3. Choose "Yes" when asked "Do you want to create (VPC Flow Logs + CloudTrail) to enhance security posture and compliance?"
+


Redundant line.

toby-thanathip · 2026-03-05T05:19:46Z

src/generators/addons/aws/advanced.test.ts

      expect(applyAwsEcs).toHaveBeenCalledWith(options);
    });
+
+    describe('given enabledSecurityFeatures is not set', () => {


Hmmmm, I'm not entirely convinced that VPC Flow and CloudTrail are security features.

→ Should we label them as Audit or Monitoring features instead?

If yes, we should rename Security.md accordingly and other variables too.

VPC Flow Logs and AWS CloudTrail are foundational components that bridge Security, Audit, and Monitoring in AWS. @toby-thanathip I used security since it includes Audit, and Monitoring.

toby-thanathip · 2026-03-05T05:39:16Z

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

+variable "s3_bucket_name" {
+  description = "The name of the S3 bucket to store the flow logs."
+  type        = string
+}


Do we need s3_key_prefix here too? Similar what we did for CloudTrail.

Updated in 0d16418

toby-thanathip · 2026-03-05T05:45:18Z

templates/addons/aws/modules/vpc_flow_log/main.tf

+    "projection.aws_region.type"       = "enum"
+    "projection.aws_region.values"     = "${data.aws_region.current.region}"
+    "projection.year.type"             = "integer"
+    "projection.year.range"            = "2025,2030" # Update the range as needed


Suggested change

"projection.year.range" = "2025,2030" # Update the range as needed

"projection.year.range" = "2026,2030" # Update the range as needed

Or, should it be a variable? 💭

Updated in 0d16418

toby-thanathip · 2026-03-05T07:13:18Z

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

+
+    env_namespace    = local.env_namespace
+    bucket_name      = "\${local.env_namespace}-flow-logs-\${data.aws_caller_identity.current.account_id}"
+    force_destroy    = true


Sounds concerning, are we sure we want to have it as true? 🙏

Updated in 0d16418

toby-thanathip · 2026-03-05T07:14:43Z

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

+const vpcFlowLogLocalesContent = dedent`
+  ### Begin VPC Flow Log ###
+  locals {
+    vpc_flow_log_s3_bucket_policy = {


I'm new to Terraform, forgive me; Why don't we add this to locals.tf?

→ Or should some parts be defined inside main.tf? 💭

It will be created inside core/locals.tf after we generate for the project, @toby-thanathip. According to our convention, constants should be defined in locals.tf, not in main.tf. Also, we shouldn’t move everything directly into core/locals.tf at this stage for two reasons: It would separate these values from the main module logic, which could make the flow harder to follow. Some locals may require dynamic values passed from modules, so defining them too early or too centrally could limit flexibility.

toby-thanathip · 2026-03-05T07:15:13Z

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

+
+import { AWS_TEMPLATE_PATH } from '../constants';
+
+const vpcFlowLogLocalesContent = dedent`


Suggested change

const vpcFlowLogLocalesContent = dedent`

const vpcFlowLogLocalsContent = dedent`

toby-thanathip · 2026-03-05T07:26:09Z

templates/addons/aws/modules/vpc_flow_log/locals.tf

+    { key = "flow_direction", value = "string" },
+    { key = "traffic_path", value = "int" },
+    { key = "ecs_task_id", value = "string" },
+    { key = "reject_reason", value = "string" },


Do you think all these columns are useful to us?

→ Just thinking if we reduce the amount of columns, it would reduce the cost for storage.

Fewer columns = smaller files = lower storage cost over time.

I think yes, it provides us with some useful information like reject_reason, traffic_path... we can adjust these columns at the project level. I think these columns are fine at the template level

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

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

+variable "log_retention_days" {
+  description = "The number of days to retain the flow logs in S3."
+  type        = number
+  default     = 90


Do you think 90 days is too long for troubleshooting cases? 💭

→ "The longer the retention, the more you accumulate, the more you pay."

Changed it to 30 days in 0d16418

toby-thanathip · 2026-03-05T07:48:09Z

src/generators/addons/aws/index.ts

+    type: 'confirm',
+    name: 'enabledSecurityFeatures',
+    message:
+      'Do you want to create (VPC Flow Logs + CloudTrail) to enhance security posture and compliance?',


Just thinking, if it's too strict too force them together.

→ Would there be any case where we only want one (instead of both), to reduce costs?

Good idea. updated in 0d16418

toby-thanathip · 2026-03-05T07:54:38Z

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

+};
+
+export default applyAwsVpcFlowLog;
+export { vpcFlowLogModuleContent, vpcFlowLogVariablesContent };


Any reason we don't export the outputs, same as we did for Cloudtrail?

I think we won't need any output from these modules for now. That's why I don't expose it here.

tung-nimblehq self-assigned this Oct 5, 2025

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

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

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch 2 times, most recently from debef21 to b55003b Compare October 5, 2025 07:24

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

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

hoangmirs reviewed Oct 8, 2025

View reviewed changes

templates/addons/aws/modules/vpc_flow_log/main.tf Outdated Show resolved Hide resolved

templates/addons/aws/modules/vpc_flow_log/main.tf Show resolved Hide resolved

templates/addons/aws/modules/vpc_flow_log/main.tf Outdated Show resolved Hide resolved

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 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-flows-log-module branch from 220e3c5 to c222713 Compare January 10, 2026 04:38

tung-nimblehq changed the base branch from develop to feature/316-separate-s3-and-alb-module January 10, 2026 04:39

tung-nimblehq force-pushed the feature/316-separate-s3-and-alb-module branch from 5fc06d7 to 0aa0817 Compare January 15, 2026 15:26

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-separate-s3-and-alb-module branch 3 times, most recently from a7c33ad to b619cf2 Compare January 16, 2026 11:23

Base automatically changed from feature/316-separate-s3-and-alb-module to develop January 16, 2026 11:27

tung-nimblehq added 2 commits January 16, 2026 21:48

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

f032b0f

…and create dedicated locals and data files

[#316] Add VPC flow log module

84a9710

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

toby-thanathip reviewed Mar 5, 2026

View reviewed changes

tung-nimblehq force-pushed the feature/316-add-vpc-flows-log-module branch from 795a49a to 0d16418 Compare March 6, 2026 06:29

tung-nimblehq requested a review from toby-thanathip March 6, 2026 06:31

[#316] Add checkbox type prompt allow select individual module

fe38800

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

	"projection.year.range" = "2025,2030" # Update the range as needed
	"projection.year.range" = "2026,2030" # Update the range as needed


		import { AWS_TEMPLATE_PATH } from '../constants';

		const vpcFlowLogLocalesContent = dedent`

	const vpcFlowLogLocalesContent = dedent`
	const vpcFlowLogLocalsContent = dedent`

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 📹

Apply on AWS correctly

Create VPC flow log in the Advanced template with flag set true.

Does not create VPC flow log in Blank template.

Does not create VPC flow log in the Advanced template with the flag not set.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tung-nimblehq commented Oct 17, 2025

Uh oh!

hoangmirs commented Oct 17, 2025

Uh oh!

tung-nimblehq commented Oct 17, 2025

Uh oh!

toby-thanathip left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toby-thanathip Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

tung-nimblehq Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

toby-thanathip Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

toby-thanathip left a comment •

edited

Loading

toby-thanathip Mar 5, 2026 •

edited

Loading

tung-nimblehq Mar 6, 2026 •

edited

Loading

toby-thanathip Mar 5, 2026 •

edited

Loading