wip(dns): DNS-redirect module capture (needs integration) - #1196
Open
mdheller wants to merge 3 commits into
Open
wip(dns): DNS-redirect module capture (needs integration)#1196mdheller wants to merge 3 commits into
mdheller wants to merge 3 commits into
Conversation
…mecheap NS delegation Adds cloud-agnostic dns-records module and per-cloud zone modules (AWS Route53, GCP Cloud DNS) for the portfolio NS-delegation plan; registrar-namecheap gains NS-delegation variables. Not applied — feature branch only.
…mitters Completes the portability refactor (new modules were landed in b61443e but the env still referenced the old dns-zone): - env now computes records via dns-records (cloud-agnostic) and renders through the emitter selected by var.dns_cloud (gcp default | aws), fail-closed on unknown clouds. - auto cross-domain DMARC _report._dmarc authorizations on the rua domain. - registrar delegation now fail-closed (refuses canonical/redirect with no records). - record_manifest output = auditable pre-apply record set. - remove superseded modules/dns-zone. Verified: fmt + init + validate (google+aws+namecheap) pass; standalone apply-test of dns-records confirms parked lockdown, mail observe-mode, apex-TXT coalescing, and the cross-domain report authorizations.
…ter integration Working-tree changes from feat/dns-redirect-service (7 tofu files + new web-redirect-gcp module) captured here so they are backed up upstream. Cherry-pick/merge into the feature branch when integrating.
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a cloud-agnostic DNS “record model” (dns-records) and pluggable per-cloud emitters (GCP Cloud DNS + AWS Route53), plus a new GCP HTTPS redirect load balancer module intended to support redirect-role domains managed from envs/dns/domains.yaml.
Changes:
- Add
dns-recordsmodule that normalizes per-domain DNS records (baseline + optional redirects + app records) for consumption by cloud-specific emitters. - Add new emitter modules:
dns-zone-gcp(Cloud DNS) anddns-zone-aws(Route53), and rewireinfra/tofu/envs/dnsto select viavar.dns_cloud. - Add
web-redirect-gcpmodule and hook it intoenvs/dnsbehindenable_redirects, plus extend registrar delegation guards.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/tofu/modules/web-redirect-gcp/versions.tf | Declares Terraform/provider version constraints for the redirect LB module. |
| infra/tofu/modules/web-redirect-gcp/variables.tf | Defines inputs for the redirect LB (project, host mapping, cert domains). |
| infra/tofu/modules/web-redirect-gcp/outputs.tf | Exposes redirect LB IP for DNS wiring. |
| infra/tofu/modules/web-redirect-gcp/main.tf | Implements the GCP external HTTPS LB URL-map redirect behavior. |
| infra/tofu/modules/registrar-namecheap/variables.tf | Adds delegation safety inputs (role, has_records). |
| infra/tofu/modules/registrar-namecheap/main.tf | Adds a fail-closed precondition to prevent delegating “empty” canonical/redirect domains. |
| infra/tofu/modules/dns-zone/variables.tf | Removes legacy monolithic GCP DNS-zone module (replaced by records+emitters). |
| infra/tofu/modules/dns-zone/main.tf | Removes legacy monolithic GCP DNS-zone module implementation. |
| infra/tofu/modules/dns-zone-gcp/versions.tf | Adds provider/version constraints for the GCP emitter module. |
| infra/tofu/modules/dns-zone-gcp/variables.tf | Defines emitter inputs (zone metadata + normalized records). |
| infra/tofu/modules/dns-zone-gcp/outputs.tf | Adjusts outputs exposed by the GCP emitter. |
| infra/tofu/modules/dns-zone-gcp/main.tf | Renders normalized records onto Google Cloud DNS. |
| infra/tofu/modules/dns-zone-aws/versions.tf | Adds provider/version constraints for the AWS emitter module. |
| infra/tofu/modules/dns-zone-aws/variables.tf | Defines emitter inputs (zone metadata + normalized records). |
| infra/tofu/modules/dns-zone-aws/outputs.tf | Exposes Route53 nameservers + hosted zone id. |
| infra/tofu/modules/dns-zone-aws/main.tf | Renders normalized records onto Route53 (with DNSSEC fail-closed guard). |
| infra/tofu/modules/dns-records/versions.tf | Establishes a provider-agnostic module boundary for normalized records. |
| infra/tofu/modules/dns-records/variables.tf | Defines the portable record model inputs and safety knobs. |
| infra/tofu/modules/dns-records/outputs.tf | Exposes normalized records + zone metadata and delegation guard signals. |
| infra/tofu/modules/dns-records/main.tf | Computes the baseline safety records + redirect/app record normalization. |
| infra/tofu/envs/dns/versions.tf | Adds AWS provider requirement alongside existing providers. |
| infra/tofu/envs/dns/variables.tf | Adds dns_cloud, AWS region, and redirect toggles; makes project optional. |
| infra/tofu/envs/dns/README.md | Documents the new cloud-portable architecture and audit outputs. |
| infra/tofu/envs/dns/outputs.tf | Changes outputs (nameservers via selected emitter; adds record_manifest + redirect_ip). |
| infra/tofu/envs/dns/main.tf | Rewires env to dns-records + selected emitter, adds optional redirect service, updates registrar wiring. |
| infra/tofu/envs/dns/domains.yaml | Adds per-domain redirect_to overrides for specific redirect domains. |
Suppressed comments (1)
infra/tofu/modules/dns-zone-aws/main.tf:26
module.records[*].records[*].nameis normalized as an FQDN with trailing dot (e.g. "www.example.com."), but Route53 record names should be provided without the trailing dot to avoid API/provider errors.
resource "aws_route53_record" "this" {
for_each = { for r in var.records : "${r.type} ${r.name}" => r }
zone_id = aws_route53_zone.this.zone_id
name = each.value.name
type = each.value.type
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+7
| resource "aws_route53_zone" "this" { | ||
| name = var.dns_name | ||
| tags = merge({ |
Comment on lines
11
to
+15
| variable "project" { | ||
| type = string | ||
| description = "GCP project that hosts the Cloud DNS managed zones." | ||
| default = "" | ||
| description = "GCP project hosting the Cloud DNS zones (required when dns_cloud=gcp)." | ||
| } |
Comment on lines
+13
to
+15
| provider "aws" { | ||
| region = var.aws_region | ||
| } |
Comment on lines
+1
to
+5
| # Global external Application LB that 301-redirects each host to its canonical target. | ||
| # GCP-specific by nature (an L7 redirect service); the redirect INTENT stays portable in | ||
| # domains.yaml and the A records flow through the agnostic dns-records model. An AWS | ||
| # equivalent (CloudFront function / S3 redirect) would be a sibling module with the same | ||
| # ip_address/output contract. |
Comment on lines
+17
to
+21
| variable "role" { | ||
| type = string | ||
| default = "reserved" | ||
| description = "Portfolio role of the domain, used by the fail-closed delegation guard." | ||
| } |
Member
Author
|
Superseded by #1198, which hand-extracts just the real redirect-feature content (8 files) onto current main — merging/rebasing this branch directly would have been destructive (273 files / 13,791 deletions vs. current main). Leaving this open as the historical record of the original capture rather than closing it silently. |
mdheller
added a commit
that referenced
this pull request
Aug 1, 2026
…provisions nothing by default) (#1198) * feat(infra/dns): redirect-role domains 301 through a GCP LB (opt-in) Lands the real content of #1196 (a WIP capture that got tangled with 45 commits of drift and would have deleted ~13k lines of unrelated, already-shipped work if merged/rebased directly -- hand-extracted just the 8 files that are this feature). #1196 stays open as the historical record; this supersedes it. - New web-redirect-gcp module: global HTTPS LB, managed cert, URL-map 301s, HTTP->HTTPS upgrade. No compute. GCP-specific by nature (an L7 redirect service); the redirect intent stays portable in domains.yaml, an AWS/other-cloud sibling would satisfy the same ip_address contract. - dns-records module: new redirect_ip input emits apex+www A records for role=redirect domains once a redirect service exists; new has_delegable_records output (app_records OR emitted redirect records) replaces has_app_records for the registrar fail-closed delegation check, so a redirect domain whose only record comes from the redirect IP isn't wrongly treated as empty. - domains.yaml: redirect_to on the 5 domains already marked role:redirect (socioslinux.org/sourceos.digital/.software/.systems -> sourceos.org, truthorbot.org -> truthorbot.net) -- consolidating domains already owned, not new claims. - enable_redirects defaults to false. Merging this provisions nothing -- no new GCP resources, no DNS changes, no cost. Flipping it on for real is a separate, deliberate follow-up. Verified: tofu validate clean on both the composed envs/dns config and the new module standalone; tofu fmt clean. * fix(infra/dns): www hosts fell through to the wrong redirect target Copilot review on #1198: the DNS module emits both apex AND www A records at the redirect LB's IP for every redirect-role domain, but the URL map's host_rule only matched the apex host. A www visitor would hit this LB (DNS routes there) but match no host_rule, falling through to default_url_redirect -- the global default_target, socioprophet.com -- instead of that domain's actual redirect_to (e.g. www.sourceos.digital would have redirected to socioprophet.com instead of sourceos.org). Fixed: each host_rule now matches both the apex and www host. Verified in total isolation (throwaway local-only module, zero cloud/backend calls) that the for_each now produces {hosts: [apex, www], path_matcher} correctly per domain for all 5 real redirect domains in domains.yaml. Also tightened enable_redirects' description per the second review comment -- it undersold what actually gets provisioned (a full global external HTTP(S) LB: URL maps, target proxies, forwarding rules -- not just an IP and a cert).
mdheller
marked this pull request as ready for review
August 1, 2026 21:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Captures uncommitted work from a 2026-07-31 session that was interrupted by a usage-limit cutoff before it could be integrated into
feat/dns-redirect-service. Opening as a draft so it's tracked upstream instead of sitting as a pushed-but-invisible branch.Contents: a new
web-redirect-gcpTofu module (Cloud Run + load balancer redirect) plus supportingdns-records/domains.yamlwiring, per the original commit message ("capture uncommitted DNS-redirect module + tofu edits for later integration... cherry-pick/merge into the feature branch when integrating").Status: genuinely incomplete — needs review against current
main(45 commits ahead as of 2026-08-01) and integration with whateverfeat/dns-redirect-servicealready has, not a ready-to-merge change. Filed as part of the daily cross-account inflight-work review so it isn't silently lost.