From 97c745a029e069b549825c91b2e55abf1d4ab85f Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Tue, 7 Jul 2026 16:08:18 +0500 Subject: [PATCH] fix(client create): don't mint over a live client when the UID read fails (#158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RFC-0001 §7.2 adopt-backfill ran only inside `if clusterID != ""`, so a kube-system UID read that failed for a reason OTHER than unreachability (RBAC on namespaces, a transient API error) skipped live-client detection entirely and minted a duplicate — orphaning the live install. Worst under --yes. adoptLiveInClusterClient already detects the live client (readInClusterClient) independently of the UID; only the anchor backfill needs it. So run adopt unconditionally and, when clusterID == "" but a live owned client is found, adopt it as-is (no backfill Patch, no anchor-mismatch check) and warn that the idempotency anchor was left unstamped. A genuinely-unreachable cluster fails detection too and still falls through to a non-anchored mint — unchanged. Adds a regression test (UID read fails + live owned client ⇒ pure adopt, no mint, no PATCH) and stubs readInClusterClient in the cancel test that bypasses withClientBackend (adopt now always calls it). Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 36 ++++++++++++++++++------- internal/cli/client_test.go | 39 ++++++++++++++++++++++++++++ internal/cli/verbose_install_test.go | 8 ++++++ 3 files changed, 73 insertions(+), 10 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 93ad2209..99ba8e0f 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -254,16 +254,22 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien // live on this cluster but its backend cluster_id is null (it predates the // anchor), a create keyed on the freshly-read UID matches nothing and mints a // DUPLICATE, orphaning the live client. Instead adopt the live client and - // backfill its anchor onto it. Needs a readable anchor (nothing to stamp - // otherwise); without one we fall through to a plain create (dual-mode). - if clusterID != "" { - adoptedPC, handled, aerr := adoptLiveInClusterClient(ctx, p, ilog, client, opts, accountClients, listErr, clusterID) - if aerr != nil { - return aerr - } - if handled { - pc, adopted = adoptedPC, true - } + // backfill its anchor onto it. + // + // Run this even when the UID read failed (clusterID == ""): detecting the live + // client (readInClusterClient) is independent of the anchor, and only the + // backfill needs it. Gating the whole block on clusterID != "" meant a + // reachable cluster whose kube-system UID read failed (RBAC on + // namespaces/kube-system, a transient API error) skipped detection entirely and + // minted over the live client — orphaning it (#158). adopt now adopts-without- + // backfill in that case; a genuinely-unreachable cluster fails detection too and + // falls through to a non-anchored mint, unchanged. + adoptedPC, handled, aerr := adoptLiveInClusterClient(ctx, p, ilog, client, opts, accountClients, listErr, clusterID) + if aerr != nil { + return aerr + } + if handled { + pc, adopted = adoptedPC, true } if pc == nil { @@ -490,6 +496,16 @@ func adoptLiveInClusterClient( } switch { + case clusterID == "": + // A live owned client is here, but the cluster UID read failed + // (namespaces/kube-system unreadable — RBAC, a transient API error) so + // there's nothing to backfill and no anchor to compare against. The + // invariant that matters is §7.2 — never mint over a live client — so + // adopt it as-is (no Patch, no mismatch check) rather than fall through + // and mint a duplicate. Warn that idempotency wasn't (re)stamped; a + // cluster where kube-system is readable enables the backfill. + p.Hintf("A tracebloc client is already running on this cluster — adopting it. Couldn't read the cluster identity, so its idempotency anchor was left unchanged; point --kubeconfig/--context at a cluster where kube-system is readable to stamp it.") + ilog.Logf("adopting live client id=%d without anchor backfill (cluster UID unread)", owner.ID) case owner.ClusterID == "": // The R7 case: backfill the freshly-read anchor onto the live client. patched, perr := apiClient.PatchClientClusterID(ctx, owner.ID, clusterID) diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index f622c328..e3c2ab66 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -203,6 +203,45 @@ func TestClientCreate_R7_AdoptBackfill(t *testing.T) { } } +// TestClientCreate_R7_UIDReadFailsAdoptsLive: the kube-system UID read fails +// (e.g. RBAC on namespaces, a transient API error) while the cluster is still +// reachable enough to discover a live owned client — #158. The create must NOT +// mint over it; it adopts the live client as-is, with no backfill PATCH (there's +// no anchor to stamp). +func TestClientCreate_R7_UIDReadFailsAdoptsLive(t *testing.T) { + postCalled, patchCalled := false, false + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/edge-device/": + _, _ = w.Write([]byte(`[{"id":7,"first_name":"box","username":"uuid-live","namespace":"ns-live","cluster_id":""}]`)) + case r.Method == http.MethodPatch && r.URL.Path == "/edge-device/7/": + patchCalled = true + t.Error("no anchor to backfill when the UID read failed — PATCH must NOT be called") + case r.Method == http.MethodPost && r.URL.Path == "/edge-device/": + postCalled = true + t.Error("must NOT mint over a live client when the UID read failed (#158)") + default: + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) + } + }) + // UID read fails, but the live client is still discoverable. + stubClusterID(t, "", errors.New(`namespaces "kube-system" is forbidden`)) + stubInClusterClient(t, &cluster.InClusterClient{ClientID: "uuid-live", Namespace: "ns-live"}, nil) + + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), nil, + clientCreateOpts{name: "box", location: "DE", yes: true}); err != nil { + t.Fatalf("create: %v", err) + } + if postCalled || patchCalled { + t.Fatal("expected a pure adopt (no mint, no backfill)") + } + cfg, _ := config.Load() + if cfg.Current().ActiveClientID != "7" { + t.Errorf("active client = %q, want 7 (adopted live client)", cfg.Current().ActiveClientID) + } +} + // TestClientCreate_R7_AlreadyAnchored: the live client already carries this // cluster's anchor → adopt directly, no PATCH, no mint. func TestClientCreate_R7_AlreadyAnchored(t *testing.T) { diff --git a/internal/cli/verbose_install_test.go b/internal/cli/verbose_install_test.go index 0a4e5e00..78ec84fe 100644 --- a/internal/cli/verbose_install_test.go +++ b/internal/cli/verbose_install_test.go @@ -137,6 +137,14 @@ func TestClientCreate_CancelLogsCancelledNotDone(t *testing.T) { return "", errors.New("no cluster (test)") } t.Cleanup(func() { readClusterID = origCID }) + // The R7 adopt-backfill now runs even when the UID read fails (#158), so it + // calls readInClusterClient regardless — stub it to "no live client" so this + // test doesn't touch the developer's real kubeconfig. + origLive := readInClusterClient + readInClusterClient = func(context.Context, cluster.KubeconfigOptions) (*cluster.InClusterClient, error) { + return nil, nil + } + t.Cleanup(func() { readInClusterClient = origLive }) confirmNo := false pr := &fakePrompter{confirm: &confirmNo}