cherry-pick: dual-mode TLS + ACK ACM controller from fabric-fixes by sanmesh-kakade · Pull Request #525 · Facets-cloud/facets-modules

sanmesh-kakade · 2026-03-19T06:59:08Z

Summary

Cherry-pick of PR #524 (and related ACK ACM controller commits) from fabric-fixes to master.

Included commits

Restrict ACK ACM controller IAM policy to read-only access
Add acm:ExportCertificate to IAM policy for adopted cert export
feat: dual-mode TLS for NGINX Gateway Fabric AWS module
feat: dual-mode TLS, DNS-01/ACM guard, remove branch ref, update README
fix: deduplicate ACM ARNs in NLB ssl-cert annotation
remove export permission

Dual-mode TLS

Automatically selects TLS termination point based on configuration:

Condition	Mode	TLS Termination
No ACM ARNs	cert-manager	Gateway (HTTPS listeners)
ACM ARNs + ACK controller	ACK	Gateway (HTTPS listeners, ACK-exported secrets)
ACM ARNs + no ACK controller	ACM mode	NLB (single HTTP listener on 443)

See PR #524 for full details.

Test plan

Verify existing cert-manager deployments unchanged
Verify ACM mode with NLB TLS termination
Verify ACK path with ACK controller
Verify no duplicate Route53 records

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

New Features
- Added ACK ACM Controller module for managing AWS Certificate Manager certificates on EKS
- Restructured NGINX Gateway Fabric into cloud-specific implementations for AWS, Azure, and GCP
- Added DNS-01 wildcard certificate support for cloud providers
- Added AWS NLB-terminated TLS and ACK ACM integration options
Deprecated
- Deprecated generic NGINX Gateway Fabric Legacy module in favor of cloud-specific variants

coderabbitai · 2026-03-19T06:59:23Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 39e37b94-4073-4b07-a89c-e0f4849e072d

📥 Commits

Reviewing files that changed from the base of the PR and between 941568b and a537c8b.

📒 Files selected for processing (26)

modules/ack_acm_controller/legacy/1.0/README.md
modules/ack_acm_controller/legacy/1.0/facets.yaml
modules/ack_acm_controller/legacy/1.0/main.tf
modules/ack_acm_controller/legacy/1.0/outputs.tf
modules/ack_acm_controller/legacy/1.0/variables.tf
modules/ingress/nginx_gateway_fabric_legacy/1.0/README.md
modules/ingress/nginx_gateway_fabric_legacy/1.0/charts/nginx-gateway-fabric-2.3.0.tgz
modules/ingress/nginx_gateway_fabric_legacy/1.0/charts/nginx-gateway-fabric-2.4.1.tgz
modules/ingress/nginx_gateway_fabric_legacy/1.0/main.tf
modules/ingress/nginx_gateway_fabric_legacy/1.0/outputs.tf
modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/README.md
modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/facets.yaml
modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/main.tf
modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/outputs.tf
modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/variables.tf
modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/README.md
modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/facets.yaml
modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/main.tf
modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/outputs.tf
modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/variables.tf
modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/README.md
modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/facets.yaml
modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/main.tf
modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/outputs.tf
modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/variables.tf
outputs/ack_acm_controller/output.facets.yaml

Walkthrough

Adds an ACK ACM Controller module and cloud-specific legacy wrappers (AWS, Azure, GCP) around nginx_gateway_fabric, introducing ACM integration, DNS-01 wildcard automation, certificate bootstrapping, and Route53 record management while removing the original nginx_gateway_fabric_legacy base implementation.

Changes

