Unverified Commit 7d3be649 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

feat(cfsetup): start the tunnel connector as a setup step

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 <path> 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.
parent 2810fe84
Loading
Loading
Loading
Loading
+16 −0
Changes for internal/cfsetup/cfsetup.go: 16 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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…")
+70 −0
Changes for internal/cfsetup/cfsetup_test.go: 70 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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{"[email protected]"}},
		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{"[email protected]"}},
		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
+24 −0
Changes for internal/cfsetup/runner.go: 24 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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 <configPath> 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) {