From 7d3be649199c9a09a1f89690f4f73c394fcdd20b Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Wed, 1 Jul 2026 15:11:35 +0900 Subject: [PATCH] feat(cfsetup): start the tunnel connector as a setup step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setup created the tunnel, routed DNS, and wrote config.yml, but nothing installed or started a connector for it. A one-click run therefore left the tunnel routed-but-dead: every web hostname returned Cloudflare error 1033 (tunnel has no connector) even though the config was correct on disk. Add a StartConnector step to the Runner seam, invoked right after the config is written (and gated on ConfigPath, so a caller wanting only the Access config is not forced to install a service). The ExecRunner implementation runs `cloudflared --config service install`, which installs and starts a managed system service (systemd/launchd/Windows), and is idempotent on an already-installed service. The orchestration — connector started, and only after its config exists — is unit-tested against the fake Runner; the actual service install is INTEGRATION-ONLY. Together with the RouteDNS --overwrite-dns fix, this closes both distinct paths to a 1033 half-state from a fresh setup: a stale DNS binding and a missing connector. --- internal/cfsetup/cfsetup.go | 16 ++++++++ internal/cfsetup/cfsetup_test.go | 70 ++++++++++++++++++++++++++++++++ internal/cfsetup/runner.go | 24 +++++++++++ 3 files changed, 110 insertions(+) diff --git a/internal/cfsetup/cfsetup.go b/internal/cfsetup/cfsetup.go index aa99610..3c65f93 100644 --- a/internal/cfsetup/cfsetup.go +++ b/internal/cfsetup/cfsetup.go @@ -339,6 +339,13 @@ type Runner interface { RouteDNS(ctx context.Context, tunnelID, hostname string) error // WriteTunnelConfig persists the rendered config.yml. WriteTunnelConfig(path string, contents []byte) error + // StartConnector installs and starts the local cloudflared connector bound to + // the written config so the tunnel actually has a running process serving it. + // Without this step the tunnel is created and the DNS is routed, but nothing + // runs the config — so every routed hostname returns Cloudflare error 1033 + // (tunnel has no connector), the other half of the 1033 failure mode that + // RouteDNS's --overwrite-dns closes. + StartConnector(ctx context.Context, configPath string) error // CreateAccessApplication creates the self-hosted Access app and returns its // id and the issued JWT `aud` (which felis [auth] access_jwt_aud must adopt). CreateAccessApplication(ctx context.Context, app AccessApplication) (appID, aud string, err error) @@ -453,6 +460,15 @@ func Setup(ctx context.Context, runner Runner, p Params) (*Result, error) { return nil, fmt.Errorf("cfsetup: write config: %w", err) } prog = append(prog, "Wrote "+p.ConfigPath) + // 6b. Install and start the connector for the config just written, so the + // tunnel is actually served rather than routed-but-dead (error 1033). + // Guarded by ConfigPath: with no config there is nothing to run, and a + // caller wanting only the Access config is not forced to install a service. + notify("Starting tunnel connector…") + if err := runner.StartConnector(ctx, p.ConfigPath); err != nil { + return nil, fmt.Errorf("cfsetup: start connector: %w", err) + } + prog = append(prog, "Started connector") } // 7. Front the admin face with a self-hosted Access app. notify("Creating Access application…") diff --git a/internal/cfsetup/cfsetup_test.go b/internal/cfsetup/cfsetup_test.go index 3d3b0e2..38af91b 100644 --- a/internal/cfsetup/cfsetup_test.go +++ b/internal/cfsetup/cfsetup_test.go @@ -69,6 +69,11 @@ func (r *recordingRunner) WriteTunnelConfig(path string, _ []byte) error { return nil } +func (r *recordingRunner) StartConnector(_ context.Context, configPath string) error { + r.calls = append(r.calls, "StartConnector:"+configPath) + return nil +} + func (r *recordingRunner) CreateAccessApplication(_ context.Context, app AccessApplication) (string, string, error) { r.calls = append(r.calls, "CreateAccessApplication:"+app.Domain) appID := r.appID @@ -331,6 +336,71 @@ func TestSetupSucceedsAndReportsAud(t *testing.T) { } } +// TestSetupStartsConnectorAfterWritingConfig pins the anti-1033 invariant, not a +// call order for its own sake: a successful Setup that wrote a config MUST also start +// a connector for it (a routed tunnel with no connector returns Cloudflare error +// 1033), and it must do so only AFTER the config exists (starting a connector for an +// unwritten config would serve nothing). This is the orchestration half of the fix; +// the actual `cloudflared service install` is INTEGRATION-ONLY (runner.go). +func TestSetupStartsConnectorAfterWritingConfig(t *testing.T) { + const cfgPath = "/etc/felis/cloudflared.yml" + runner := &recordingRunner{} + p := Params{ + PanelHostname: "console." + testRoot, + AdminHostname: "op.console." + testRoot, + TunnelName: "felis", + ConfigPath: cfgPath, + AccessIdentity: AccessIdentity{Emails: []string{"owner@example.net"}}, + Pre: goodPreconditions(), + } + if _, err := Setup(context.Background(), runner, p); err != nil { + t.Fatalf("Setup: %v", err) + } + write := indexOfCall(runner.calls, "WriteTunnelConfig:"+cfgPath) + start := indexOfCall(runner.calls, "StartConnector:"+cfgPath) + if write < 0 { + t.Fatalf("config was never written: %v", runner.calls) + } + if start < 0 { + t.Fatalf("connector was never started — a routed tunnel with no connector returns error 1033: %v", runner.calls) + } + if start < write { + t.Fatalf("connector started before its config was written (would serve nothing): %v", runner.calls) + } +} + +// TestSetupSkipsConnectorWhenNoConfigPath proves the connector step is gated on a +// written config: with ConfigPath unset the caller wants only the Access config, so +// no service is installed (and no config is written to install one around). +func TestSetupSkipsConnectorWhenNoConfigPath(t *testing.T) { + runner := &recordingRunner{} + p := Params{ + PanelHostname: "console." + testRoot, + AdminHostname: "op.console." + testRoot, + TunnelName: "felis", + AccessIdentity: AccessIdentity{Emails: []string{"owner@example.net"}}, + Pre: goodPreconditions(), + } + if _, err := Setup(context.Background(), runner, p); err != nil { + t.Fatalf("Setup: %v", err) + } + for _, c := range runner.calls { + if len(c) >= len("StartConnector") && c[:len("StartConnector")] == "StartConnector" { + t.Fatalf("connector was started with no config path to serve: %v", runner.calls) + } + } +} + +// indexOfCall returns the position of want in calls, or -1. +func indexOfCall(calls []string, want string) int { + for i, c := range calls { + if c == want { + return i + } + } + return -1 +} + // TestIngressSafetyInvariants parses the generated cloudflared config back and // asserts the properties that protect the origin: the final rule is the // hostname-less fail-shut 404 catch-all, every routed hostname points at the panel diff --git a/internal/cfsetup/runner.go b/internal/cfsetup/runner.go index ee86ddf..8d4074a 100644 --- a/internal/cfsetup/runner.go +++ b/internal/cfsetup/runner.go @@ -171,6 +171,30 @@ func (r *ExecRunner) WriteTunnelConfig(path string, contents []byte) error { return os.WriteFile(path, contents, 0o644) } +// StartConnector installs and starts the cloudflared connector as a managed system +// service bound to configPath (`cloudflared --config service install`), +// so the tunnel written by WriteTunnelConfig has a running process serving it. On +// Linux this installs and starts a systemd unit; on macOS a launchd agent; on +// Windows a service. It is the step that turns a routed-but-dead tunnel (Cloudflare +// error 1033) into a reachable one, and it runs as the operator (root under the +// break-glass TUI) since installing a system service requires it. +// +// It is idempotent on re-run: an already-installed service is reported by cloudflared +// and treated as success rather than failing the whole setup. Re-applying a CHANGED +// config to an already-installed service would need a restart this method does not +// perform — a caveat noted honestly. INTEGRATION-ONLY. +func (r *ExecRunner) StartConnector(ctx context.Context, configPath string) error { + if strings.TrimSpace(configPath) == "" { + return fmt.Errorf("cfsetup: connector config path is required") + } + // The global --config flag must precede the `service install` subcommand. + _, err := r.runCloudflared(ctx, "--config", configPath, "service", "install") + if err != nil && strings.Contains(err.Error(), "already installed") { + return nil + } + return err +} + // CreateAccessApplication POSTs the self-hosted Access app and returns its id and // issued aud (spec §14: the aud felis [auth] access_jwt_aud must adopt). func (r *ExecRunner) CreateAccessApplication(ctx context.Context, app AccessApplication) (string, string, error) {