Cohort / File(s)	Summary
ACK ACM Controller Module `modules/ack_acm_controller/legacy/1.0/README.md`, `modules/ack_acm_controller/legacy/1.0/facets.yaml`, `modules/ack_acm_controller/legacy/1.0/main.tf`, `modules/ack_acm_controller/legacy/1.0/variables.tf`, `modules/ack_acm_controller/legacy/1.0/outputs.tf`, `outputs/ack_acm_controller/output.facets.yaml`	New Terraform module deploying ACK ACM Controller to EKS via Helm with IRSA (IAM policy + role), exposes outputs (namespace, release_name, chart_version, role_arn, helm_release_id), and provides documentation and facet schema.
Removed: NGINX Gateway Fabric Legacy Base `modules/ingress/nginx_gateway_fabric_legacy/1.0/README.md`, `modules/ingress/nginx_gateway_fabric_legacy/1.0/main.tf`, `modules/ingress/nginx_gateway_fabric_legacy/1.0/outputs.tf`	Deleted legacy base implementation and docs (removed Helm deployment, Gateway API resource generation, cert-manager bootstrapping, Route53 records, basic-auth generation, and outputs).
NGINX Gateway Fabric AWS Wrapper `modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/README.md`, `modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/facets.yaml`, `modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/main.tf`, `modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/outputs.tf`, `modules/ingress/nginx_gateway_fabric_legacy_aws/1.0/variables.tf`	New AWS-specific wrapper adding three TLS modes (cert-manager, ACK ACM export-to-secret, NLB-terminated ACM), domain rewriting, conditional ack_acm controller integration, DNS-01 wildcard support, cert resources and Route53 record creation, plus facet/schema and outputs.
NGINX Gateway Fabric Azure Wrapper `modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/README.md`, `modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/facets.yaml`, `modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/main.tf`, `modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/outputs.tf`, `modules/ingress/nginx_gateway_fabric_legacy_azure/1.0/variables.tf`	New Azure-specific wrapper implementing optional DNS-01 wildcard issuance with cert-manager, bootstrap TLS secrets, domain rewriting, Route53 record management, facet/schema, and outputs.
NGINX Gateway Fabric GCP Wrapper `modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/README.md`, `modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/facets.yaml`, `modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/main.tf`, `modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/outputs.tf`, `modules/ingress/nginx_gateway_fabric_legacy_gcp/1.0/variables.tf`	New GCP-specific wrapper adding DNS-01 wildcard automation, domain rewriting, bootstrap TLS material and cert-manager Certificate CRDs, optional internal LB annotations, Route53 records, facet/schema, and outputs.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Terraform
  participant EKS as Kubernetes (EKS)
  participant AWS as AWS ACM / IAM
  participant Route53

  rect rgba(0,128,0,0.5)
  User->>Terraform: apply modules/ack_acm_controller
  end

  Terraform->>AWS: create aws_iam_policy
  Terraform->>AWS: create IRSA role (module.irsa)
  Terraform->>EKS: create ServiceAccount annotated with role ARN
  Terraform->>EKS: helm_release ack-acm-controller (install chart)
  EKS->>AWS: ACK controller uses IRSA to call ACM APIs
  AWS-->>EKS: ACM issues certificates / ACK exports TLS secrets
  Terraform->>Route53: (when DNS-01 enabled) create base and wildcard records

Possibly related PRs

feat: Add nginx gateway fabric legacy ingress module #508 — touches nginx_gateway_fabric_legacy-area (Gateway API and NGINX Gateway Fabric) and likely overlaps with this refactor.

Suggested Reviewers

rr0hit
anshulsao
vishnukv-facets
ishaankalra
pramodh-ayyappan
unni-facets

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch cherry-pick/dual-mode-tls-aws

📝 Coding Plan

Generate coding plan for human review comments

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Trivy (0.69.3)

Trivy execution failed: Unknown error

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…ti-domain outputs - Azure: always set load-balancer-internal annotation (true/false) - OVH: add loadbalancer.ovhcloud.com/flavor annotation - Migrate kubernetes_secret to kubernetes_secret_v1 (deprecated) - Wildcard hostname for certificate_reference domains (*.domain) - ParentRef: subdomain routes use wildcard domain listener when cert_ref present - Filter additional_hostname_configs to exclude cert_ref covered subdomains - cross_namespace_backends: use distinct() to avoid duplicate key error - ClusterIssuer count guard: use certmanager_managed_domains length - Output interfaces: multi-domain rule × domain cross-product - tls_secret: map of domain keys to cert secret names - Add missing outputs: lb_record_value, record_type, name, base_domain, etc. - Remove ACM-specific sections from README (ACM will be in cloud-specific module) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…tes via ACK Deploys the AWS ACK ACM Controller on EKS via Helm with IRSA. Enables managing ACM certificates as Kubernetes CRDs and exporting them to K8s TLS secrets for use with Gateway API listeners. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

