From cb23f123ae3cafcacb3aebdbbf6216e51ff354c8 Mon Sep 17 00:00:00 2001 From: Gleb Tv Date: Wed, 12 Aug 2026 20:48:01 +0300 Subject: [PATCH] feat(worker): adopt canonical public URL --- .env.example | 5 +- README.md | 2 +- cmd/rsmon-worker/main.go | 30 +++- cmd/rsmon-worker/management.go | 1 + docs/architecture.md | 12 +- docs/changelog.md | 26 ++++ docs/control-plane-protocol.md | 20 ++- docs/implementation-roadmap.md | 6 +- docs/install.md | 22 ++- docs/public-endpoint-and-identity.md | 20 +++ internal/distworker/config.go | 192 +++++++++++++++++++++---- internal/distworker/config_test.go | 200 +++++++++++++++++++++++++-- internal/distworker/runner.go | 19 ++- internal/distworker/runner_test.go | 56 ++++++++ internal/installer/install.go | 53 +++++-- internal/installer/install_test.go | 125 +++++++++++++++++ internal/wire/types.go | 9 ++ internal/wire/types_test.go | 76 ++++++++++ packaging/systemd/worker.env.example | 4 +- 19 files changed, 804 insertions(+), 74 deletions(-) diff --git a/.env.example b/.env.example index a21d7c2..2da8e82 100644 --- a/.env.example +++ b/.env.example @@ -9,7 +9,10 @@ RSMON_WORKER_IMAGE_DIGEST=replace-with-64-lowercase-hex-digest WORKER_HOST=0.0.0.0 WORKER_PORT=27401 WORKER_BIND_IP=127.0.0.1 -WORKER_URL= +# Advertised public origin: absolute http(s) origin (scheme + host, no path). +# PUBLIC_URL is canonical. Legacy WORKER_URL is still read during the bounded +# migration but must not be used for new installs. +PUBLIC_URL= WORKER_LOGIN=admin WORKER_PASSWORD=replace-with-a-long-random-password diff --git a/README.md b/README.md index 54fa670..8557007 100644 --- a/README.md +++ b/README.md @@ -185,7 +185,7 @@ used in a trusted disposable environment. | `RSMON_TOKEN` | yes | none | Worker bearer token. | | `WORKER_HOST` | no | `0.0.0.0` | Operator-console bind address. | | `WORKER_PORT` | no | `27401` | Operator-console port. | -| `WORKER_URL` | no | none | Public URL advertised to the control plane. | +| `PUBLIC_URL` | no | none | Advertised public origin (scheme + host, no path). Canonical name; the legacy `WORKER_URL` is still read during the bounded migration in `docs/public-endpoint-and-identity.md`. | | `WORKER_LOGIN` | yes | none | Operator-console basic-auth login. | | `WORKER_PASSWORD` | yes | none | Operator-console basic-auth password. | | `RSMON_WEBAPP_DATA_DIR` | no | user data directory | SQLite and local UI state. | diff --git a/cmd/rsmon-worker/main.go b/cmd/rsmon-worker/main.go index f43bc53..707e5d1 100644 --- a/cmd/rsmon-worker/main.go +++ b/cmd/rsmon-worker/main.go @@ -76,6 +76,14 @@ func main() { if err := webapp.ValidateBasicAuth(cfg.HTTP.Login, cfg.HTTP.Password); err != nil { log.Fatalf("worker: %v", err) } + // Enforce the advertised PUBLIC_URL origin rules (and the + // production HTTPS policy) before accepting work. willListen=false + // because the webapp's both-empty local bcrypt mode is legitimate + // and already gated by ValidateBasicAuth above; here we only check + // the URL invariants. + if err := distworker.ValidateHTTPConfig(cfg.HTTP, false); err != nil { + log.Fatalf("worker: %v", err) + } runner := distworker.NewRunner(&cfg) @@ -501,20 +509,30 @@ func logHTTPSettings(h distworker.HTTPConfig, willListen bool) { if h.Login == "" { login = "(empty)" } - url := h.URL + url := h.PublicURL if url == "" { url = "(empty)" } - log.Printf("worker http settings: host=%s port=%d url=%s login=%s will_listen=%t", + log.Printf("worker http settings: host=%s port=%d public_url=%s login=%s will_listen=%t", h.Host, h.Port, url, login, willListen) - if h.URL != "" { - if host, warn := distworker.WarnInsecurePublicURL(h.URL); warn { + if h.PublicURL != "" { + if host, warn := distworker.WarnInsecurePublicURL(h.PublicURL); warn { + label := distworker.EnvPublicURL + if h.URLSource == distworker.PublicURLSourceLegacy { + label = distworker.EnvWorkerURLLegacy + } log.Printf( - "worker http settings: WARN WORKER_URL=http://%s uses plain HTTP on a non-loopback host; "+ - "production deployments usually terminate TLS at a reverse proxy", host, + "worker http settings: WARN %s=http://%s uses plain HTTP on a non-loopback host; "+ + "production deployments usually terminate TLS at a reverse proxy", label, host, ) } } + if _, source := distworker.PublicURLFromEnv(); source == distworker.PublicURLSourceLegacy { + log.Printf( + "worker http settings: WARNING WORKER_URL is deprecated (bounded migration); " + + "rename it to PUBLIC_URL before it is removed (docs/public-endpoint-and-identity.md milestone 1)", + ) + } } func loadDotEnv() { diff --git a/cmd/rsmon-worker/management.go b/cmd/rsmon-worker/management.go index 24a4e1d..340bc3d 100644 --- a/cmd/rsmon-worker/management.go +++ b/cmd/rsmon-worker/management.go @@ -35,6 +35,7 @@ func installCommand(args []string) int { fs.StringVar(&opts.Token, "api-key", "", "worker API token (alias for --token)") fs.StringVar(&tokenFile, "token-file", "", "file containing the worker API token") fs.StringVar(&opts.URL, "url", "", "RSMon server URL (RSMON_URL; default https://rsmon.ru)") + fs.StringVar(&opts.PublicURL, "public-url", "", "advertised public origin (PUBLIC_URL; scheme + host, no path)") fs.StringVar(&opts.Host, "host", "", "operator console bind address (WORKER_HOST; default 127.0.0.1)") fs.StringVar(&opts.Port, "port", "", "operator console port (WORKER_PORT; required with --name)") fs.StringVar(&opts.Login, "login", "", "operator console login (WORKER_LOGIN; default admin with a generated password)") diff --git a/docs/architecture.md b/docs/architecture.md index e8008d2..89df68b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -78,10 +78,14 @@ provided by a validated control-plane task. Raft, and closes SQLite without exceeding the service stop timeout. - A web-console failure is reported and causes an intentional process policy; it must not silently leave a partially healthy process. -- Startup currently rejects malformed `WORKER_URL`; the accepted target is - `PUBLIC_URL` with bounded compatibility migration. Startup also rejects - incomplete auth credentials, invalid cluster settings, and unwritable data - directories before accepting work. +- Startup validates the advertised origin: canonical `PUBLIC_URL` is held to + the strict scheme-and-authority shape (no userinfo, query, fragment, or + ambiguous path) and plain HTTP on a non-loopback host is rejected in an + explicitly production environment; the legacy `WORKER_URL` is read as a + bounded-migration fallback, held only to the tolerant absolute-URL check, + and logged with a deprecation warning. Both reject a missing hostname, e.g. + `https://:27401`. Startup also rejects incomplete auth credentials, invalid + cluster settings, and unwritable data directories before accepting work. - `GET /healthz` reports process-local liveness. Control-plane reachability is a separate readiness/selfcheck signal and must not make a healthy container fail its local liveness probe. diff --git a/docs/changelog.md b/docs/changelog.md index 67a073a..3f5ebb1 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,31 @@ # Changelog +## 2026-08-12 + +### Public endpoint configuration (milestone 1 of public-endpoint-and-identity) + +- `PUBLIC_URL` is now the canonical advertised public origin; the legacy + `WORKER_URL` is accepted only for the bounded migration and logs a startup + deprecation warning. `PUBLIC_URL` wins whenever both are set, and the + installer drops `WORKER_URL` from freshly written env files when + `PUBLIC_URL` is present. +- Startup and install validate the origin shape: absolute `http`/`https` URL + with scheme and authority only; userinfo, query, fragment, and any path + other than `/` are rejected. +- Plain-HTTP `PUBLIC_URL` on a non-loopback host is rejected in an explicitly + production environment (`DEPLOY_ENV`, `RSMON_ENV`, or `GO_ENV` = + `production`); other environments keep the historical warning. +- `internal/wire` adds `public_url` to `WorkerInit` (control plane to worker), + keeping the legacy `url` field for old control planes; the worker prefers + `public_url` and rejects unusable values, keeping the previous accepted + URL. `RegisterRequest.public_url` is the registration contract for the + pending RSMon counterpart (the worker does not currently transmit the URL + during registration; it consumes the accepted endpoint from `WorkerInit`). +- The legacy `WORKER_URL` is held only to the tolerant absolute-URL check + (no newly rejected legacy shapes); `PUBLIC_URL` is held to the strict + scheme-and-authority origin shape. Both reject a missing hostname, e.g. + `https://:27401`. + ## 2026-07-19 ### Standalone installation and deployment diff --git a/docs/control-plane-protocol.md b/docs/control-plane-protocol.md index a2043f0..dd9bfe3 100644 --- a/docs/control-plane-protocol.md +++ b/docs/control-plane-protocol.md @@ -46,9 +46,9 @@ without execution or reporting. ## Initialization And Refresh -The worker proposes `PUBLIC_URL` during registration. The control plane -validates and canonicalizes it; `wire.WorkerInit` returns the accepted endpoint -and supplies runtime values owned by the control plane: +The worker is expected to propose `PUBLIC_URL` during registration and the +control plane to validate and canonicalize it; `wire.WorkerInit` returns the +accepted endpoint and supplies runtime values owned by the control plane: - worker ID, region, advertised URL, capabilities, and concurrency; - allowed notification methods and account IDs; @@ -57,6 +57,12 @@ and supplies runtime values owned by the control plane: - scoped notification credentials and system contacts; - signed, cluster-scoped peer topology for selfcheck and Raft behavior. +Current worker behavior: the worker validates its local `PUBLIC_URL` +configuration at startup and *consumes* the accepted endpoint from +`wire.WorkerInit` (preferring `public_url`, falling back to the legacy `url` +field). Transmitting the proposed URL during registration is the pending RSMon +control-plane counterpart; the worker does not currently send it. + Worker ID, account, region, cluster, membership, role, topology generation, and certificate identity are control-plane authority. Local environment or a peer response cannot override them. Static peer environment remains lab-only. @@ -65,6 +71,14 @@ The worker clamps supplied concurrency to its local maximum. Credentials are replaced atomically in memory on refresh. Removed credentials must become unavailable immediately after the refresh is applied. +`wire.WorkerInit` returns the accepted endpoint as `public_url`, with the +legacy `url` field still populated during the bounded migration; the worker +prefers `public_url` and ignores an unusable value (keeping the previous +accepted URL). `RegisterRequest.public_url` is the registration contract the +RSMon control-plane counterpart must populate when it wires worker-initiated +registration; the worker does not transmit it today. See +[public-endpoint-and-identity.md](public-endpoint-and-identity.md). + Private-worker hardening will add an immutable worker account ID, config version, expiry, and signature. Until then the executable trusts the authenticated control plane to send a correctly scoped config; server-side diff --git a/docs/implementation-roadmap.md b/docs/implementation-roadmap.md index 4630678..80a8397 100644 --- a/docs/implementation-roadmap.md +++ b/docs/implementation-roadmap.md @@ -30,8 +30,10 @@ Worker repository: - wire local inventory and metrics collector lifecycle into web server start and shutdown; -- validate full HTTP config, including the accepted `PUBLIC_URL`, in main - startup; +- [x] validate full HTTP config, including the accepted `PUBLIC_URL`, in main + startup (milestone 1 of + [public-endpoint-and-identity.md](public-endpoint-and-identity.md): origin + shape, production HTTPS, legacy `WORKER_URL` fallback); - [x] reconnect in memory on token rotation without stopping the runner; - [x] resend bounded check/notification results after websocket reconnect; - define process policy when the web listener exits unexpectedly; diff --git a/docs/install.md b/docs/install.md index 3eccbe3..8b0ccd6 100644 --- a/docs/install.md +++ b/docs/install.md @@ -33,8 +33,8 @@ sudo apt-get install -y ca-certificates chromium libcap2-bin tzdata The installer reads the same environment variables the worker runtime reads. Each variable is resolved with this precedence (highest first): -1. **Explicit flags** (`--url`, `--token`, `--host`, `--port`, `--login`, - `--password`, `--name`). +1. **Explicit flags** (`--url`, `--public-url`, `--token`, `--host`, `--port`, + `--login`, `--password`, `--name`). 2. **`--env-file`** — a strict, systemd-safe `KEY=VALUE` file (validated before anything is written to disk). 3. **Process environment**, including a `.env` file in the working directory @@ -54,7 +54,7 @@ values automatically. To override a value, pass the matching flag. | `RSMON_TOKEN` | yes | none | Worker bearer token. | | `WORKER_HOST` | no | `127.0.0.1` | Operator-console bind address. | | `WORKER_PORT` | no | `27401` (primary) | Operator-console port. **Required** for named instances. | -| `PUBLIC_URL` | no | none | Proposed public HTTPS origin; accepted by the control plane. | +| `PUBLIC_URL` | no | none | Advertised public origin: absolute http(s) URL with scheme and authority only (no userinfo, query, fragment, or path). Canonical name; `WORKER_URL` is a deprecated legacy alias read only during the bounded migration. | | `WORKER_LOGIN` | no | `admin` (generated) | Operator-console basic-auth login. | | `WORKER_PASSWORD` | no | generated | Operator-console basic-auth password. | | `WORKER_COMPOSE_ENABLED` | no | feature default (on) | Enable Docker Compose discovery/management. | @@ -69,8 +69,19 @@ endpoints. `PUBLIC_URL` does not bind a listener or terminate TLS. It advertises the one external origin used for the console, authenticated peer status, and planned -Raft `/raft` transport. See -[public-endpoint-and-identity.md](public-endpoint-and-identity.md). +Raft `/raft` transport. It must be an absolute `http`/`https` URL with a scheme +and authority and nothing else; a path (other than `/`), userinfo, query, or +fragment is rejected at install time and at worker startup. `PUBLIC_URL` is the +canonical variable; the legacy `WORKER_URL` is still accepted for the bounded +migration defined in +[public-endpoint-and-identity.md](public-endpoint-and-identity.md), and is +dropped from a freshly written env file whenever `PUBLIC_URL` is also set. +The legacy `WORKER_URL` is held only to the tolerant absolute-URL check, so +shapes that previously installed keep working. In an explicitly production +environment (`DEPLOY_ENV=production`, or `RSMON_ENV`/`GO_ENV=production`) a +plain-HTTP `PUBLIC_URL` on a non-loopback host is rejected at startup; a legacy +`WORKER_URL` keeps the historical warn-only behavior. Both variables reject a +missing hostname, e.g. `https://:27401`. Values must be systemd-safe: no whitespace, quotes, backslashes, or `$` interpolation inside a value. This keeps the file unambiguous across systemd @@ -208,6 +219,7 @@ rsmon-worker install [--token TOKEN|--token-file FILE|--env-file FILE] | `--token-file` | File containing the worker token (avoids shell history). | | `--env-file` | Strict worker env file; validated then used as the config source. | | `--url` | Control-plane URL (`RSMON_URL`). | +| `--public-url` | Advertised public origin (`PUBLIC_URL`; scheme + host, no path). | | `--host` | Console bind address (`WORKER_HOST`). | | `--port` | Console port (`WORKER_PORT`; required with `--name`). | | `--login` | Console login (`WORKER_LOGIN`). | diff --git a/docs/public-endpoint-and-identity.md b/docs/public-endpoint-and-identity.md index 6f9e532..c8d94ba 100644 --- a/docs/public-endpoint-and-identity.md +++ b/docs/public-endpoint-and-identity.md @@ -6,6 +6,16 @@ Accepted target architecture. Ordinary peer status checks already support an external HTTPS worker URL. Raft currently uses a separate plaintext listener, static peers, and shared Basic authentication; it does not yet meet this plan. +Milestone 1 (worker side) is implemented: the worker configures `PUBLIC_URL` +as the canonical advertised origin with the legacy `WORKER_URL` accepted only +for a bounded migration, and validates the origin shape at startup and install +time. On the wire the worker consumes the accepted endpoint from +`wire.WorkerInit` (`public_url`, falling back to the legacy `url` field) and +rejects unusable values. Transmitting the proposed URL during registration is +the pending RSMon control-plane counterpart: it must populate `public_url` in +`WorkerInit` and accept `RegisterRequest.public_url`; until then the worker +reads whichever field the control plane sends. + ## One Worker, One Public URL Every worker configures one absolute `PUBLIC_URL`, for example: @@ -103,6 +113,16 @@ voter. 1. Add `PUBLIC_URL` wire/config fields while accepting legacy `WORKER_URL` only for a bounded migration. + + Worker side implemented: `PUBLIC_URL` is canonical, `WORKER_URL` is a + deprecated fallback with a startup warning, `internal/wire` carries + `public_url` on `WorkerInit` (keeping `url` for compatibility), and + startup/install enforce the strict origin shape for `PUBLIC_URL` while + tolerating legacy `WORKER_URL` shapes. Control-plane counterpart: read + `RegisterRequest.public_url` when worker-initiated registration is wired, + populate `public_url` (not `url`) in `WorkerInit`, and persist the accepted + origin. Until then the worker consumes whichever of `public_url`/`url` the + control plane sends. 2. Validate ownership/reachability and return accepted signed configuration. 3. Add scoped, versioned in-memory peer topology and concurrent health probes. 4. Extend status APIs/UI with networking, control-plane RTT, and Raft state. diff --git a/internal/distworker/config.go b/internal/distworker/config.go index 23115a5..5abcd76 100644 --- a/internal/distworker/config.go +++ b/internal/distworker/config.go @@ -21,8 +21,18 @@ const ( // same host. Operators are still free to override via WORKER_PORT. DefaultHTTPPort = 27401 + // EnvPublicURL is the canonical environment variable for the + // publicly advertised origin. It wins over the legacy + // EnvWorkerURLLegacy when both are set. + EnvPublicURL = "PUBLIC_URL" + // EnvWorkerURLLegacy is the legacy name for the advertised public + // origin. It is accepted only for the bounded migration defined in + // docs/public-endpoint-and-identity.md milestone 1 and is + // deprecated: new configuration must set EnvPublicURL. + EnvWorkerURLLegacy = "WORKER_URL" + // schemeHTTP / schemeHTTPS are the only schemes accepted on - // WORKER_URL. Peer workers and the main app need an http(s) origin + // PUBLIC_URL. Peer workers and the main app need an http(s) origin // they can dial with Go's net/http stack. schemeHTTP = "http" schemeHTTPS = "https" @@ -34,20 +44,55 @@ const ( // HTTPConfig holds the settings that govern the worker's local HTTP // listener (web app MVP in Task 3 and Raft peer connections in Task 4). -// Host/Port are the bind interface. URL is the publicly-advertised -// location peers and the main app use to reach the worker; it is NOT +// Host/Port are the bind interface. PublicURL is the publicly-advertised +// origin peers and the main app use to reach the worker; it is NOT // derived from Host:Port because workers commonly sit behind a reverse // proxy / Traefik with HTTPS while listening on plain HTTP internally. // +// PublicURL is sourced from PUBLIC_URL (strict origin) or, for the +// bounded migration, from the deprecated WORKER_URL (legacy-tolerant +// absolute URL). URLSource records which variable supplied the value so +// validation and diagnostics name the correct source. +// // Login/Password are basic auth credentials for the worker web app API // (Task 3). They are kept in memory only; Task 3 may hash them before // any persistent store. type HTTPConfig struct { - Host string - Port int - URL string - Login string - Password string + Host string + Port int + PublicURL string + URLSource PublicURLSource + Login string + Password string +} + +// PublicURLSource identifies which environment variable supplied the +// advertised public origin, so callers can log the legacy deprecation +// exactly once. +type PublicURLSource int + +const ( + // PublicURLSourceNone means no advertised origin is configured. + PublicURLSourceNone PublicURLSource = iota + // PublicURLSourceCanonical means PUBLIC_URL was configured. + PublicURLSourceCanonical + // PublicURLSourceLegacy means only WORKER_URL was configured; the + // value is accepted for the bounded migration. + PublicURLSourceLegacy +) + +// PublicURLFromEnv reads the advertised public origin. PUBLIC_URL is +// the canonical source; the deprecated WORKER_URL is used only when +// PUBLIC_URL is unset. The returned source lets the caller warn about +// the legacy fallback without re-reading the environment. +func PublicURLFromEnv() (string, PublicURLSource) { + if v := strings.TrimSpace(os.Getenv(EnvPublicURL)); v != "" { + return v, PublicURLSourceCanonical + } + if v := strings.TrimSpace(os.Getenv(EnvWorkerURLLegacy)); v != "" { + return v, PublicURLSourceLegacy + } + return "", PublicURLSourceNone } // IsAuthConfigured reports whether both WORKER_LOGIN and WORKER_PASSWORD @@ -91,7 +136,9 @@ func ConfigFromEnv() Config { // HTTPConfigFromEnv reads HTTP listener settings from the environment. // Empty WORKER_HOST defaults to DefaultHTTPHost; empty WORKER_PORT defaults // to DefaultHTTPPort. A malformed WORKER_PORT falls back to the default. -// URL is parsed loosely here; ValidateHTTPConfig does the real check. +// The public origin is resolved from PUBLIC_URL (canonical) with the +// deprecated WORKER_URL as the migration fallback; it is parsed loosely +// here — ValidateHTTPConfig does the real check. func HTTPConfigFromEnv() HTTPConfig { port := DefaultHTTPPort if raw := strings.TrimSpace(os.Getenv("WORKER_PORT")); raw != "" { @@ -103,16 +150,18 @@ func HTTPConfigFromEnv() HTTPConfig { if host == "" { host = DefaultHTTPHost } + publicURL, source := PublicURLFromEnv() return HTTPConfig{ - Host: host, - Port: port, - URL: strings.TrimSpace(os.Getenv("WORKER_URL")), - Login: os.Getenv("WORKER_LOGIN"), - Password: os.Getenv("WORKER_PASSWORD"), + Host: host, + Port: port, + PublicURL: publicURL, + URLSource: source, + Login: os.Getenv("WORKER_LOGIN"), + Password: os.Getenv("WORKER_PASSWORD"), } } -// ValidateHTTPConfig enforces the Task 2 invariants: +// ValidateHTTPConfig enforces the local HTTP-listener invariants: // // - WORKER_LOGIN and WORKER_PASSWORD must be both set or both empty. // A mixed state (XOR) is a config bug and must fail fast so an @@ -121,12 +170,15 @@ func HTTPConfigFromEnv() HTTPConfig { // - When the listener would actually start (WORKER_PORT > 0), both // must be set; otherwise we are going to expose an unauthenticated // endpoint. -// - WORKER_URL, if set, must be a parseable absolute URL. Relative -// URLs are rejected because peer workers and the main app need a -// concrete origin to dial. -// - A WORKER_URL with scheme=http on a non-loopback host logs a -// warning: production deployments normally terminate TLS at a -// reverse proxy (Traefik, nginx). +// - A configured advertised origin must validate. The canonical +// PUBLIC_URL is held to the strict origin rules of ValidatePublicURL +// and, in an explicitly production environment +// (DEPLOY_ENV/RSMON_ENV/GO_ENV = production), plain HTTP on a +// non-loopback host is rejected. A legacy WORKER_URL (URLSource is +// PublicURLSourceLegacy) is held only to the tolerant +// ValidateAdvertisedURL so shapes that previously ran keep running; +// it never fails production startup, preserving the historical +// warn-only behavior. func ValidateHTTPConfig(c HTTPConfig, willListen bool) error { loginSet := c.Login != "" passSet := c.Password != "" @@ -138,23 +190,103 @@ func ValidateHTTPConfig(c HTTPConfig, willListen bool) error { if willListen && !c.IsAuthConfigured() { return fmt.Errorf("HTTP listener refused to start: WORKER_LOGIN and WORKER_PASSWORD must be set when WORKER_PORT > 0") } - if c.URL == "" { + if c.PublicURL == "" { return nil } - u, err := url.Parse(c.URL) - if err != nil { - return fmt.Errorf("WORKER_URL is not a valid URL: %v", err) + if c.URLSource == PublicURLSourceLegacy { + if err := ValidateAdvertisedURL(c.PublicURL); err != nil { + return fmt.Errorf("%s: %w", EnvWorkerURLLegacy, err) + } + return nil } - if u.Scheme == "" || u.Host == "" { - return fmt.Errorf("WORKER_URL must be an absolute URL with scheme and host (got %q)", c.URL) + if err := ValidatePublicURL(c.PublicURL); err != nil { + return err } - if u.Scheme != schemeHTTP && u.Scheme != schemeHTTPS { - return fmt.Errorf("WORKER_URL scheme must be http or https (got %q)", u.Scheme) + if productionEnv() { + if host, warn := WarnInsecurePublicURL(c.PublicURL); warn { + return fmt.Errorf( + "%s uses plain HTTP on a non-loopback host (%s), which is not permitted in production; front %s with a TLS-terminating reverse proxy", + EnvPublicURL, host, EnvPublicURL, + ) + } } return nil } -// WarnInsecurePublicURL logs a warning when WORKER_URL uses plain http +// ValidatePublicURL enforces the strict origin shape required by +// docs/public-endpoint-and-identity.md ("One Worker, One Public URL") +// for the canonical PUBLIC_URL configuration: an absolute http(s) URL +// with a scheme and authority and nothing else. Userinfo, query, +// fragment, any path other than "/", and a missing hostname (for example +// "https://:27401") are rejected because every route lives on the origin +// root and credentials must not leak into the advertised endpoint. +func ValidatePublicURL(raw string) error { + u, err := parseURL(raw, EnvPublicURL) + if err != nil { + return err + } + if u.User != nil { + return fmt.Errorf("%s must not contain userinfo (got %q)", EnvPublicURL, raw) + } + if u.RawQuery != "" { + return fmt.Errorf("%s must not contain a query (got %q)", EnvPublicURL, raw) + } + if u.Fragment != "" { + return fmt.Errorf("%s must not contain a fragment (got %q)", EnvPublicURL, raw) + } + if u.Path != "" && u.Path != "/" { + return fmt.Errorf("%s must not contain a path (got %q)", EnvPublicURL, raw) + } + return nil +} + +// ValidateAdvertisedURL enforces the tolerant absolute-URL shape used +// for the legacy WORKER_URL and for endpoints supplied by the control +// plane: an http(s) URL with a scheme, a host, and a non-empty hostname. +// Unlike ValidatePublicURL it does not reject a path, userinfo, query, +// or fragment, because legacy configurations and old control planes may +// carry such shapes and must not newly fail. It still rejects values +// that could never be dialed, such as "https://:27401". +func ValidateAdvertisedURL(raw string) error { + _, err := parseURL(raw, EnvWorkerURLLegacy) + return err +} + +// parseURL parses an absolute http(s) advertised URL. Both validators +// share the scheme/host/hostname rules; label names the source in error +// messages so diagnostics point at the variable the operator set. +func parseURL(raw, label string) (*url.URL, error) { + u, err := url.Parse(raw) + if err != nil { + return nil, fmt.Errorf("%s is not a valid URL: %v", label, err) + } + if u.Scheme == "" || u.Host == "" { + return nil, fmt.Errorf("%s must be an absolute URL with scheme and host (got %q)", label, raw) + } + if u.Scheme != schemeHTTP && u.Scheme != schemeHTTPS { + return nil, fmt.Errorf("%s scheme must be http or https (got %q)", label, u.Scheme) + } + if u.Hostname() == "" { + return nil, fmt.Errorf("%s must include a host (got %q)", label, raw) + } + return u, nil +} + +// productionEnv reports whether the worker is explicitly configured for +// production. Only an explicit literal enables the HTTPS-only PUBLIC_URL +// policy; an unset variable keeps the historical warn-on-plain-HTTP +// behavior so existing installations upgrade without surprise. +func productionEnv() bool { + for _, key := range []string{"DEPLOY_ENV", "RSMON_ENV", "GO_ENV"} { + switch strings.ToLower(strings.TrimSpace(os.Getenv(key))) { + case "production", "prod": + return true + } + } + return false +} + +// WarnInsecurePublicURL logs a warning when PUBLIC_URL uses plain http // for a non-loopback host. Returns the host so the caller can log it. // A no-op for the loopback case (typical local dev) and for https URLs. func WarnInsecurePublicURL(rawURL string) (host string, shouldWarn bool) { diff --git a/internal/distworker/config_test.go b/internal/distworker/config_test.go index 78fb1a3..79b2949 100644 --- a/internal/distworker/config_test.go +++ b/internal/distworker/config_test.go @@ -9,12 +9,13 @@ import ( ) // TestHTTPConfigFromEnv_Defaults verifies the documented defaults when no -// HTTP-related env vars are set: 0.0.0.0:27401 and empty URL / login / -// password. The default port was bumped from 7401 to 27401 to avoid +// HTTP-related env vars are set: 0.0.0.0:27401 and empty public URL / +// login / password. The default port was bumped from 7401 to 27401 to avoid // colliding with the main RSMon app when the worker is co-located. func TestHTTPConfigFromEnv_Defaults(t *testing.T) { t.Setenv("WORKER_HOST", "") t.Setenv("WORKER_PORT", "") + t.Setenv("PUBLIC_URL", "") t.Setenv("WORKER_URL", "") t.Setenv("WORKER_LOGIN", "") t.Setenv("WORKER_PASSWORD", "") @@ -22,7 +23,7 @@ func TestHTTPConfigFromEnv_Defaults(t *testing.T) { cfg := HTTPConfigFromEnv() assert.Equal(t, DefaultHTTPHost, cfg.Host, "host should default to 0.0.0.0") assert.Equal(t, DefaultHTTPPort, cfg.Port, "port should default to 27401") - assert.Equal(t, "", cfg.URL) + assert.Equal(t, "", cfg.PublicURL) assert.Equal(t, "", cfg.Login) assert.Equal(t, "", cfg.Password) assert.False(t, cfg.IsAuthConfigured()) @@ -33,14 +34,14 @@ func TestHTTPConfigFromEnv_Defaults(t *testing.T) { func TestHTTPConfigFromEnv_Overrides(t *testing.T) { t.Setenv("WORKER_HOST", "127.0.0.1") t.Setenv("WORKER_PORT", "9100") - t.Setenv("WORKER_URL", "https://worker.example.com") + t.Setenv("PUBLIC_URL", "https://worker.example.com") t.Setenv("WORKER_LOGIN", "ops") t.Setenv("WORKER_PASSWORD", "s3cret") cfg := HTTPConfigFromEnv() assert.Equal(t, "127.0.0.1", cfg.Host) assert.Equal(t, 9100, cfg.Port) - assert.Equal(t, "https://worker.example.com", cfg.URL) + assert.Equal(t, "https://worker.example.com", cfg.PublicURL) assert.Equal(t, "ops", cfg.Login) assert.Equal(t, "s3cret", cfg.Password) assert.True(t, cfg.IsAuthConfigured()) @@ -70,6 +71,44 @@ func TestHTTPConfigFromEnv_PortOutOfRangeFallsBackToDefault(t *testing.T) { } } +// TestPublicURLFromEnv_PublicURLWins pins the canonical-source rule: +// when both PUBLIC_URL and the legacy WORKER_URL are set, PUBLIC_URL wins +// and the source is reported as canonical. +func TestPublicURLFromEnv_PublicURLWins(t *testing.T) { + t.Setenv("PUBLIC_URL", "https://canonical.example.com") + t.Setenv("WORKER_URL", "http://legacy.example.com:27401") + + url, source := PublicURLFromEnv() + assert.Equal(t, "https://canonical.example.com", url) + assert.Equal(t, PublicURLSourceCanonical, source) + + cfg := HTTPConfigFromEnv() + assert.Equal(t, "https://canonical.example.com", cfg.PublicURL) +} + +// TestPublicURLFromEnv_LegacyFallback verifies the bounded migration: the +// legacy WORKER_URL is resolved when PUBLIC_URL is unset and the source is +// reported as legacy so the caller can emit the deprecation warning. +func TestPublicURLFromEnv_LegacyFallback(t *testing.T) { + t.Setenv("PUBLIC_URL", "") + t.Setenv("WORKER_URL", "https://legacy.example.com") + + url, source := PublicURLFromEnv() + assert.Equal(t, "https://legacy.example.com", url) + assert.Equal(t, PublicURLSourceLegacy, source) +} + +// TestPublicURLFromEnv_None verifies that no configured origin reports the +// none source and an empty value. +func TestPublicURLFromEnv_None(t *testing.T) { + t.Setenv("PUBLIC_URL", "") + t.Setenv("WORKER_URL", "") + + url, source := PublicURLFromEnv() + assert.Equal(t, "", url) + assert.Equal(t, PublicURLSourceNone, source) +} + // TestValidateHTTPConfig_LoginXORPasswordRejected covers the central // invariant: a mixed login/password state is a config bug and must // fail fast. @@ -130,7 +169,7 @@ func TestValidateHTTPConfig_URLAbsoluteRequired(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - cfg := HTTPConfig{Host: "0.0.0.0", Port: 27401, URL: tc.url} + cfg := HTTPConfig{Host: "0.0.0.0", Port: 27401, PublicURL: tc.url} err := ValidateHTTPConfig(cfg, false) // empty url is allowed if tc.url == "" { @@ -148,7 +187,7 @@ func TestValidateHTTPConfig_URLSchemeAllowed(t *testing.T) { t.Run(scheme, func(t *testing.T) { cfg := HTTPConfig{ Host: "0.0.0.0", Port: 27401, - URL: scheme + "://localhost:27401", + PublicURL: scheme + "://localhost:27401", } assert.NoError(t, ValidateHTTPConfig(cfg, false)) }) @@ -161,7 +200,7 @@ func TestValidateHTTPConfig_URLSchemeRejected(t *testing.T) { t.Run(scheme, func(t *testing.T) { cfg := HTTPConfig{ Host: "0.0.0.0", Port: 27401, - URL: scheme + "://localhost:27401", + PublicURL: scheme + "://localhost:27401", } err := ValidateHTTPConfig(cfg, false) require.Error(t, err) @@ -170,6 +209,151 @@ func TestValidateHTTPConfig_URLSchemeRejected(t *testing.T) { } } +// TestValidatePublicURL_OriginShapeRejected covers the plan's negative +// cases: the advertised origin must be scheme + authority only, with no +// userinfo, query, fragment, or ambiguous path. +func TestValidatePublicURL_OriginShapeRejected(t *testing.T) { + cases := []struct { + name string + url string + want string + }{ + {"userinfo", "https://user:pass@worker.example.com", "userinfo"}, + {"query", "https://worker.example.com?x=1", "query"}, + {"fragment", "https://worker.example.com#frag", "fragment"}, + {"path", "https://worker.example.com/web", "path"}, + {"path nested", "https://worker.example.com/raft/", "path"}, + {"missing scheme", "worker.example.com", "absolute url"}, + {"missing host", "https://", "absolute url"}, + {"bad scheme", "ftp://worker.example.com", "scheme must be http or https"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := ValidatePublicURL(tc.url) + require.Error(t, err) + assert.Contains(t, strings.ToLower(err.Error()), tc.want) + assert.Contains(t, err.Error(), EnvPublicURL, + "error must name the canonical PUBLIC_URL variable") + }) + } +} + +// TestValidatePublicURL_RootSlashAllowed confirms that an empty path and +// the root path "/" are both accepted as an origin. +func TestValidatePublicURL_RootSlashAllowed(t *testing.T) { + for _, u := range []string{"https://worker.example.com", "https://worker.example.com/"} { + assert.NoError(t, ValidatePublicURL(u)) + } +} + +// TestValidatePublicURL_HostnameRequired pins the hostname rule: an +// authority that parses to no hostname (for example "https://:27401") +// must be rejected even though it has a host:port string. +func TestValidatePublicURL_HostnameRequired(t *testing.T) { + for _, u := range []string{"https://:27401", "http://:7401"} { + t.Run(u, func(t *testing.T) { + err := ValidatePublicURL(u) + require.Error(t, err) + assert.Contains(t, strings.ToLower(err.Error()), "must include a host") + }) + } +} + +// TestValidateAdvertisedURL_Tolerant pins the legacy-tolerant shape used +// for WORKER_URL and control-plane-supplied endpoints: an absolute +// http(s) URL with a scheme, host, and hostname. Paths, userinfo, query, +// and fragments that previously ran must keep validating; only values +// that could never be dialed (relative, bad scheme, missing hostname) +// are rejected. +func TestValidateAdvertisedURL_Tolerant(t *testing.T) { + for _, u := range []string{ + "https://worker.example.com", + "https://worker.example.com/web", + "https://worker.example.com/raft/", + "http://localhost:27401", + "https://user:pass@worker.example.com/web?x=1#frag", + } { + assert.NoError(t, ValidateAdvertisedURL(u), "tolerant validator must accept %q", u) + } + for _, u := range []string{ + "worker.example.com", + "https://", + "https://:27401", + "ftp://worker.example.com", + "file:///tmp/x", + } { + assert.Error(t, ValidateAdvertisedURL(u), "tolerant validator must reject %q", u) + } +} + +// TestValidateHTTPConfig_LegacySourceTolerant is the core bounded-migration +// guarantee: a legacy WORKER_URL that previously ran (path, userinfo, even +// plain HTTP in production) must not newly fail startup. Only genuinely +// unusable values are rejected. +func TestValidateHTTPConfig_LegacySourceTolerant(t *testing.T) { + t.Setenv("DEPLOY_ENV", "production") + + for _, u := range []string{ + "https://worker.example.com", + "https://worker.example.com/web", + "http://worker.example.com", + } { + cfg := HTTPConfig{Host: "0.0.0.0", Port: 27401, PublicURL: u, URLSource: PublicURLSourceLegacy} + assert.NoError(t, ValidateHTTPConfig(cfg, false), + "legacy WORKER_URL=%q must not newly fail startup", u) + } + + cfg := HTTPConfig{Host: "0.0.0.0", Port: 27401, PublicURL: "https://:27401", URLSource: PublicURLSourceLegacy} + err := ValidateHTTPConfig(cfg, false) + require.Error(t, err) + assert.Contains(t, err.Error(), EnvWorkerURLLegacy, + "legacy diagnostics must name the WORKER_URL source") +} + +// TestValidateHTTPConfig_CanonicalSourceStrict verifies the canonical +// PUBLIC_URL stays strict even when the same value would be tolerated as +// a legacy WORKER_URL. +func TestValidateHTTPConfig_CanonicalSourceStrict(t *testing.T) { + cfg := HTTPConfig{Host: "0.0.0.0", Port: 27401, PublicURL: "https://worker.example.com/web", URLSource: PublicURLSourceCanonical} + err := ValidateHTTPConfig(cfg, false) + require.Error(t, err) + assert.Contains(t, err.Error(), EnvPublicURL, + "canonical diagnostics must name the PUBLIC_URL source") +} + +// TestValidateHTTPConfig_ProductionRejectsPlainHTTP pins the production +// HTTPS policy for the canonical PUBLIC_URL: an explicitly production +// worker must not advertise plain HTTP on a non-loopback host. Loopback +// and https stay accepted. +func TestValidateHTTPConfig_ProductionRejectsPlainHTTP(t *testing.T) { + t.Setenv("DEPLOY_ENV", "production") + + err := ValidateHTTPConfig(HTTPConfig{ + Host: "0.0.0.0", Port: 27401, + PublicURL: "http://worker.example.com", + URLSource: PublicURLSourceCanonical, + }, false) + require.Error(t, err) + assert.Contains(t, err.Error(), "not permitted in production") + + cfg := HTTPConfig{Host: "0.0.0.0", Port: 27401, PublicURL: "https://worker.example.com", URLSource: PublicURLSourceCanonical} + assert.NoError(t, ValidateHTTPConfig(cfg, false)) + + cfg = HTTPConfig{Host: "0.0.0.0", Port: 27401, PublicURL: "http://localhost:27401", URLSource: PublicURLSourceCanonical} + assert.NoError(t, ValidateHTTPConfig(cfg, false)) +} + +// TestValidateHTTPConfig_NonProductionAllowsPlainHTTP keeps the historical +// warn-only behavior when no explicit production environment is configured. +func TestValidateHTTPConfig_NonProductionAllowsPlainHTTP(t *testing.T) { + t.Setenv("DEPLOY_ENV", "") + t.Setenv("RSMON_ENV", "development") + t.Setenv("GO_ENV", "") + + cfg := HTTPConfig{Host: "0.0.0.0", Port: 27401, PublicURL: "http://worker.example.com"} + assert.NoError(t, ValidateHTTPConfig(cfg, false)) +} + // TestWarnInsecurePublicURL exercises the warning helper: only http on // non-loopback hosts should warn. The returned host is the parsed host // (host:port when present) so the caller can log a useful target. diff --git a/internal/distworker/runner.go b/internal/distworker/runner.go index a123000..3e1f554 100644 --- a/internal/distworker/runner.go +++ b/internal/distworker/runner.go @@ -990,7 +990,24 @@ func (r *Runner) applyInit(init *wire.WorkerInit) { r.systemContactsMu.Unlock() r.urlMu.Lock() - r.url = init.URL + accepted, field := init.PublicURL, "public_url" + if accepted == "" { + accepted, field = init.URL, "url" // legacy wire field (bounded migration) + } + // Validate the control-plane-supplied endpoint before accepting it. + // On an unusable value keep the previous accepted URL (never regress + // to a blank or garbage endpoint) and surface the rejection. A valid + // empty value still clears the stored URL. + var err error + if accepted != "" { + err = ValidateAdvertisedURL(accepted) + } + if err != nil { + log.Printf("worker: ignoring invalid advertised URL from control plane (%s=%q): %v", + field, accepted, err) + } else { + r.url = accepted + } r.urlMu.Unlock() // Peers is the slice of other workers this node can reach for diff --git a/internal/distworker/runner_test.go b/internal/distworker/runner_test.go index 27042a0..1dc9449 100644 --- a/internal/distworker/runner_test.go +++ b/internal/distworker/runner_test.go @@ -1149,3 +1149,59 @@ func TestApplyInitStoresURLInMemory(t *testing.T) { assert.Equal(t, "", r.URL(), "URL() must return empty after applyInit with empty URL") } + +// TestApplyInitPrefersPublicURLOverLegacyURL verifies the bounded-migration +// precedence on the init/config frame: PublicURL (canonical) wins whenever +// it is non-empty, and the legacy URL field remains the fallback for old +// control planes. +func TestApplyInitPrefersPublicURLOverLegacyURL(t *testing.T) { + executor := func(payload interface{}) interface{} { + return []wire.CheckResultReport{} + } + r := newTestRunner(t, 4, 1, executor) + + r.applyInit(&wire.WorkerInit{ + WorkerID: "w-1", + Concurrency: 2, + PublicURL: "https://canonical.example.com", + URL: "https://legacy.example.com", + }) + assert.Equal(t, "https://canonical.example.com", r.URL(), + "PublicURL must win over the legacy URL field") + + r.applyInit(&wire.WorkerInit{ + WorkerID: "w-1", + Concurrency: 2, + URL: "https://legacy.example.com", + }) + assert.Equal(t, "https://legacy.example.com", r.URL(), + "legacy URL field must be used when PublicURL is empty") +} + +// TestApplyInitInvalidAcceptedURLKeepsPrior verifies the safe-fallback +// behavior when the control plane supplies an unusable advertised URL: +// the previous accepted value is kept (never regressed to a garbage +// endpoint), while a valid empty init still clears it. +func TestApplyInitInvalidAcceptedURLKeepsPrior(t *testing.T) { + executor := func(payload interface{}) interface{} { + return []wire.CheckResultReport{} + } + r := newTestRunner(t, 4, 1, executor) + + r.applyInit(&wire.WorkerInit{WorkerID: "w-1", Concurrency: 2, PublicURL: "https://worker.example.com"}) + assert.Equal(t, "https://worker.example.com", r.URL()) + + // Invalid value: keep the previous accepted URL. + r.applyInit(&wire.WorkerInit{WorkerID: "w-1", Concurrency: 2, PublicURL: "https://:27401"}) + assert.Equal(t, "https://worker.example.com", r.URL(), + "invalid accepted URL must not replace the stored value") + + r.applyInit(&wire.WorkerInit{WorkerID: "w-1", Concurrency: 2, URL: "not a url"}) + assert.Equal(t, "https://worker.example.com", r.URL(), + "invalid legacy url field must not replace the stored value") + + // Valid empty init clears, as before. + r.applyInit(&wire.WorkerInit{WorkerID: "w-1", Concurrency: 2}) + assert.Equal(t, "", r.URL(), + "valid empty init must clear the stored URL") +} diff --git a/internal/installer/install.go b/internal/installer/install.go index d855b13..1e25c79 100644 --- a/internal/installer/install.go +++ b/internal/installer/install.go @@ -15,6 +15,8 @@ import ( "unicode" "github.com/joho/godotenv" + + "rocketgit.ru/rsmon/worker/internal/distworker" ) var ( @@ -34,11 +36,16 @@ const ( // installEnvKeys is the canonical, ordered set of worker environment // variables the installer understands and writes to the unit's env // file. Order matters: the rendered file is stable and readable. +// +// PUBLIC_URL is the canonical advertised origin. WORKER_URL stays in the +// list for the bounded migration so legacy env files still resolve; it +// is dropped from the written file whenever PUBLIC_URL is also present. var installEnvKeys = []string{ "RSMON_URL", "RSMON_TOKEN", "WORKER_HOST", "WORKER_PORT", + "PUBLIC_URL", "WORKER_URL", "WORKER_LOGIN", "WORKER_PASSWORD", @@ -56,18 +63,19 @@ var installEnvKeys = []string{ // installs a co-located worker under rsmon-worker- with its own // binary path, config dir, data dir, systemd unit, and port. type InstallOptions struct { - Binary string - EnvFile string - Token string - URL string - Host string - Port string - Login string - Password string - Name string - Docker bool - Image string - NoStart bool + Binary string + EnvFile string + Token string + URL string + PublicURL string + Host string + Port string + Login string + Password string + Name string + Docker bool + Image string + NoStart bool } // paths is the fully-resolved on-disk layout for an instance. Every @@ -258,6 +266,7 @@ func resolveInstallEnv(opts InstallOptions, name string, fileEnv map[string]stri flagVals := map[string]string{ "RSMON_URL": opts.URL, "RSMON_TOKEN": opts.Token, + "PUBLIC_URL": opts.PublicURL, "WORKER_HOST": opts.Host, "WORKER_PORT": opts.Port, "WORKER_LOGIN": opts.Login, @@ -312,6 +321,26 @@ func resolveInstallEnv(opts InstallOptions, name string, fileEnv map[string]stri return nil, fmt.Errorf("WORKER_LOGIN and WORKER_PASSWORD must both be set or both be empty") } + // PUBLIC_URL is the canonical advertised origin. When both the + // canonical and the legacy WORKER_URL resolve, the legacy variable + // is superseded and must not be written to a fresh env file. + if values["PUBLIC_URL"] != "" { + delete(values, "WORKER_URL") + } + // Canonical PUBLIC_URL is held to the strict origin shape; the + // legacy WORKER_URL only to the tolerant absolute-URL check so env + // files that previously installed keep working. + if v := values["PUBLIC_URL"]; v != "" { + if err := distworker.ValidatePublicURL(v); err != nil { + return nil, fmt.Errorf("PUBLIC_URL: %w", err) + } + } + if v := values["WORKER_URL"]; v != "" { + if err := distworker.ValidateAdvertisedURL(v); err != nil { + return nil, fmt.Errorf("WORKER_URL: %w", err) + } + } + // Validate every value we will write is systemd/docker safe. for _, key := range installEnvKeys { v, ok := values[key] diff --git a/internal/installer/install_test.go b/internal/installer/install_test.go index 3645f6c..01693b7 100644 --- a/internal/installer/install_test.go +++ b/internal/installer/install_test.go @@ -90,6 +90,131 @@ func TestValidateEnvironmentFileInputErrors(t *testing.T) { } } +// TestResolveInstallEnvPublicURLWins verifies the installer canonicalizes +// the advertised origin: PUBLIC_URL is written and the legacy WORKER_URL +// is dropped from the resolved env when both are present. +func TestResolveInstallEnvPublicURLWins(t *testing.T) { + v, err := resolveInstallEnv(InstallOptions{}, "", map[string]string{ + "RSMON_URL": "https://rsmon.ru", + "RSMON_TOKEN": "secret", + "PUBLIC_URL": "https://worker.example.com", + "WORKER_URL": "http://legacy.example.com", + }) + if err != nil { + t.Fatal(err) + } + if v["PUBLIC_URL"] != "https://worker.example.com" { + t.Fatalf("PUBLIC_URL not resolved: %+v", v) + } + if _, ok := v["WORKER_URL"]; ok { + t.Fatalf("legacy WORKER_URL must be dropped when PUBLIC_URL is set: %+v", v) + } +} + +// TestResolveInstallEnvLegacyWorkerURLPassesThrough keeps the bounded +// migration: an env file that only carries the legacy WORKER_URL still +// resolves and is written unchanged so existing installs upgrade in place. +func TestResolveInstallEnvLegacyWorkerURLPassesThrough(t *testing.T) { + v, err := resolveInstallEnv(InstallOptions{}, "", map[string]string{ + "RSMON_URL": "https://rsmon.ru", + "RSMON_TOKEN": "secret", + "WORKER_URL": "https://legacy.example.com", + }) + if err != nil { + t.Fatal(err) + } + if v["WORKER_URL"] != "https://legacy.example.com" { + t.Fatalf("legacy WORKER_URL not preserved: %+v", v) + } + if v["PUBLIC_URL"] != "" { + t.Fatalf("PUBLIC_URL must stay empty: %+v", v) + } +} + +// TestResolveInstallEnvLegacyWorkerURLTolerant verifies the bounded +// migration does not newly reject legacy shapes that previously +// installed (a path-bearing WORKER_URL) while a path-bearing PUBLIC_URL +// stays strict. +func TestResolveInstallEnvLegacyWorkerURLTolerant(t *testing.T) { + v, err := resolveInstallEnv(InstallOptions{}, "", map[string]string{ + "RSMON_URL": "https://rsmon.ru", + "RSMON_TOKEN": "secret", + "WORKER_URL": "https://legacy.example.com/web", + }) + if err != nil { + t.Fatalf("legacy WORKER_URL with a path must keep installing: %v", err) + } + if v["WORKER_URL"] != "https://legacy.example.com/web" { + t.Fatalf("legacy WORKER_URL not preserved: %+v", v) + } + + _, err = resolveInstallEnv(InstallOptions{}, "", map[string]string{ + "RSMON_URL": "https://rsmon.ru", + "RSMON_TOKEN": "secret", + "PUBLIC_URL": "https://worker.example.com/web", + }) + if err == nil { + t.Fatal("path-bearing canonical PUBLIC_URL must be rejected") + } +} + +// TestResolveInstallEnvPublicURLFlagBeatsEnv verifies the --public-url +// flag follows the installer precedence: the flag wins over the env file +// and the legacy WORKER_URL is dropped when PUBLIC_URL is present. +func TestResolveInstallEnvPublicURLFlagBeatsEnv(t *testing.T) { + v, err := resolveInstallEnv(InstallOptions{PublicURL: "https://flag.example.com"}, "", map[string]string{ + "RSMON_URL": "https://rsmon.ru", + "RSMON_TOKEN": "secret", + "PUBLIC_URL": "https://file.example.com", + "WORKER_URL": "https://legacy.example.com", + }) + if err != nil { + t.Fatal(err) + } + if v["PUBLIC_URL"] != "https://flag.example.com" { + t.Fatalf("--public-url flag must win: %+v", v) + } + if _, ok := v["WORKER_URL"]; ok { + t.Fatalf("legacy WORKER_URL must be dropped when PUBLIC_URL is set: %+v", v) + } +} + +// TestResolveInstallEnvRejectsMalformedPublicURL verifies the installer +// rejects an advertised origin that violates the plan's origin shape +// (path, userinfo, and non-http(s) schemes). +func TestResolveInstallEnvRejectsMalformedPublicURL(t *testing.T) { + for _, bad := range []string{ + "https://worker.example.com/web", + "https://user:pass@worker.example.com", + "ftp://worker.example.com", + "worker.example.com", + } { + t.Run(bad, func(t *testing.T) { + _, err := resolveInstallEnv(InstallOptions{}, "", map[string]string{ + "RSMON_URL": "https://rsmon.ru", + "RSMON_TOKEN": "secret", + "PUBLIC_URL": bad, + }) + if err == nil { + t.Fatalf("PUBLIC_URL=%q accepted", bad) + } + }) + } +} + +// TestRenderEnvFileOrder includes the canonical PUBLIC_URL ordering. +func TestRenderEnvFilePublicURLEmptyOmitted(t *testing.T) { + got := string(renderEnvFile(map[string]string{ + "RSMON_URL": "https://rsmon.ru", + "RSMON_TOKEN": "secret", + "PUBLIC_URL": "", + "WORKER_URL": "", + })) + if strings.Contains(got, "PUBLIC_URL=") || strings.Contains(got, "WORKER_URL=") { + t.Fatalf("empty public URL keys must be omitted: %q", got) + } +} + func TestEnvironment(t *testing.T) { got := string(Environment("https://example.test", "secret")) for _, want := range []string{"RSMON_URL=https://example.test\n", "RSMON_TOKEN=secret\n", "WORKER_HOST=127.0.0.1\n"} { diff --git a/internal/wire/types.go b/internal/wire/types.go index 88e0190..23d0fce 100644 --- a/internal/wire/types.go +++ b/internal/wire/types.go @@ -4,10 +4,15 @@ package wire import "encoding/json" // RegisterRequest is sent to the control plane registration API. +// PublicURL is the canonical advertised origin (scheme and authority +// only, see docs/public-endpoint-and-identity.md). The legacy URL field +// remains on the wire for the bounded migration from WORKER_URL; new +// senders populate PublicURL and old control planes keep reading URL. type RegisterRequest struct { WorkerID string `json:"worker_id" binding:"required"` RegionCode string `json:"region_code" binding:"required"` Version string `json:"version"` + PublicURL string `json:"public_url,omitempty"` URL string `json:"url,omitempty"` Capabilities []string `json:"capabilities"` TaskEnvelope bool `json:"task_envelope"` @@ -203,6 +208,9 @@ type PeerInfo struct { } // WorkerInit is sent by the control plane after websocket authentication. +// PublicURL is the canonical advertised origin accepted by the control +// plane; URL is the legacy wire field kept for the bounded migration. +// A worker must prefer PublicURL when it is non-empty. type WorkerInit struct { WorkerID string `json:"worker_id"` RegionCode string `json:"region_code"` @@ -211,6 +219,7 @@ type WorkerInit struct { NotificationMethods []string `json:"notification_methods,omitempty"` NotificationAccounts []int64 `json:"notification_accounts,omitempty"` Concurrency int `json:"concurrency"` + PublicURL string `json:"public_url,omitempty"` URL string `json:"url,omitempty"` ServerID *int64 `json:"server_id,omitempty"` LLMs []LLMConfig `json:"llms,omitempty"` diff --git a/internal/wire/types_test.go b/internal/wire/types_test.go index 914e883..2967583 100644 --- a/internal/wire/types_test.go +++ b/internal/wire/types_test.go @@ -151,6 +151,82 @@ func TestRegisterRequest_URLOmittedWhenEmpty(t *testing.T) { require.NoError(t, err) assert.False(t, strings.Contains(string(data), `"url"`), "empty URL must be omitted from the register payload, got %s", string(data)) + assert.False(t, strings.Contains(string(data), `"public_url"`), + "empty PublicURL must be omitted from the register payload, got %s", string(data)) +} + +// TestRegisterRequest_PublicURLRoundTrip verifies the canonical public_url +// wire field serializes and deserializes alongside the legacy url field. +func TestRegisterRequest_PublicURLRoundTrip(t *testing.T) { + req := RegisterRequest{ + WorkerID: "worker-eu-1", + RegionCode: "eu", + Version: "v1", + PublicURL: "https://worker-eu.example.com", + } + + data, err := json.Marshal(req) + require.NoError(t, err) + assert.Contains(t, string(data), `"public_url":"https://worker-eu.example.com"`, + "PublicURL must serialize as the top-level public_url field, got %s", string(data)) + + var decoded RegisterRequest + require.NoError(t, json.Unmarshal(data, &decoded)) + assert.Equal(t, "https://worker-eu.example.com", decoded.PublicURL) +} + +// TestWorkerInit_PublicURLRoundTrip covers the canonical public_url field +// on the init/config frame. +func TestWorkerInit_PublicURLRoundTrip(t *testing.T) { + init := WorkerInit{ + WorkerID: "worker-1", + RegionCode: "ru", + Version: "v1", + Concurrency: 4, + PublicURL: "https://worker-eu.example.com", + } + + data, err := json.Marshal(init) + require.NoError(t, err) + assert.Contains(t, string(data), `"public_url":"https://worker-eu.example.com"`, + "PublicURL must serialize as a top-level public_url field, got %s", string(data)) + + var decoded WorkerInit + require.NoError(t, json.Unmarshal(data, &decoded)) + assert.Equal(t, "https://worker-eu.example.com", decoded.PublicURL) + assert.Equal(t, "", decoded.URL, + "legacy URL field must stay empty when only public_url is set") +} + +// TestWorkerInit_LegacyURLFieldStillDecodes keeps the bounded-migration +// contract: a control plane that still sends the legacy url field must +// remain wire-compatible with the new worker struct. +func TestWorkerInit_LegacyURLFieldStillDecodes(t *testing.T) { + data := []byte(`{"worker_id":"worker-1","region_code":"ru","concurrency":4,"url":"https://legacy.example.com"}`) + var decoded WorkerInit + require.NoError(t, json.Unmarshal(data, &decoded)) + assert.Equal(t, "https://legacy.example.com", decoded.URL) + assert.Equal(t, "", decoded.PublicURL) +} + +// TestWorkerInit_PublicURLAndLegacyCoexist ensures a frame that carries +// both fields keeps both on the wire for old and new control planes. +func TestWorkerInit_PublicURLAndLegacyCoexist(t *testing.T) { + init := WorkerInit{ + WorkerID: "w-1", + PublicURL: "https://canonical.example.com", + URL: "https://legacy.example.com", + } + data, err := json.Marshal(init) + require.NoError(t, err) + out := string(data) + assert.Contains(t, out, `"public_url":"https://canonical.example.com"`) + assert.Contains(t, out, `"url":"https://legacy.example.com"`) + + var decoded WorkerInit + require.NoError(t, json.Unmarshal(data, &decoded)) + assert.Equal(t, "https://canonical.example.com", decoded.PublicURL) + assert.Equal(t, "https://legacy.example.com", decoded.URL) } // TestWorkerInit_NotificationCapabilitiesRoundTrip ensures the diff --git a/packaging/systemd/worker.env.example b/packaging/systemd/worker.env.example index 94b7501..f42c706 100644 --- a/packaging/systemd/worker.env.example +++ b/packaging/systemd/worker.env.example @@ -2,7 +2,9 @@ RSMON_URL=https://rsmon.ru RSMON_TOKEN=replace-with-worker-token WORKER_HOST=127.0.0.1 WORKER_PORT=27401 -WORKER_URL= +# Advertised public origin: absolute http(s) origin (scheme + host, no path). +# PUBLIC_URL is canonical; WORKER_URL is a bounded-migration legacy alias. +PUBLIC_URL= WORKER_LOGIN=admin WORKER_PASSWORD=replace-with-a-long-random-password RSMON_WEBAPP_DATA_DIR=/var/lib/rsmon-worker/webapp