feat(perforce): add P4 Broker ECS submodule by hwkiem · Pull Request #871 · aws-games/cloud-game-development-toolkit

hwkiem · 2026-02-14T00:36:57Z

Summary

Add P4 Broker (Perforce Helix Broker) as a fourth component in the Perforce Terraform module:

Dockerfile and documentation for building the P4 Broker container image
Complete ECS submodule (modules/perforce/modules/p4-broker/) with 12 files covering IAM, security groups, NLB target group, ECS Fargate service, S3 config bucket, and broker configuration template
Root module integration: p4_broker_config variable, shared ECS cluster condition, NLB TCP listener, inter-service security group rules, and outputs
Unit tests: 6 test scenarios covering conditional creation, ECS cluster behavior, NLB listener creation, existing cluster support, and full-stack deployment
Documentation: mkdocs navigation entries for Docker image and submodule READMEs

Dependencies

This PR is staged after:

refactor(docs): rename section index files to README.md #869 — refactor/docs-rename-section-indexes (mkdocs paths)
fix(perforce): restructure tests and fix pre-existing module bugs #870 — fix/perforce-test-infrastructure-and-bugs (module fixes, flat test structure, netapp-ontap provider)

Both must merge before this PR can cleanly merge to main.

Architecture

The P4 Broker runs as an ECS Fargate service behind the shared NLB. It accepts TCP connections on a configurable port (default 1666) and proxies them to the upstream P4 Server, applying command filtering rules defined in the broker configuration.

Client → NLB (TCP :1666) → P4 Broker (ECS Fargate) → P4 Server (EC2)

Test plan

terraform test in modules/perforce/ — all 20 tests pass (8 conditional + 6 shared + 6 broker)
terraform fmt -check -recursive modules/perforce/ — formatting OK
terraform validate in modules/perforce/ — configuration valid
Pre-commit hooks pass on all changed files
Documentation build succeeds

github-actions · 2026-02-14T00:37:52Z

📚 Documentation Preview

✅ Preview deployed successfully!

🔗 Preview URL: https://aws-games.github.io/cloud-game-development-toolkit/preview-pr-871/

🔒 Maintainer Action Required

The preview requires approval before it's accessible. A maintainer must approve the GitHub Pages deployment in the Environments section.

Once approved, the preview will be accessible within 1-2 minutes.

Build Information

Status: ✅ Deployed (awaiting approval)
Commit: d68d8db7977999bbe4c50a7a521962901c200cd6
Workflow Run: #97

This preview will be automatically deleted when the PR is merged or closed.

modules/perforce/lb.tf

+resource "aws_lb_listener" "perforce_broker" {
+  count             = var.p4_broker_config != null && var.create_shared_network_load_balancer ? 1 : 0
+  load_balancer_arn = aws_lb.perforce[0].arn
+  port              = var.p4_broker_config.container_port
+  protocol          = "TCP"
+
+  default_action {
+    type             = "forward"
+    target_group_arn = module.p4_broker[0].target_group_arn
+  }
+
+  #checkov:skip=CKV2_AWS_74: TCP listener does not support TLS ciphers
+  tags = merge(var.tags, {
+    TrafficSource      = (var.shared_network_load_balancer_name != null ? var.shared_network_load_balancer_name : "${var.project_prefix}-perforce-shared-nlb")
+    TrafficDestination = "${var.project_prefix}-${var.p4_broker_config.name}-service"
+  })
+}


modules/perforce/lb.tf

+resource "aws_lb_listener" "perforce_broker" {
+  count             = var.p4_broker_config != null && var.create_shared_network_load_balancer ? 1 : 0
+  load_balancer_arn = aws_lb.perforce[0].arn
+  port              = var.p4_broker_config.container_port
+  protocol          = "TCP"
+
+  default_action {
+    type             = "forward"
+    target_group_arn = module.p4_broker[0].target_group_arn
+  }
+
+  #checkov:skip=CKV2_AWS_74: TCP listener does not support TLS ciphers
+  tags = merge(var.tags, {
+    TrafficSource      = (var.shared_network_load_balancer_name != null ? var.shared_network_load_balancer_name : "${var.project_prefix}-perforce-shared-nlb")
+    TrafficDestination = "${var.project_prefix}-${var.p4_broker_config.name}-service"
+  })
+}


modules/perforce/lb.tf

