feat(settings): add flat public hostname strategy#146
Conversation
(cherry picked from commit 11482bd)
bherila
force-pushed
the
feature/flat-public-hostnames-upstream-pr
branch
from
b3ccaa3 to
98b659d
Compare
|
I am against this type of domain structure since it disables the ability to have dynamic environments with automatic HTTPS. This feature will be for advanced users. Can you explain what's the workflow you envision to mitigate this? |
|
This will be useful for all users on Cloudflare's free or Pro plan. Otherwise, if you use Cloudflare's reverse proxy, the free wildcard certificate covers However, |
|
But you could point the wildcard to your server without WAF and then add individual ones with the proxy enabled. If you don't want to have any non protected CF pointing to Temps you could add individual ones to Temps and then enable the firewall for these ones. I don't want to make it more complicated for users to decide what url strategy to use, though I think there's value to have a configurable template for the domain generation. The issue is consistency If you have a lot of environments and change the url strategy you end up in a messy situation where some projects/environments end up with one URL strategy and others with another. Or worse, you change the URL strategy and all the projects/environments are updated, breaking external applications |
|
I agree it shouldn't be a required choice- the default is probably ok. But given the current defaults, Cloudflare users will run into this unexpectedly, by default, when they add Cloudflare. I know it sure took me by surprise, and a bit of debugging to figure out what was actually going on there. That wasn't a very user-friendly experience either! At the end of the day I really just don't want to reveal my server's ip at all, it kind of defeats the point of proxying some but not all subdomains... |
|
Would getting rid of the template string settings and just having the drop down menu be more user-friendly in your opinion? Another option is possibly using the Cloudflare integration itself to detect when a user's plan doesn't support nested subdomains and transparently apply the naming policy, but that could also lead to confusion and might require an API key with more permissions than would otherwise be needed. Yet another alternative could be to allow the user to select their Cloudflare plan when they configure that plugin. Many options... |
…e zone sync Rework the global "flat public hostname strategy" (PR gotempsh#146) into a per-managed-domain capability configured where the DNS provider lives, addressing the maintainer's concern about cluttering global settings with an abstract URL strategy. - temps-core: simplify the generator — `PublicHostnameStrategy` carries the layout methods; drop the global `PublicHostnameSettings`/templates and the `AppSettings.public_hostnames` field. Add `PublicHostnameResolver` trait so routes/deployments resolve the per-domain mode without depending on temps-dns. Add a global `edge_target` setting for DNS record sync. - temps-entities/migrations: add `generated_hostname_mode`, `sync_generated_records`, `zone_access_ok`, `zone_access_error` to `dns_managed_domains`; backfill `flat` onto existing managed domains when the removed global strategy was `flat`. - temps-dns: add a `flat_hostnames` provider capability (Cloudflare = true); surface Cloudflare 401/403 as `PermissionDenied` and a `check_zone_access` helper; per-domain update/preview/apply endpoints; per-hostname DNS zone reconciliation (A/AAAA for an IP edge target, else CNAME) that only removes Temps-generated records; route reload + audit on apply. - Only the per-service hostname layout differs between Standard and Flat, so the resolver is threaded into just the deployments handler and the route table; all other sites use the strategy-independent Standard methods. - web: remove the global hostname-strategy UI (add `edge_target`); per-domain Flat/Sync toggles, token zone-access banner, and a preview→confirm dialog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e zone sync Rework the global "flat public hostname strategy" (PR gotempsh#146) into a per-managed-domain capability configured where the DNS provider lives, addressing the maintainer's concern about cluttering global settings with an abstract URL strategy. - temps-core: simplify the generator — `PublicHostnameStrategy` carries the layout methods; drop the global `PublicHostnameSettings`/templates and the `AppSettings.public_hostnames` field. Add `PublicHostnameResolver` trait so routes/deployments resolve the per-domain mode without depending on temps-dns. Add a global `edge_target` setting for DNS record sync. - temps-entities/migrations: add `generated_hostname_mode`, `sync_generated_records`, `zone_access_ok`, `zone_access_error` to `dns_managed_domains`; backfill `flat` onto existing managed domains when the removed global strategy was `flat`. - temps-dns: add a `flat_hostnames` provider capability (Cloudflare = true); surface Cloudflare 401/403 as `PermissionDenied` and a `check_zone_access` helper; per-domain update/preview/apply endpoints; per-hostname DNS zone reconciliation (A/AAAA for an IP edge target, else CNAME) that only removes Temps-generated records; route reload + audit on apply. - Only the per-service hostname layout differs between Standard and Flat, so the resolver is threaded into just the deployments handler and the route table; all other sites use the strategy-independent Standard methods. - web: remove the global hostname-strategy UI (add `edge_target`); per-domain Flat/Sync toggles, token zone-access banner, and a preview→confirm dialog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Reworked this based on @dviejokfs's feedback — the global URL-strategy setting is gone. Summary of what changed (pushed to this branch):
@dviejokfs on pointing the wildcard at the origin without WAF and adding individual proxied records: that works, but it exposes the origin IP via the unproxied wildcard, which is the thing I'm trying to avoid. The per-domain opt-in plus per-hostname proxied sync keeps every public host behind the proxy without needing the higher-tier plan for nested certs — and because it's opt-in and default-off, it doesn't change anything for users who don't want it. Tested: |
Two safety bugs found testing against a live zone (careowner.com): - The stale-record cleanup deleted any single-label record under the managed domain that Temps didn't generate — which would have deleted app.careowner.com (prod, on another VM) and every other apex record. Deletion is now limited to records Temps tagged as managed (`metadata["comment"] == temps:managed`); the cloudflare crate exposes no record comment, so this is a no-op there and pre-existing/user records are NEVER deleted. The sync only creates/updates. - The sync enumerated all environments, so it tried to point prod-env hosts at the staging/preview edge_target. Production environments (slug/name prod|production) are now excluded. Also: add a proper create-vs-update path (previously existing records were never updated), and enumerate from environments.subdomain instead of environment_domains (which can hold user custom FQDNs). Adds CF-free unit tests via an in-memory mock DnsProvider covering: untagged records are never deleted, only tagged-stale are deleted, create/update/no-op transitions, dry-run writes nothing, A/AAAA/CNAME selection, and prod exclusion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up: DNS-sync safety fix + CF-free test coveragePushed The bugThe first cut of the per-hostname sync computed deletes too aggressively: a dry-run against a real zone (
The fix (
|
2 participants
Motivation
*.example.com), not nested hosts likeapi.staging.example.com. By default, Cloudflare users hit broken HTTPS on generated service hostnames without an obvious cause.Closes #139.
Description (reworked per review feedback)
The original global "hostname strategy" setting has been replaced with a per-managed-domain capability, configured where the DNS provider is set up rather than in global platform settings.
dns_managed_domains.generated_hostname_mode(standard|flat). Defaultstandard, opt-in per domain — no global switch that retroactively rewrites every environment's URLs.service-envvsenv-service).flat_hostnamescapability (Cloudflare = true); the UI surfaces/recommends Flat only where it helps, so it stays out of the way for everyone else.is_calculated=falsedeployment domains,project_custom_domains, andcustom_routesare never touched.sync_generated_records): with a globaledge_targetconfigured, apply reconciles one proxied record per generated hostname against the provider zone (A/AAAA for an IP target, else CNAME), only ever removing records Temps generated.zone_access_ok/zone_access_errorso a mis-scoped token is flagged in the UI instead of failing mysteriously.temps-core; aPublicHostnameResolver(registered bytemps-dns) resolves the per-domain mode for the route table and deployments handler without a crate-dependency cycle.Migration / compatibility
flatonto existing managed domains for any instance that had the removed globalpublic_hostnames.strategy = flat, so behavior doesn't silently revert on upgrade.edge_targetsetting (where synced DNS records point). The old globalpublic_hostnamessetting (strategy + templates) is removed.Testing
cargo test -p temps-core public_hostname --lib(9 pass),cargo test -p temps-dns --lib(224 pass)cargo check --bin temps(full binary) cleanweb/src/api/client) was hand-updated for the new fields/endpoints; regenerate withbun openapi-tsagainst a running backend before a web release.