… cert support AWS-specific wrapper for nginx_gateway_fabric with NLB, Proxy Protocol v2, ACM certificate integration, and DNS-01 wildcard certificate support. When use_dns01 is enabled, all domain listeners switch to wildcard hostnames (*.domain) and certificates are issued via a pre-existing ClusterIssuer (default: gts-production). Base domain is also covered by DNS-01 when active. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

When use_dns01=true, we set disable_base_domain=true internally to take over base domain management, which causes the utility module to skip Route53 record creation. Create the records in the AWS wrapper instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ove old legacy module - Add nginx_gateway_fabric_legacy_gcp and nginx_gateway_fabric_legacy_azure modules with DNS-01 wildcard cert support (no ACM) - Pass var.environment.default_tolerations to utility module via merged inputs in all three wrapper modules (AWS, GCP, Azure) - Update migration script and docs to target nginx_gateway_fabric_legacy_aws - Remove standalone nginx_gateway_fabric_legacy module (replaced by cloud-specific wrappers) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove permissions that allow creating, importing, deleting, renewing, or modifying ACM certificates. The controller now only needs to adopt and sync existing certificates via AdoptedResource. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Required for exportTo to work with adopted certificates. This permission only reads existing cert data (cert body + private key), does not create new certificates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Adds ACM mode detection: when domains have ACM ARNs as certificate_reference and no ACK controller is available, TLS terminates at the NLB instead of the Gateway pod. - Detects ACM mode via certificate_reference + ACK controller presence - Adds NLB ssl-cert/ssl-ports annotations in ACM mode - Passes var.instance as-is in ACM mode (no domain rewriting) - Gates ACK Certificate CRD resources on ACK controller availability - Passes external_tls_termination=true to base utility module Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Remove temporary branch ref from utility module source - Disable DNS-01 when ACM mode is active (incompatible) - Fix inconsistent conditional types in modified_instance - Update README with dual-mode TLS architecture and examples Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Use distinct() to avoid duplicate ARNs when multiple domains share the same ACM wildcard certificate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sanmesh-kakade requested review from anshulsao, ishaankalra, pramodh-ayyappan, rr0hit, unni-facets and vishnukv-facets as code owners March 19, 2026 06:59

sanmesh-kakade force-pushed the cherry-pick/dual-mode-tls-aws branch 2 times, most recently from f7976bd to 2e4909c Compare March 19, 2026 07:11

sanmesh-kakade and others added 13 commits March 19, 2026 12:47

fix: rename Route53 resources to match utility module naming convention

b35bff2

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix: remove leftover migration files from deleted legacy module

3a7060f

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix: deduplicate ACM ARNs in NLB ssl-cert annotation

1775c73

Use distinct() to avoid duplicate ARNs when multiple domains share the same ACM wildcard certificate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

remove export permission

a537c8b

sanmesh-kakade force-pushed the cherry-pick/dual-mode-tls-aws branch from 73e5b85 to a537c8b Compare March 19, 2026 07:18

unni-facets approved these changes Mar 19, 2026

View reviewed changes

sanmesh-kakade merged commit 368c742 into master Mar 19, 2026
1 check failed

sanmesh-kakade deleted the cherry-pick/dual-mode-tls-aws branch March 19, 2026 07:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cherry-pick: dual-mode TLS + ACK ACM controller from fabric-fixes#525

cherry-pick: dual-mode TLS + ACK ACM controller from fabric-fixes#525
sanmesh-kakade merged 13 commits into
masterfrom
cherry-pick/dual-mode-tls-aws

sanmesh-kakade commented Mar 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 19, 2026 •

edited

Loading

Review failed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sanmesh-kakade commented Mar 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Included commits

Dual-mode TLS

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Possibly related PRs

Suggested Reviewers

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sanmesh-kakade commented Mar 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 19, 2026 •

edited

Loading