feat: add support for zetaclient external DNS name #4234
Conversation
📝 WalkthroughWalkthroughAdds DNS hostname support alongside public IP for zetaclient: new CLI flag (--public-dns), config field (PublicDNS), telemetry DNS exposure (/dns, setters/getters), startup wiring to set DNS, TSS setup accepting DNS or IP and passing ExternalDNS, localnet docker DNS mappings, and a go-tss dependency update. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Operator
participant CLI as zetaclientd (CLI)
participant Cfg as Config
participant Tele as TelemetryServer
participant TSS as TSS Server
Operator->>CLI: initialize --public-dns <name> (or --public-ip)
CLI->>Cfg: InitializeConfig(opts)\nset Cfg.PublicDNS/PublicIP
note right of Cfg: Config now includes PublicDNS
Operator->>CLI: start
CLI->>Tele: SetIPAddress(Cfg.PublicIP)
CLI->>Tele: SetDNSName(Cfg.PublicDNS)
Tele-->>Operator: /dns endpoint exposes DNS
CLI->>TSS: NewServer(Cfg)
alt Valid network identity
TSS->>TSS: Validate Cfg.PublicIP || Cfg.PublicDNS
TSS->>TSS: NetworkConfig{ExternalIP, ExternalDNS}
TSS-->>CLI: Server ready
else Missing identity
TSS-->>CLI: Error (require IP or DNS)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ws4charlie
added
zetaclient
ws4charlie
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4234 +/- ##
==========================================
- Coverage 65.00% 64.97% -0.03%
==========================================
Files 452 452
Lines 34124 34137 +13
==========================================
Hits 22182 22182
- Misses 10967 10980 +13
Partials 975 975
🚀 New features to boost your workflow:
|
…t-support-tss-externalDNS
ws4charlie
added
zetaclient
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contrib/localnet/scripts/start-zetaclientd.sh (1)
99-112: Fix Bash syntax: missing semicolon/newline before then.if [[ … ]] then is invalid; it must be if [[ … ]]; then or place then on the next line. This will cause the script to fail.
Apply for this block (and mirror the same fix for the earlier if at Line 82):
-if [[ $HOSTNAME != "zetaclient0" && ! -f ~/.zetacored/config/zetaclient_config.json ]] then +if [[ $HOSTNAME != "zetaclient0" && ! -f ~/.zetacored/config/zetaclient_config.json ]]; thenOptional hardening:
- Add set -euo pipefail at the top to fail fast on errors.
🧹 Nitpick comments (6)
changelog.md (1)
21-21: Tighten wording for clarity.Consider: “add support for zetaclient public DNS name” (drop “additional”).
cmd/zetaclientd/start.go (1)
227-228: Guard SetDNSName when empty.If cfg.PublicDNS is empty, skip calling SetDNSName to avoid exposing blank state on /dns.
-telemetry.SetDNSName(cfg.PublicDNS) +if cfg.PublicDNS != "" { + telemetry.SetDNSName(cfg.PublicDNS) +}cmd/zetaclientd/initconfig.go (1)
17-17: Validate public-dns/public-ip inputs and mutual exclusivity.Enforce at least one of --public-ip or --public-dns; warn if both are set.
Add after opts are parsed in InitializeConfig():
// ensure addressing is provided if opts.publicIP == "" && opts.publicDNS == "" { return errors.New("one of --public-ip or --public-dns must be set") } if opts.publicIP != "" && opts.publicDNS != "" { // non-fatal: prefer explicitness fmt.Println("warning: both --public-ip and --public-dns set; both will be stored") }Also applies to: 54-54, 66-66, 96-96
zetaclient/metrics/telemetry.go (3)
32-37: DNS field addition is fine; consider a unified accessor for external hostCallers often want “the externally reachable host,” regardless of whether it’s IP or DNS. Adding a helper reduces duplication and ambiguity across handlers/consumers.
Example helper (outside the selected range):
// GetExternalHost returns DNS name if set, otherwise the IP address. func (t *TelemetryServer) GetExternalHost() string { t.mu.Lock() defer t.mu.Unlock() if t.dnsName != "" { return t.dnsName } return t.ipAddress }
111-123: Normalize and sanitize the DNS on set (trim + lowercase)Domain names are case-insensitive and often sourced from flags/env; normalizing prevents subtle mismatches. Minimal, safe change.
Apply within this block:
func (t *TelemetryServer) SetDNSName(dns string) { - t.mu.Lock() - defer t.mu.Unlock() - t.dnsName = dns + dns = strings.ToLower(strings.TrimSpace(dns)) + t.mu.Lock() + defer t.mu.Unlock() + t.dnsName = dns }Add import (outside the selected range):
import "strings"
251-256: Set a Content-Type and return 204 when DNS is unsetImprove HTTP semantics and client interoperability by signaling absence and avoiding content sniffing.
-// dnsHandler returns the dns name -func (t *TelemetryServer) dnsHandler(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - fmt.Fprintf(w, "%s", t.GetDNSName()) -} +// dnsHandler returns the DNS name +func (t *TelemetryServer) dnsHandler(w http.ResponseWriter, _ *http.Request) { + dns := t.GetDNSName() + if dns == "" { + w.WriteHeader(http.StatusNoContent) + return + } + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + fmt.Fprint(w, dns) +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
changelog.md(1 hunks)cmd/zetaclientd/initconfig.go(4 hunks)cmd/zetaclientd/start.go(1 hunks)contrib/localnet/docker-compose.yml(4 hunks)contrib/localnet/scripts/start-zetaclientd.sh(1 hunks)go.mod(1 hunks)zetaclient/config/types.go(1 hunks)zetaclient/metrics/telemetry.go(4 hunks)zetaclient/tss/setup.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Files:
zetaclient/config/types.gozetaclient/tss/setup.gocmd/zetaclientd/start.gozetaclient/metrics/telemetry.gocmd/zetaclientd/initconfig.go
**/*.sh
⚙️ CodeRabbit configuration file
Review the shell scripts, point out issues relative to security, performance, and maintainability.
Files:
contrib/localnet/scripts/start-zetaclientd.sh
🧠 Learnings (1)
📚 Learning: 2025-09-21T17:54:21.110Z
Learnt from: kingpinXD
PR: zeta-chain/node#4225
File: Makefile:383-385
Timestamp: 2025-09-21T17:54:21.110Z
Learning: In upgrade tests, docker-compose-upgrade.yml is used specifically to build older versions of zetacored and zetaclientd, while docker-compose.yml handles the newer version configuration. Environment variables like USE_ZETAE2E_ANTE are intended for the newer version and don't need to be propagated to the upgrade compose file.
Applied to files:
contrib/localnet/docker-compose.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build-zetanode
- GitHub Check: test-sim-import-export / sim
- GitHub Check: test-sim-after-import / sim
- GitHub Check: test-sim-nondeterminism / sim
- GitHub Check: test-sim-fullappsimulation / sim
- GitHub Check: gosec
- GitHub Check: lint
- GitHub Check: build-and-test
- GitHub Check: build
- GitHub Check: analyze (go)
🔇 Additional comments (3)
zetaclient/config/types.go (1)
101-101: LGTM: Config gains PublicDNS.The field addition is straightforward and backward compatible with existing JSON.
Please confirm any external docs and sample configs are updated to mention PublicDNS.
go.mod (1)
309-309: Pin go‑tss to a released tag and verify NetworkConfig.ExternalDNS
- go.mod currently uses a pseudo‑version (v0.6.4-...) while upstream tags stop at v0.6.3; repo search shows no NetworkConfig/ExternalDNS in go‑tss.
- Action: pin to a released tag (prefer v0.6.3) or wait for an official v0.6.4, and confirm the chosen release exposes NetworkConfig.ExternalDNS; if it does not, update zetaclient/tss/setup.go ( ExternalDNS at zetaclient/tss/setup.go:228 ) to match the tagged API or vendor the specific commit with justification.
zetaclient/metrics/telemetry.go (1)
197-199: Update dashboards/probes/docs to consume the new /dns telemetry endpointTelemetry now exposes /dns; wiring confirmed — SetDNSName is called in cmd/zetaclientd/start.go:227 and PublicDNS is defined in zetaclient/config/types.go:101. Update dashboards, monitoring probes, and any docs that scrape telemetry to use /dns (or validate fallbacks to /ip).
| case cfg.PublicIP == "" && cfg.PublicDNS == "": | ||
| logger.Warn().Msg("no public IP or DNS is provided") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Require at least one of PublicIP/PublicDNS; fail fast.
Starting TSS without an externally reachable address will produce unreachable peers. Make this a hard error instead of a warning.
- case cfg.PublicIP == "" && cfg.PublicDNS == "":
- logger.Warn().Msg("no public IP or DNS is provided")
+ case cfg.PublicIP == "" && cfg.PublicDNS == "":
+ return nil, errors.New("no public IP or DNS provided; set --public-ip or --public-dns")Also good to log both when set:
logger.Info().Str("external_ip", cfg.PublicIP).Str("external_dns", cfg.PublicDNS).Msg("tss external addressing")Also applies to: 228-229
🤖 Prompt for AI Agents
In zetaclient/tss/setup.go around lines 210-212 (and similarly at 228-229),
change the warning for missing external addressing to a hard error that stops
startup: check if both cfg.PublicIP and cfg.PublicDNS are empty and return/log a
fatal error (or return an error from this function) instead of logger.Warn();
additionally, when either or both are set log them with logger.Info including
structured fields "external_ip" and "external_dns" as suggested to make the
configured external addressing explicit.
|
Note: will not be merged to the current develop branch |
|
losing in favor of the develop PR |
Description
The
go-tssside modification is in PR 64Closes #1038
How Has This Been Tested?
Summary by CodeRabbit
New Features
Documentation
Chores