Skip to content

Added NAT Gateway IDs to the Subnets stats map #225

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Added NAT Gateway IDs to the Subnets stats map #225

wants to merge 3 commits into from

Conversation

@j4zzcat
Copy link

@j4zzcat j4zzcat commented Aug 22, 2025 •
edited
Loading

There are scenarios where downstream components require the NAT Gateway IDs, e.g., when wiring the routing for a network firewall. Exposing them makes the subnet_stats_map more complete and has little risk.

Changes

  • Added a local variables az_public_nat_gateway_ids_map that hold a map of NAT gatway IDs by AZ. For example:
az_public_nat_gateway_ids_map = {
  "us-east-1a" = ["nat-1", "nat-2"]
  "us-east-1b" = ["nat-3"]
}
  • This is then added to the existing subnet stat maps, named_private_subnets_stats_map and named_public_subnets_stats_map.

Note: based on @aknysh code.

@j4zzcat j4zzcat requested review from a team as code owners August 22, 2025 22:45
@j4zzcat j4zzcat requested review from Gowiem and johncblandii August 22, 2025 22:45
@mergify mergify bot added the triage Needs triage label Aug 22, 2025
@aknysh
Copy link
Member

aknysh commented Nov 2, 2025

/terratest

aknysh added a commit that referenced this pull request Nov 2, 2025
@aknysh @claude
Add NAT Gateway IDs to subnet stats maps
45bc6f1 
Incorporate feature from PR #225 to expose NAT Gateway IDs in the subnet stats outputs.

- Add `nat_gateway_id` field to `named_private_subnets_stats_map` (maps to the NAT Gateway that the private subnet routes to for egress)
- Add `nat_gateway_id` field to `named_public_subnets_stats_map` (maps to the NAT Gateway in that public subnet, if any)
- Create helper locals to correctly map subnets to NAT Gateways
- Update output descriptions to reflect the new fourth field

This makes the subnet stats more complete and enables downstream components to reference NAT Gateway IDs when needed (e.g., network firewall routing configurations).

Implementation correctly handles our new NAT placement features:
- Works with index-based NAT placement (`nat_gateway_public_subnet_indices`)
- Works with name-based NAT placement (`nat_gateway_public_subnet_names`)
- Private subnets correctly map to the NAT they route to (using existing `private_route_table_to_nat_map`)
- Public subnets correctly identify which ones contain NAT Gateways

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
aknysh added a commit that referenced this pull request Nov 2, 2025
@aknysh @claude
Fix AWS Provider v5+ compatibility for EIP resource
2b10416 
Update deprecated `vpc = true` to `domain = "vpc"` in aws_eip resource.

The `vpc` argument was deprecated in AWS Provider v5.0 and removed completely.
The new syntax uses `domain = "vpc"` instead.

This fixes test failures where Terraform validation fails with:
"Error: Unsupported argument - An argument named 'vpc' is not expected here"