+resource "aws_lb_listener" "perforce_broker" {
+  count             = var.p4_broker_config != null && var.create_shared_network_load_balancer ? 1 : 0
+  load_balancer_arn = aws_lb.perforce[0].arn
+  port              = var.p4_broker_config.container_port
+  protocol          = "TCP"
+
+  default_action {
+    type             = "forward"
+    target_group_arn = module.p4_broker[0].target_group_arn
+  }
+
+  #checkov:skip=CKV2_AWS_74: TCP listener does not support TLS ciphers
+  tags = merge(var.tags, {
+    TrafficSource      = (var.shared_network_load_balancer_name != null ? var.shared_network_load_balancer_name : "${var.project_prefix}-perforce-shared-nlb")
+    TrafficDestination = "${var.project_prefix}-${var.p4_broker_config.name}-service"
+  })
+}


modules/perforce/modules/p4-broker/config.tf

+resource "aws_s3_bucket" "broker_config" {
+  bucket        = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  force_destroy = true
+
+  #checkov:skip=CKV_AWS_21: Versioning not required for broker config
+  #checkov:skip=CKV_AWS_144: Cross-region replication not required
+  #checkov:skip=CKV_AWS_145: KMS encryption with CMK not currently supported
+  #checkov:skip=CKV_AWS_18: S3 access logs not necessary
+  #checkov:skip=CKV2_AWS_62: Event notifications not necessary
+  #checkov:skip=CKV2_AWS_61: Lifecycle configuration not necessary for config bucket
+  #checkov:skip=CKV2_AWS_6: S3 Buckets have public access blocked by default
+
+  tags = merge(var.tags, {
+    Name = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  })
+}


modules/perforce/modules/p4-broker/config.tf

+resource "aws_s3_bucket" "broker_config" {
+  bucket        = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  force_destroy = true
+
+  #checkov:skip=CKV_AWS_21: Versioning not required for broker config
+  #checkov:skip=CKV_AWS_144: Cross-region replication not required
+  #checkov:skip=CKV_AWS_145: KMS encryption with CMK not currently supported
+  #checkov:skip=CKV_AWS_18: S3 access logs not necessary
+  #checkov:skip=CKV2_AWS_62: Event notifications not necessary
+  #checkov:skip=CKV2_AWS_61: Lifecycle configuration not necessary for config bucket
+  #checkov:skip=CKV2_AWS_6: S3 Buckets have public access blocked by default
+
+  tags = merge(var.tags, {
+    Name = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  })
+}


modules/perforce/modules/p4-broker/config.tf

+resource "aws_s3_bucket" "broker_config" {
+  bucket        = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  force_destroy = true
+
+  #checkov:skip=CKV_AWS_21: Versioning not required for broker config
+  #checkov:skip=CKV_AWS_144: Cross-region replication not required
+  #checkov:skip=CKV_AWS_145: KMS encryption with CMK not currently supported
+  #checkov:skip=CKV_AWS_18: S3 access logs not necessary
+  #checkov:skip=CKV2_AWS_62: Event notifications not necessary
+  #checkov:skip=CKV2_AWS_61: Lifecycle configuration not necessary for config bucket
+  #checkov:skip=CKV2_AWS_6: S3 Buckets have public access blocked by default
+
+  tags = merge(var.tags, {
+    Name = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  })
+}


modules/perforce/modules/p4-broker/config.tf

+resource "aws_s3_bucket" "broker_config" {
+  bucket        = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  force_destroy = true
+
+  #checkov:skip=CKV_AWS_21: Versioning not required for broker config
+  #checkov:skip=CKV_AWS_144: Cross-region replication not required
+  #checkov:skip=CKV_AWS_145: KMS encryption with CMK not currently supported
+  #checkov:skip=CKV_AWS_18: S3 access logs not necessary
+  #checkov:skip=CKV2_AWS_62: Event notifications not necessary
+  #checkov:skip=CKV2_AWS_61: Lifecycle configuration not necessary for config bucket
+  #checkov:skip=CKV2_AWS_6: S3 Buckets have public access blocked by default
+
+  tags = merge(var.tags, {
+    Name = "${local.name_prefix}-config-${random_string.broker_config.result}"
+  })
+}


modules/perforce/modules/p4-broker/main.tf

+resource "aws_cloudwatch_log_group" "log_group" {
+  #checkov:skip=CKV_AWS_158: KMS Encryption disabled by default
+  name              = "${local.name_prefix}-log-group"
+  retention_in_days = var.cloudwatch_log_retention_in_days
+  tags = merge(var.tags, {
+    Name = "${local.name_prefix}-log-group"
+  })
+}


modules/perforce/modules/p4-broker/main.tf

+resource "aws_cloudwatch_log_group" "log_group" {
+  #checkov:skip=CKV_AWS_158: KMS Encryption disabled by default
+  name              = "${local.name_prefix}-log-group"
+  retention_in_days = var.cloudwatch_log_retention_in_days
+  tags = merge(var.tags, {
+    Name = "${local.name_prefix}-log-group"
+  })
+}


modules/perforce/modules/p4-broker/main.tf

