fix(cfsetup): keep connector install in the host apply layer only
Setup previously called runner.StartConnector (`cloudflared service install`) while the TUI applyCloudflareEdge separately installs cloudflared-felis.service for the same tunnel from the same config -- two managed services serving one tunnel from one connector config. Drop StartConnector from cfsetup: running a connector is a host-specific side effect (systemd/launchd/Windows service) that belongs with the caller, not in this host- and domain-agnostic package whose documented side effects are tunnel creation, DNS routing, and the Access app/policy calls. installCloudflaredService in the host layer stays the single connector installer, so the routed-but-dead 1033 is still closed; RouteDNS --overwrite-dns still closes the stale-DNS 1033.
This commit is contained in:
3 files changed
+7
-110
No files matched your search
@@ -339,13 +339,6 @@ type Runner interface {
|
|||||||
RouteDNS(ctx context.Context, tunnelID, hostname string) error
|
RouteDNS(ctx context.Context, tunnelID, hostname string) error
|
||||||
// WriteTunnelConfig persists the rendered config.yml.
|
// WriteTunnelConfig persists the rendered config.yml.
|
||||||
WriteTunnelConfig(path string, contents []byte) error
|
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
|
// CreateAccessApplication creates the self-hosted Access app and returns its
|
||||||
// id and the issued JWT `aud` (which felis [auth] access_jwt_aud must adopt).
|
// 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)
|
CreateAccessApplication(ctx context.Context, app AccessApplication) (appID, aud string, err error)
|
||||||
@@ -460,15 +453,13 @@ func Setup(ctx context.Context, runner Runner, p Params) (*Result, error) {
|
|||||||
return nil, fmt.Errorf("cfsetup: write config: %w", err)
|
return nil, fmt.Errorf("cfsetup: write config: %w", err)
|
||||||
}
|
}
|
||||||
prog = append(prog, "Wrote "+p.ConfigPath)
|
prog = append(prog, "Wrote "+p.ConfigPath)
|
||||||
// 6b. Install and start the connector for the config just written, so the
|
// Setup stops at writing the config: RUNNING a connector for it is a
|
||||||
// tunnel is actually served rather than routed-but-dead (error 1033).
|
// host-specific side effect (systemd/launchd/Windows service) that lives
|
||||||
// Guarded by ConfigPath: with no config there is nothing to run, and a
|
// with the caller, not in this host-agnostic package. The felis TUI does it
|
||||||
// caller wanting only the Access config is not forced to install a service.
|
// right after Setup returns (installCloudflaredService in tui_edge_apply.go),
|
||||||
notify("Starting tunnel connector…")
|
// closing the routed-but-dead 1033 the same way RouteDNS's --overwrite-dns
|
||||||
if err := runner.StartConnector(ctx, p.ConfigPath); err != nil {
|
// closes the stale-DNS 1033. A caller that skips that step gets a routed
|
||||||
return nil, fmt.Errorf("cfsetup: start connector: %w", err)
|
// tunnel with no connector — Cloudflare error 1033 — by its own choice.
|
||||||
}
|
|
||||||
prog = append(prog, "Started connector")
|
|
||||||
}
|
}
|
||||||
// 7. Front the admin face with a self-hosted Access app.
|
// 7. Front the admin face with a self-hosted Access app.
|
||||||
notify("Creating Access application…")
|
notify("Creating Access application…")
|
||||||
|
|||||||
@@ -69,11 +69,6 @@ func (r *recordingRunner) WriteTunnelConfig(path string, _ []byte) error {
|
|||||||
return nil
|
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) {
|
func (r *recordingRunner) CreateAccessApplication(_ context.Context, app AccessApplication) (string, string, error) {
|
||||||
r.calls = append(r.calls, "CreateAccessApplication:"+app.Domain)
|
r.calls = append(r.calls, "CreateAccessApplication:"+app.Domain)
|
||||||
appID := r.appID
|
appID := r.appID
|
||||||
@@ -336,71 +331,6 @@ 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
|
// TestIngressSafetyInvariants parses the generated cloudflared config back and
|
||||||
// asserts the properties that protect the origin: the final rule is the
|
// 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
|
// hostname-less fail-shut 404 catch-all, every routed hostname points at the panel
|
||||||
|
|||||||
@@ -171,30 +171,6 @@ func (r *ExecRunner) WriteTunnelConfig(path string, contents []byte) error {
|
|||||||
return os.WriteFile(path, contents, 0o644)
|
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
|
// 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).
|
// 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) {
|
func (r *ExecRunner) CreateAccessApplication(ctx context.Context, app AccessApplication) (string, string, error) {
|
||||||
|
|||||||
Reference in new issue
Block a user