feat: add godaddy-tofu provider and DNS A records #436
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
ldraney/pal-e-platform!436
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/dns-terraform-provider"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #435
Summary
Discovered Scope
None
Terraform Changes
Provider is not in any public registry -- uses
dev_overridesin~/.terraformrcpointing to thego installoutput of godaddy-tofu.tofu plancannot be run in CI without the provider binary installed locally.tofu fmtpassedtofu validatepassed (pre-existing module-not-installed errors only; no errors from new code)README Impact
Review Checklist
Related Notes
Changes
terraform/versions.tf-- addedgodaddytorequired_providerswith sourceldraney/godaddyterraform/providers.tf-- addedprovider "godaddy"block usingvar.godaddy_api_keyandvar.godaddy_api_secretterraform/dns.tf(new) -- twogodaddy_dns_recordresources:palinks_aandlandscaping_assistant_a, both A records pointing to 178.156.129.142 with TTL 600terraform/variables.tf-- addedgodaddy_api_keyandgodaddy_api_secretvariables (both sensitive)terraform/secrets.auto.tfvars.example-- added GoDaddy credential placeholdersMakefile-- addedgodaddy_api_key godaddy_api_secrettoTF_SECRET_VARSfor Salt pillar renderingTest Plan
make secretsto rendersecrets.auto.tfvarsdev_overridesin~/.terraformrcpointing togo installoutput of godaddy-tofucd terraform && tofu plan-- expect 2 newgodaddy_dns_recordresourcestofu apply-- records created in GoDaddy DNSdig palinks.appanddig landscaping-assistant.app-- both resolve to 178.156.129.142PR #436 Review
DOMAIN REVIEW
Tech stack: Terraform/OpenTofu (HCL), Makefile
This PR integrates the custom
godaddy-tofuprovider into the pal-e-platform Terraform configuration, adding two DNS A records forpalinks.appandlandscaping-assistant.app. Six files changed, 45 additions, 0 deletions. Scope is tight and well-bounded.Provider registration (
versions.tf)ldraney/godaddymatches the provider'smain.goaddress (registry.terraform.io/ldraney/godaddy). Correct.dev_overrides(local binary, not a registry download). The PR body documents this correctly.Provider block (
providers.tf)api_keyandapi_secretmatch the provider schema inprovider.goexactly. Both are passed from sensitive variables. Correct.DNS resources (
dns.tf)godaddy_dns_recordresources using attributesdomain,type,name,data,ttl. All match the resource schema inresource_dns_record.go.178.156.129.142(Hetzner edge VPS). This IP is hardcoded rather than referenced from thehcloudmodule output. This is consistent with how the IP is documented acrossgodaddy-tofu/docs/but is a cross-module coupling risk (see NITS).Variables (
variables.tf)godaddy_api_keyandgodaddy_api_secretare typedstring, markedsensitive = true, and have descriptions. Follows the existing variable pattern used by other providers (hcloud, minio, tailscale).Secrets pipeline (
Makefile,secrets.auto.tfvars.example)TF_SECRET_VARSupdated with both credential names. Insertion point is alphabetically reasonable (after hetzner, before telegram).CHANGEMEplaceholders with source comments pointing to GoDaddy Developer Portal. Consistent with existing entries.Terraform style compliance
tofu fmtpassed per PR body. Indentation and alignment in the diff are consistent with existing files.tofu validatepassed (pre-existing module-not-installed warnings only).BLOCKERS
None.
tofu plan/tofu apply/digverification. Automated testing is not feasible without the provider binary in CI (documented limitation).NITS
Hardcoded IP address in
dns.tf: Both records hardcode178.156.129.142. If the Hetzner edge module outputs this IP (e.g.,module.edge.ipv4_address), a cross-reference would eliminate drift risk if the edge node is ever reprovisioned. Current approach is consistent with existing documentation, so this is non-blocking.No version constraint on godaddy provider: When the provider eventually reaches a registry or stable versioning, adding
version = "~> 0.1"(or similar) would prevent unexpected breakage. Non-blocking while usingdev_overrides.Comment style in
variables.tf: GoDaddy section uses# --- GoDaddy DNS ---while the secrets example file uses Unicode box-drawing characters (---). Minor inconsistency with the rest ofvariables.tfwhich uses the same# ---style, so this is actually correct for that file. No action needed.SOP COMPLIANCE
Closes #435).envfiles, or credentials committedtofu fmtandtofu validatepassedsensitive = truePROCESS OBSERVATIONS
tofu apply. Positive impact on deployment frequency for domain changes.dev_overridesrequirement means CI cannot validate plans, which is an accepted tradeoff documented in the PR body.godaddy-tofu(provider) andpal-e-platform(consumer). The next steps in the public ingress chain are Caddy configuration (#425, #434).VERDICT: APPROVED
PR #436 Review
DOMAIN REVIEW
Stack: Terraform/OpenTofu (HCL), Makefile, secrets pipeline integration.
Terraform correctness -- versions.tf:
The
godaddyprovider entry omits aversionconstraint. Every other provider inrequired_providerspins a version (~> 2.36,~> 2.17,~> 0.28,~> 3.5,~> 1.49). This is intentional and acceptable here because the provider usesdev_overrides(not a registry), so there is no version to resolve. A comment explaining this would help future readers -- see nit below.Terraform correctness -- dns.tf:
Resource syntax is correct.
godaddy_dns_recordmatches the resource type declared in the godaddy-tofu provider (resource_dns_record.go). Attributesdomain,type,name,data,ttlall match the provider schema fromgodaddy-tofu/CLAUDE.md. TTL 600 (10 min) is reasonable for A records.Terraform correctness -- providers.tf:
Provider block uses
api_keyandapi_secretattributes, consistent with the godaddy-tofu provider schema. Referencesvar.godaddy_api_keyandvar.godaddy_api_secret-- both declared in variables.tf withsensitive = true.Terraform correctness -- variables.tf:
Both variables follow the existing pattern:
description,type = string,sensitive = true. Variable naming usesgodaddy_prefix, consistent with thetailscale_,hetzner_,woodpecker_prefix convention in the file.Secrets pipeline -- Makefile:
godaddy_api_key godaddy_api_secretadded toTF_SECRET_VARS. This meansmake tofu-secretswill render them from Salt pillar intosecrets.auto.tfvars. The placement is correct (afterhetzner_api_token edge_ssh_public_key, beforetelegram_landscaping_chat_id).Secrets pipeline -- secrets.auto.tfvars.example:
Placeholders added with helpful comment pointing to GoDaddy Developer Portal. Follows the existing
CHANGEMEpattern. Placement between Telegram and Hetzner sections is fine.Hardcoded IP (178.156.129.142):
Both DNS records hardcode the edge VPS IP. The Hetzner edge module at
terraform/modules/hetzner-edge/outputs.tfexposesserver_ipv4as a module output. Ideally,dns.tfwould referencemodule.hetzner_edge.server_ipv4instead of hardcoding178.156.129.142. However, this creates a dependency ordering concern: if the edge VPS is destroyed and recreated with a new IP, the DNS records would update automatically -- which is actually desirable behavior. See nit below.BLOCKERS
None. No security issues, no missing test coverage (declarative Terraform does not require unit tests for resource declarations), no credentials in code, no DRY violations in auth paths.
NITS
Hardcoded IP vs module reference (dns.tf): Consider replacing
"178.156.129.142"with a local or a reference to the hetzner-edge module output:Then use
local.edge_vps_ipin both records. This eliminates the hardcoded value and ensures DNS records auto-update if the edge VPS IP changes. If the ordering concern is real (DNS must exist before edge module), a comment explaining why the IP is hardcoded would suffice.Missing version comment (versions.tf): Add a comment explaining why no version constraint exists:
Missing
telegram_landscaping_chat_idin secrets.auto.tfvars.example: Pre-existing gap -- this variable is inTF_SECRET_VARSandvariables.tfbut not in the example file. Not introduced by this PR, but worth noting.Missing OIDC secrets in secrets.auto.tfvars.example:
grafana_oidc_client_secret,admin_oidc_client_secret,admin_auth_secret,forgejo_oidc_client_secret,harbor_oidc_client_secretare all inTF_SECRET_VARSandvariables.tfbut missing from the example file. Also pre-existing -- not introduced by this PR.SOP COMPLIANCE
tofu fmtpassed (stated in PR body)tofu validatepassed (stated in PR body)sensitive = truePROCESS OBSERVATIONS
tofu plancannot run in CI without the godaddy-tofu provider binary installed. This is a known limitation documented in the PR body. Consider adding the provider build step to CI once the provider is stable.docs/hetzner-edge.mdfile already documents the DNS A records for these domains. If the hardcoded IP is kept, ensuredocs/hetzner-edge.mdstays in sync.VERDICT: APPROVED