+resource "aws_cloudwatch_log_group" "log_group" {
+  #checkov:skip=CKV_AWS_158: KMS Encryption disabled by default
+  name              = "${local.name_prefix}-log-group"
+  retention_in_days = var.cloudwatch_log_retention_in_days
+  tags = merge(var.tags, {
+    Name = "${local.name_prefix}-log-group"
+  })
+}


gabebatista

lgtm

Rename documentation index files for consistency: - docs/assets/dockerfiles.md → docs/assets/docker/README.md - docs/assets/packer/index.md → docs/assets/packer/README.md - Update mkdocs.yml nav paths to match new locations

Update markdown links in docs/getting-started.md and docs/assets/index.md to reference the new file paths (packer/README.md, docker/README.md) instead of the old names (packer/index.md, dockerfiles.md).

Test restructuring: - Flatten test directory (remove unit/ and integration/ subdirectories) - Delete integration tests (require AWS credentials and SSM parameters) - Replace jsonencode() in mock_data with raw JSON strings - Add explicit disabled flags for NLB/ALB/R53 in test variables - Fix FQDN/zone name mismatch in full_stack test data - Fix assertion resource names (lb_access_logs → shared_lb_access_logs_bucket) - Simplify tests/README.md Module bug fixes: - versions.tf: add netapp-ontap provider (required by p4-server submodule) - variables.tf: add null guards to all p4_server_config validation rules - variables.tf: fix inverted shared_lb_access_logs_bucket validation - variables.tf: fix route53_private_hosted_zone_name error message - lb.tf: require both NLB and ALB for NLB→ALB resources - sg.tf: require ALB for NLB→ALB egress rule - main.tf: add try() fallback for p4_code_review secret ARN references - outputs.tf: add null guard for p4_server_lambda_link_name

…er-pr

Add P4 Broker (Perforce Helix Broker) as an ECS Fargate service for TCP-level command filtering and routing between clients and P4 Server. New files: - Dockerfile and README for the P4 Broker container image - Complete p4-broker ECS submodule (12 files): versions, locals, data, variables, config template, config, IAM, security groups, NLB target group, ECS service, outputs, and README - Unit test (03_p4_broker.tftest.hcl) with 6 test scenarios Root module integration: - variables.tf: add p4_broker_config variable - locals.tf: include broker in shared ECS cluster condition - main.tf: add module "p4_broker" block - lb.tf: add NLB TCP listener for broker traffic - sg.tf: add 4 inter-service SG rules (NLB↔Broker, Broker→P4 Server) - outputs.tf: add broker outputs Documentation: - mkdocs.yml: nest Dockerfiles section, add broker entries - README.md: add P4 Broker to features list

hwkiem requested a review from a team as a code owner February 14, 2026 00:36

hwkiem requested a review from jedobi February 14, 2026 00:36

github-project-automation bot added this to Cloud Game Development Toolkit Feb 14, 2026

github-advanced-security bot found potential problems Feb 14, 2026

View reviewed changes

hwkiem mentioned this pull request Feb 14, 2026

fix(ci): include .tftest.hcl files in terraform fmt pre-commit hook #872

Open

1 task

gabebatista approved these changes Feb 16, 2026

View reviewed changes

hwkiem added 7 commits February 17, 2026 10:11

refactor(docs): rename section index files to README.md

5916d07

Rename documentation index files for consistency: - docs/assets/dockerfiles.md → docs/assets/docker/README.md - docs/assets/packer/index.md → docs/assets/packer/README.md - Update mkdocs.yml nav paths to match new locations

fix(docs): update internal links to match renamed files

b00e895

Update markdown links in docs/getting-started.md and docs/assets/index.md to reference the new file paths (packer/README.md, docker/README.md) instead of the old names (packer/index.md, dockerfiles.md).

style(perforce): fix terraform fmt on test files

af771a4

Merge branch 'refactor/docs-rename-section-indexes' into feat/p4-brok…

eb8bfcb

…er-pr

style(perforce): fix terraform fmt on broker test file

4e0d8d2

hwkiem force-pushed the feat/p4-broker-pr branch from d68d8db to 4e0d8d2 Compare February 17, 2026 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

feat(perforce): add P4 Broker ECS submodule#871

feat(perforce): add P4 Broker ECS submodule#871
hwkiem wants to merge 7 commits intomainfrom
feat/p4-broker-pr

hwkiem commented Feb 14, 2026

Uh oh!

github-actions bot commented Feb 14, 2026 •

edited

Loading

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

gabebatista left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

hwkiem commented Feb 14, 2026

Summary

Dependencies

Architecture

Test plan

Uh oh!

github-actions bot commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📚 Documentation Preview

🔒 Maintainer Action Required

Build Information

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

gabebatista left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Feb 14, 2026 •

edited

Loading