Fixes the same issue that caused PR #225 tests to fail.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
aknysh added a commit that referenced this pull request Nov 2, 2025
@aknysh @claude
Separate Public/Private Subnet Configuration and Enhance NAT Gateway …
6f61890 
…Placement (#226)

* updates

* updates

* updates

* updates

* updates

* updates

* updates

* updates

* updates

* updates

* Add NAT Gateway IDs to subnet stats maps

Incorporate feature from PR #225 to expose NAT Gateway IDs in the subnet stats outputs.

- Add `nat_gateway_id` field to `named_private_subnets_stats_map` (maps to the NAT Gateway that the private subnet routes to for egress)
- Add `nat_gateway_id` field to `named_public_subnets_stats_map` (maps to the NAT Gateway in that public subnet, if any)
- Create helper locals to correctly map subnets to NAT Gateways
- Update output descriptions to reflect the new fourth field

This makes the subnet stats more complete and enables downstream components to reference NAT Gateway IDs when needed (e.g., network firewall routing configurations).

Implementation correctly handles our new NAT placement features:
- Works with index-based NAT placement (`nat_gateway_public_subnet_indices`)
- Works with name-based NAT placement (`nat_gateway_public_subnet_names`)
- Private subnets correctly map to the NAT they route to (using existing `private_route_table_to_nat_map`)
- Public subnets correctly identify which ones contain NAT Gateways

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Fix AWS Provider v5+ compatibility for EIP resource

Update deprecated `vpc = true` to `domain = "vpc"` in aws_eip resource.

The `vpc` argument was deprecated in AWS Provider v5.0 and removed completely.
The new syntax uses `domain = "vpc"` instead.

This fixes test failures where Terraform validation fails with:
"Error: Unsupported argument - An argument named 'vpc' is not expected here"

Fixes the same issue that caused PR #225 tests to fail.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Update VPC module to v3.0.0 in all examples

Update cloudposse/vpc/aws module from v2.0.0 to v3.0.0 across all example configurations.

Changes:
- examples/complete/main.tf
- examples/existing-ips/main.tf
- examples/multiple-subnets-per-az/main.tf
- examples/nacls/main.tf
- examples/redundant-nat-gateways/main.tf
- examples/separate-public-private-subnets/main.tf

v3.0.0 includes:
- Updates to Terraform AWS provider v6 compatibility
- Enhanced VPC endpoint support
- Bug fixes and improvements

This ensures all examples use the latest stable VPC module version with AWS Provider v6 support.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Update documentation for NAT Gateway ID exposure feature

Enhanced PRD and README with comprehensive documentation of the NAT Gateway
ID mapping feature and AWS Provider compatibility updates.

Changes:

**PRD Updates (docs/prd/separate-public-private-subnets-and-nat-placement.md):**
- Updated executive summary to include 4th feature
- Added Feature #4: NAT Gateway ID Exposure in Subnet Stats
  - Detailed problem, solution, and implementation
  - Example output structures for both private and public subnet stats
  - Benefits and use cases (network firewall routing)
- Added Bug Fix #3: AWS Provider v5+ Compatibility
  - Documents EIP `vpc = true` → `domain = "vpc"` migration
  - Notes VPC module updates to v3.0.0

**README.yaml Updates:**
- Added new section "NAT Gateway ID References in Outputs"
- Detailed explanation of `named_private_subnets_stats_map` structure (4 fields)
- Detailed explanation of `named_public_subnets_stats_map` structure (4 fields)
- Real-world use case example: network firewall routing configuration
- Shows how to extract NAT Gateway IDs from stats maps

**README.md (Generated):**
- Auto-generated from README.yaml using `atmos readme`
- Includes all new documentation sections
- Updated output descriptions to reflect 4-field structure

The NAT Gateway ID mapping enables downstream components (network firewalls,
routing policies) to reference NAT Gateways associated with subnets without
manual mapping or additional data sources.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Update AWS Provider version requirements for consistency

Standardize AWS Provider version constraints across the module and all examples
to ensure compatibility with dependencies and features.

Changes:

**Main Module (versions.tf):**
- Updated from `>= 3.71.0` to `>= 5.0`
- Rationale: AWS Provider v5 introduced `domain = "vpc"` for EIP resources
- Module is compatible with v5+ (main code uses neither `vpc` nor `domain` attributes for broad compatibility)
- Documented AWS Provider v5+ compatibility in PRD

**All Examples (examples/*/versions.tf):**
- Updated from `>= 3.71.0` or `>= 4.0` to `>= 6.0`
- Rationale: All examples use cloudposse/vpc/aws v3.0.0 which requires AWS Provider v6
- VPC module v3.0.0 released September 12, 2025 with AWS Provider v6 support
- Examples affected:
  - examples/complete/versions.tf
  - examples/existing-ips/versions.tf (also uses `domain = "vpc"`)
  - examples/multiple-subnets-per-az/versions.tf
  - examples/nacls/versions.tf
  - examples/redundant-nat-gateways/versions.tf
  - examples/separate-public-private-subnets/versions.tf

**Version Matrix:**
- Module core: AWS Provider >= 5.0 (broadest compatibility)
- Examples: AWS Provider >= 6.0 (required by VPC module dependency)

This ensures all examples work correctly with their dependencies and prevents
version conflict errors during terraform init.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Update Go to 1.25 and all dependencies to latest versions

Update test dependencies to their latest stable releases for improved
security, performance, and compatibility.

**Go Version:**
- Updated from Go 1.24 to Go 1.25
- Updated toolchain from go1.24.0 to go1.25.0

**Major Dependency Updates:**

**Testing Framework:**
- terratest: v0.41.23 → v0.52.0
- testify: v1.8.2 → v1.11.1

**Kubernetes:**
- k8s.io/apimachinery: v0.20.6 → v0.34.0
- k8s.io/api: v0.20.6 → v0.34.0
- k8s.io/client-go: v0.20.6 → v0.34.0
- k8s.io/klog/v2: v2.4.0 → v2.130.1

**AWS SDK v2 (new):**
- Migrated to AWS SDK v2 packages
- Added support for multiple AWS services (EC2, S3, IAM, RDS, etc.)

**HashiCorp:**
- go-getter: v1.7.1 → v1.8.3 + v2.2.3
- hcl/v2: v2.9.1 → v2.24.0
- terraform-json: v0.13.0 → v0.27.2
- go-version: v1.6.0 → v1.7.0

**Google Cloud:**
- Updated all google.golang.org packages to latest versions
- grpc: v1.56.3 → v1.76.0
- protobuf: v1.30.0 → v1.36.10

**golang.org/x packages:**
- crypto: v0.1.0 → v0.43.0
- net: v0.9.0 → v0.46.0
- oauth2: v0.7.0 → v0.32.0
- sys: v0.7.0 → v0.37.0
- text: v0.9.0 → v0.30.0
- tools: v0.6.0 → v0.38.0

**Other Notable Updates:**
- klauspost/compress: v1.15.11 → v1.18.1
- mattn/go-zglob: v0.0.2 → v0.6
- ulikunitz/xz: v0.5.10 → v0.5.15
- zclconf/go-cty: v1.9.1 → v1.17.0

All dependencies updated using `go get -u ./...` and cleaned with `go mod tidy`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Fix tflint warning: remove unused local.subnet_az_count

Remove unused local variable `subnet_az_count` that was causing tflint failure.

This variable was a remnant from the refactoring where we separated public
and private subnet counts. It was declared but never used anywhere in the code.

The variable was calculating `max(local.public_subnet_az_count, local.private_subnet_az_count)`
but this value is not needed since we now handle public and private subnets
independently using:
- `local.public_subnet_az_count` - for public subnet operations
- `local.private_subnet_az_count` - for private subnet operations
- `local.vpc_az_count` - for NAT gateways (one per AZ)

Fixes tflint warning:
  main.tf:86:3: warning: local.subnet_az_count is declared but not used

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* Fix test failure: Add missing nat_gateway_public_subnet_indices variable to examples

Add the nat_gateway_public_subnet_indices variable to both new example configurations
to fix test failures where the test was trying to pass this variable but it wasn't
declared in the example's variables.tf.

**Test Failure Fixed**:
TestExamplesSeparatePublicPrivateSubnetsWithIndices was failing with:
"Error: Value for undeclared variable - A variable named 'nat_gateway_public_subnet_indices'
was assigned on the command line, but the root module does not declare a variable of that name."

**Changes**:

1. examples/separate-public-private-subnets/:
   - Added nat_gateway_public_subnet_indices variable with default [0]
   - Made nat_gateway_public_subnet_names nullable
   - Passed nat_gateway_public_subnet_indices to module in main.tf

2. examples/redundant-nat-gateways/:
   - Added nat_gateway_public_subnet_indices variable with default [0, 1]
   - Made nat_gateway_public_subnet_names nullable
   - Passed nat_gateway_public_subnet_indices to module in main.tf

Both examples now support passing either nat_gateway_public_subnet_indices OR
nat_gateway_public_subnet_names, enabling the test to override with index-based
placement when needed.

Fixes test at examples_separate_public_private_subnets_test.go:169

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

* updates

* updates

---------

Co-authored-by: Claude <[email protected]>
@mergify
Copy link

mergify bot commented Nov 2, 2025

💥 This pull request now has conflicts. Could you fix it @j4zzcat? 🙏

@mergify mergify bot added the conflict This PR has conflicts label Nov 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johncblandii johncblandii Awaiting requested review from johncblandii johncblandii is a code owner automatically assigned from cloudposse/contributors

@Gowiem Gowiem Awaiting requested review from Gowiem Gowiem is a code owner automatically assigned from cloudposse/contributors

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

conflict This PR has conflicts triage Needs triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@j4zzcat @aknysh