-
Notifications
You must be signed in to change notification settings - Fork 159
feat: add support for zetaclient external DNS name #4254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds optional public DNS support alongside public IP throughout zetaclient: CLI flag and config field, telemetry server DNS handling and endpoint, TSS setup accepting DNS, localnet hostname mappings and startup script changes, and bumps go-tss to a pre-release with hostname support. Changelog updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as Operator
participant CLI as zetaclientd (init)
participant FS as Config File
participant Svc as zetaclientd (start)
participant Tele as TelemetryServer
User->>CLI: zetaclientd init --public-dns=<host>
CLI->>FS: write Config{ PublicDNS: <host>, ... }
User->>Svc: zetaclientd start
Svc->>FS: read Config
Svc->>Tele: NewTelemetryServer()
Svc->>Tele: SetIPAddress(cfg.PublicIP)
Svc->>Tele: SetDNSName(cfg.PublicDNS)
Tele-->>User: /dns HTTP returns cfg.PublicDNS
sequenceDiagram
autonumber
participant Svc as zetaclientd (start)
participant TSS as go-tss
note over Svc: Validate endpoints
Svc->>Svc: if cfg.PublicIP=="" && cfg.PublicDNS=="" then warn
Svc->>TSS: CreateServer(ExternalIP=cfg.PublicIP,<br/>ExternalDNS=cfg.PublicDNS, ...)
TSS-->>Svc: Server initialized (IP and/or DNS-based)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4254 +/- ##
===========================================
- Coverage 65.79% 65.77% -0.03%
===========================================
Files 466 466
Lines 34386 34416 +30
===========================================
+ Hits 22625 22637 +12
- Misses 10764 10782 +18
Partials 997 997
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
contrib/localnet/scripts/start-zetaclientd.sh (1)
107-112: Ensure DNS mapping exists for all peers; consider parameterizing DNS suffix.Using --public-dns "$HOSTNAME.com" assumes all containers can resolve every peer’s “.com” FQDN. Please add extra_hosts for all peers (including zetaclient-new-validator) across all zetaclient services in docker-compose, not just self-mappings. Also consider a DNS suffix env (e.g., DNS_SUFFIX, default ".com") to avoid hard-coding.
📜 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(3 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:
cmd/zetaclientd/start.gozetaclient/config/types.gozetaclient/metrics/telemetry.gozetaclient/tss/setup.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 (2)
📚 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
📚 Learning: 2024-09-24T18:43:46.232Z
Learnt from: gartnera
PR: zeta-chain/node#2817
File: contrib/rpcimportable/go.mod:1-3
Timestamp: 2024-09-24T18:43:46.232Z
Learning: In this project, minor version differences in Go modules are acceptable and do not require strict consistency.
Applied to files:
go.mod
🔇 Additional comments (8)
cmd/zetaclientd/start.go (1)
227-228: LGTM: Telemetry sets DNS name from config.Setting DNS alongside IP keeps telemetry consistent with the new PublicDNS field.
zetaclient/config/types.go (1)
82-82: LGTM: Config surface extended with PublicDNS.Field name and JSON tag are consistent with existing style. No masking required.
cmd/zetaclientd/initconfig.go (2)
15-18: LGTM: Adds publicDNS option to CLI struct.Matches the new config field; no concerns.
50-57: LGTM: Adds --public-dns flag.Clear help text and parity with --public-ip.
go.mod (1)
309-309: Confirm ExternalDNS support in go-tss pseudo-version and update go.sum
Verify that commit 2ae33848328f in github.com/zeta-chain/go-tss introducesNetworkConfig.ExternalDNSand related DNS handling, then rungo mod tidyand commit the updatedgo.sum.zetaclient/metrics/telemetry.go (3)
24-37: dnsName field integrates cleanly.The new
dnsNamestate slots in alongside the existing telemetry attributes without upsetting synchronization or construction flows.
111-123: Getter/setter concurrency handling is consistent.Locking mirrors the other accessors, so the DNS name stays thread-safe with minimal surface area.
198-255: Telemetry DNS endpoint is wired correctly.The
/dnsroute follows the existing pattern (status code first, plain response body) and keeps the handler focused and maintainable.
|
Are the IP and the DNS mutually exclusive? Is it a XOR or an OR? |
…t-support-external-DNS
* add support for zetaclient external DNS name * add changelog entry * point to go-tss tag; fix changelog PR # and add zetaclient config description * add zetaclient config file validation method
Description
The
go-tssside modification is in PR 64Closes #1038
How Has This Been Tested?
Summary by CodeRabbit