diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d9277061..f0a63d61 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -34,6 +34,26 @@ jobs: - name: scripts/sync-schema.sh --check run: ./scripts/sync-schema.sh --check + installer: + timeout-minutes: 10 + name: Installer (shell) + # The curl|sh installer is the most privileged code we ship (it places the + # binary on PATH) and had NO automated test until R8. shellcheck it under + # the POSIX sh dialect it actually runs as, parse it with dash, and run the + # functional harness that asserts cosign verification is mandatory / fails + # closed when cosign is absent (RFC-0001 R8, backend#889). + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: shellcheck + dash parse + run: | + sudo apt-get update -qq && sudo apt-get install -y -qq shellcheck dash + shellcheck --shell=sh --severity=error scripts/install.sh + dash -n scripts/install.sh + bash -n scripts/tests/install-verify.sh + - name: Verification harness (mandatory cosign / fail-closed) + run: bash scripts/tests/install-verify.sh + test: timeout-minutes: 15 name: Test diff --git a/README.md b/README.md index 3d361023..4cf03dfd 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,15 @@ tracebloc dataset push ./my-data \ > **`tracebloc: command not found` after installing?** The binary installs to `~/.local/bin` when `/usr/local/bin` isn't writable, and an already-running shell won't see the new PATH entry until you open a new terminal (or `. ~/.bashrc`). See **[Troubleshooting installation](docs/troubleshooting.md)**. +> **Signature verification is mandatory.** The installer verifies the binary's +> SHA256 **and** its cosign signature before installing. If `cosign` isn't on +> PATH it bootstraps a pinned, checksum-verified copy; if it can't, the install +> **fails closed** rather than trusting the same-channel checksum alone (it no +> longer silently skips the signature). The one escape, for a genuinely +> constrained environment, is to re-run with `TRACEBLOC_ALLOW_UNVERIFIED=1` — +> which prints a loud warning. For the highest trust, pre-install `cosign` +> (`brew install cosign`, your package manager, or the [released binary](https://github.com/sigstore/cosign/releases)) before running the installer. (RFC-0001 R8.) + What that runs under the curtain: 1. Reads kubeconfig, discovers the parent `tracebloc/client` release in the cluster diff --git a/internal/api/client.go b/internal/api/client.go index f0e8187c..11aba245 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -16,6 +16,7 @@ import ( "net/http" "net/url" "os" + "runtime" "strings" "time" ) @@ -29,6 +30,56 @@ const ( const defaultTimeout = 30 * time.Second +// ── User-Agent: minimum-CLI-version handshake (RFC-0001 §13 / §14 R11 / C.1) ── +// +// Every backend request announces the CLI build as +// +// User-Agent: tracebloc-cli/ (/) +// +// so the backend's MIN_SUPPORTED_CLI_VERSION gate (backend#888) can answer a +// too-old client with 426 Upgrade Required. The version is the ldflags-injected +// build version, recorded once at startup; until then (e.g. a bare `go run`) it +// reports "dev", which the backend can't parse and therefore lets through — the +// right fail-open for local development. + +// userAgent is the formatted header value, set once via SetUserAgent at startup. +// A package var (not a constructor arg) so every Client built anywhere — login, +// client provisioning — carries it without threading the version through each +// command; it mirrors the build metadata that already lives as a global in main. +var userAgent string + +// SetUserAgent records the CLI version used in the User-Agent on every backend +// request. Call once from cli.NewRootCmd with the ldflags-injected version. +func SetUserAgent(version string) { + if version == "" { + version = "dev" + } + userAgent = fmt.Sprintf("tracebloc-cli/%s (%s/%s)", version, runtime.GOOS, runtime.GOARCH) +} + +// currentUserAgent is the header value to send, with a "dev" fallback for builds +// that never called SetUserAgent (tests, `go run`). +func currentUserAgent() string { + if userAgent != "" { + return userAgent + } + return fmt.Sprintf("tracebloc-cli/dev (%s/%s)", runtime.GOOS, runtime.GOARCH) +} + +// userAgentTransport injects the CLI User-Agent on every request that doesn't +// already set one. It wraps the real transport (which keeps proxy + system-CA +// behavior). Per the http.RoundTripper contract it clones the request before +// mutating headers rather than modifying the caller's request in place. +type userAgentTransport struct{ base http.RoundTripper } + +func (t userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if req.Header.Get("User-Agent") == "" { + req = req.Clone(req.Context()) + req.Header.Set("User-Agent", currentUserAgent()) + } + return t.base.RoundTrip(req) +} + // BaseURL maps a CLIENT_ENV value to the backend base URL — kept in lock-step // with the installer's `_backend_url` and client-runtime's CLIENT_ENV→backend // mapping. Unknown / empty → prod. @@ -73,8 +124,10 @@ func New(env string) *Client { return &Client{ BaseURL: strings.TrimRight(BaseURL(env), "/"), HTTP: &http.Client{ - Timeout: defaultTimeout, - Transport: &http.Transport{Proxy: http.ProxyFromEnvironment}, + Timeout: defaultTimeout, + // Wrap the proxy/CA-aware transport so every request carries the + // tracebloc-cli User-Agent (RFC-0001 §14 R11 / backend#888). + Transport: userAgentTransport{base: &http.Transport{Proxy: http.ProxyFromEnvironment}}, }, } } @@ -90,6 +143,38 @@ func (e *APIError) Error() string { return fmt.Sprintf("%s returned HTTP %d: %s", e.URL, e.StatusCode, strings.TrimSpace(e.Body)) } +// UpgradeRequiredError is returned for an HTTP 426 from any endpoint: the CLI is +// below the backend's MIN_SUPPORTED_CLI_VERSION floor (RFC-0001 §14 R11 / +// backend#888). It's detected centrally (in post/get) so every call degrades to +// the same actionable "upgrade your CLI" message instead of a raw HTTP error. +type UpgradeRequiredError struct { + MinVersion string // the server's minimum supported version, when it tells us +} + +func (e *UpgradeRequiredError) Error() string { + floor := "a newer version" + if e.MinVersion != "" { + floor = ">= " + e.MinVersion + } + return fmt.Sprintf( + "this tracebloc CLI is too old for the server (requires %s). Upgrade to the "+ + "latest release — re-run the tracebloc install script, or download it from "+ + "https://github.com/tracebloc/cli/releases/latest — then retry.", + floor, + ) +} + +// parseUpgradeRequired builds an *UpgradeRequiredError from a 426 body +// ({"error":"upgrade_required","min_version":"X"}). min_version is best-effort: +// the 426 status is the contract, so a body we can't parse still upgrades. +func parseUpgradeRequired(raw []byte) *UpgradeRequiredError { + var body struct { + MinVersion string `json:"min_version"` + } + _ = json.Unmarshal(raw, &body) + return &UpgradeRequiredError{MinVersion: body.MinVersion} +} + // post sends an optional JSON body and returns the status code + raw response. func (c *Client) post(ctx context.Context, path string, body any) (int, []byte, error) { var rdr io.Reader @@ -116,6 +201,9 @@ func (c *Client) post(ctx context.Context, path string, body any) (int, []byte, if err != nil { return resp.StatusCode, nil, fmt.Errorf("reading response from %s: %w", url, err) } + if resp.StatusCode == http.StatusUpgradeRequired { + return resp.StatusCode, raw, parseUpgradeRequired(raw) + } return resp.StatusCode, raw, nil } @@ -146,6 +234,9 @@ func (c *Client) get(ctx context.Context, path string) (int, []byte, error) { if err != nil { return resp.StatusCode, nil, fmt.Errorf("reading response from %s: %w", url, err) } + if resp.StatusCode == http.StatusUpgradeRequired { + return resp.StatusCode, raw, parseUpgradeRequired(raw) + } return resp.StatusCode, raw, nil } @@ -251,6 +342,23 @@ func (c *Client) WhoAmI(ctx context.Context) (*Identity, error) { return &id, nil } +// RevokeToken revokes the presenting credential server-side via POST /auth/revoke +// (backend#887, shipped in backend#903): Bearer in, 204 out, idempotent. `logout` +// calls this so a copied/leaked token stops authenticating after sign-out — local +// clearing alone left it valid (RFC-0001 §7.5 / R2). Requires Token. A non-2xx is +// returned as an *APIError; callers treat the call as best-effort. +func (c *Client) RevokeToken(ctx context.Context) error { + url := c.BaseURL + "/auth/revoke" + status, raw, err := c.post(ctx, "/auth/revoke", nil) + if err != nil { + return err + } + if status < 200 || status >= 300 { + return &APIError{StatusCode: status, Body: string(raw), URL: url} + } + return nil +} + // ── Client provisioning (Bearer-authed) — backend#836, /edge-device/ ── // ProvisionedClient is a tracebloc client (machine), as returned by the diff --git a/internal/api/client_test.go b/internal/api/client_test.go index e091b79c..621c92f7 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -6,6 +6,8 @@ import ( "errors" "net/http" "net/http/httptest" + "runtime" + "strings" "testing" ) @@ -253,3 +255,142 @@ func TestListClients_BareArrayUnpaginated(t *testing.T) { t.Fatalf("want 2 clients [1,2] from a bare array, got %+v", got) } } + +// ── cli#98: User-Agent version header + 426 Upgrade Required (RFC-0001 §14 R11) ── + +// TestUserAgentHeaderSent proves the transport wrapper puts the configured +// version on the wire for a real request (here a GET via WhoAmI). +func TestUserAgentHeaderSent(t *testing.T) { + old := userAgent + defer func() { userAgent = old }() + SetUserAgent("7.7.7") + + want := "tracebloc-cli/7.7.7 (" + runtime.GOOS + "/" + runtime.GOARCH + ")" + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.Header.Get("User-Agent") + _, _ = w.Write([]byte(`{"email":"x","type":"y","account":"z"}`)) + })) + defer srv.Close() + c := New("prod") + c.BaseURL = srv.URL + c.Token = "t" + if _, err := c.WhoAmI(context.Background()); err != nil { + t.Fatal(err) + } + if got != want { + t.Errorf("User-Agent = %q, want %q", got, want) + } +} + +// TestUserAgentDevFallback: a build that never called SetUserAgent (or with an +// empty version) reports "dev" — which the backend can't parse, so it fails open. +func TestUserAgentDevFallback(t *testing.T) { + old := userAgent + defer func() { userAgent = old }() + + want := "tracebloc-cli/dev (" + runtime.GOOS + "/" + runtime.GOARCH + ")" + userAgent = "" + if got := currentUserAgent(); got != want { + t.Errorf("unset → currentUserAgent() = %q, want %q", got, want) + } + SetUserAgent("") + if got := currentUserAgent(); got != want { + t.Errorf(`SetUserAgent("") → %q, want %q`, got, want) + } +} + +// TestUpgradeRequired426 pins the central 426 handling: a 426 from any endpoint +// (GET or POST) surfaces as a typed *UpgradeRequiredError carrying min_version, +// not a raw *APIError — so every command degrades to the same upgrade message. +func TestUpgradeRequired426(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUpgradeRequired) // 426 + _, _ = w.Write([]byte(`{"error":"upgrade_required","min_version":"0.4.0"}`)) + })) + defer srv.Close() + c := New("prod") + c.BaseURL = srv.URL + c.Token = "t" + + var ue *UpgradeRequiredError + if _, err := c.WhoAmI(context.Background()); !errors.As(err, &ue) || ue.MinVersion != "0.4.0" { + t.Errorf("GET on 426: want *UpgradeRequiredError{0.4.0}, got %v", err) + } + ue = nil + if _, err := c.RequestDeviceCode(context.Background()); !errors.As(err, &ue) || ue.MinVersion != "0.4.0" { + t.Errorf("POST on 426: want *UpgradeRequiredError{0.4.0}, got %v", err) + } +} + +// TestUpgradeRequiredErrorMessage: the message is actionable (names the floor) +// and stays sensible when the server didn't send a min_version. +func TestUpgradeRequiredErrorMessage(t *testing.T) { + msg := (&UpgradeRequiredError{MinVersion: "0.4.0"}).Error() + if !strings.Contains(msg, "0.4.0") || !strings.Contains(msg, "too old") { + t.Errorf("message not actionable: %q", msg) + } + if got := (&UpgradeRequiredError{}).Error(); strings.Contains(got, ">=") { + t.Errorf("empty min_version should not print a bare '>=': %q", got) + } +} + +// TestUpgradeRequired426_UnparseableBody: a 426 whose body doesn't parse still +// yields an *UpgradeRequiredError (the status is the contract), with no min_version. +func TestUpgradeRequired426_UnparseableBody(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUpgradeRequired) + _, _ = w.Write([]byte(`not json`)) + })) + defer srv.Close() + c := New("prod") + c.BaseURL = srv.URL + var ue *UpgradeRequiredError + if _, err := c.RequestDeviceCode(context.Background()); !errors.As(err, &ue) { + t.Fatalf("want *UpgradeRequiredError even on unparseable body, got %v", err) + } + if ue.MinVersion != "" { + t.Errorf("MinVersion = %q, want empty", ue.MinVersion) + } +} + +// ── cli#112: logout server-side revoke (POST /auth/revoke, backend#887) ── + +// TestRevokeToken: a 204 from the endpoint → nil, with Bearer + POST on the wire. +func TestRevokeToken(t *testing.T) { + var sawAuth, sawMethod, sawPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawAuth, sawMethod, sawPath = r.Header.Get("Authorization"), r.Method, r.URL.Path + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + c := New("prod") + c.BaseURL = srv.URL + c.Token = "usertoken123" + if err := c.RevokeToken(context.Background()); err != nil { + t.Fatalf("revoke: %v", err) + } + if sawMethod != http.MethodPost || sawPath != "/auth/revoke" { + t.Errorf("revoke hit %s %s, want POST /auth/revoke", sawMethod, sawPath) + } + if sawAuth != "Bearer usertoken123" { + t.Errorf("revoke auth header = %q, want %q", sawAuth, "Bearer usertoken123") + } +} + +// TestRevokeTokenServerError: a non-2xx surfaces as *APIError so logout can log +// it (then clear local state regardless — see the cli-package logout tests). +func TestRevokeTokenServerError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"detail":"boom"}`)) + })) + defer srv.Close() + c := New("prod") + c.BaseURL = srv.URL + c.Token = "t" + var ae *APIError + if err := c.RevokeToken(context.Background()); !errors.As(err, &ae) || ae.StatusCode != http.StatusInternalServerError { + t.Errorf("want APIError 500, got %v", err) + } +} diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 9176586c..b30deb71 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -57,6 +57,7 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { } env := api.ResolveEnv(envFlag) client := newAPIClient(env) + p.Detailf("backend %s — requesting a device code …", client.BaseURL) dc, err := client.RequestDeviceCode(ctx) if err != nil { @@ -101,21 +102,27 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { tok, err := client.PollToken(ctx, dc.DeviceCode) switch { case err == nil: - cfg.Env = env - cfg.Token = tok + // Switch the active env and write into THAT env's profile, leaving the + // other envs' tokens + active-client pointers intact (R10). Profile() + // returns env's existing profile, so a re-login preserves its + // active_client_id rather than clobbering it. + cfg.CurrentEnv = env + prof := cfg.Profile(env) + prof.Token = tok // Confirm the freshly-issued token actually authenticates, and // capture the account to show + store. Best-effort: don't fail a // successful sign-in just because this lookup couldn't run. client.Token = tok + p.Detailf("authorized — confirming the token with the backend …") if id, werr := client.WhoAmI(ctx); werr == nil { - cfg.Email = id.Email + prof.Email = id.Email } if err := cfg.Save(); err != nil { return &exitError{code: 1, err: err} } p.Newline() - if cfg.Email != "" { - p.Successf("Signed in as %s. Token saved to ~/.tracebloc (0600).", cfg.Email) + if prof.Email != "" { + p.Successf("Signed in as %s. Token saved to ~/.tracebloc (0600).", prof.Email) } else { p.Successf("Signed in. Token saved to ~/.tracebloc (0600).") } @@ -136,32 +143,55 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { } } -// newLogoutCmd implements `tracebloc logout` — clears the stored token. +// newLogoutCmd implements `tracebloc logout` — revokes the token server-side +// (so a copied/leaked credential stops working) and clears it locally. func newLogoutCmd() *cobra.Command { return &cobra.Command{ Use: "logout", - Short: "Sign out (clear the stored token)", + Short: "Sign out (revoke the token server-side and clear it locally)", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { + p := printerFor(cmd) cfg, err := config.Load() if err != nil { return &exitError{code: 1, err: err} } if !cfg.SignedIn() { - printerFor(cmd).Hintf("Already signed out.") + p.Hintf("Already signed out.") return nil } - cfg.Token = "" - cfg.Email = "" - // Also drop the active-client pointer: it's account-scoped, so leaving - // it would bleed into the next account's session (a later `login` as a - // different user would inherit a stale active client in `auth status` / - // `client list`). - cfg.ActiveClientID = "" + + // Capture what the server-side revoke needs BEFORE clearing local + // state. Resolve the env the same way authedClient does (current env, + // else $CLIENT_ENV, else prod) so revoke hits the host the token was + // issued for, not a hardcoded prod. + prof := cfg.Current() + token := prof.Token + env := sessionEnv(cfg) + + // Clear and persist local state FIRST — it's logout's primary job and + // the always-safe step. Saving before the network call means a failed + // Save can't leave a token that's already been revoked server-side + // sitting on disk as a broken "signed in" state. Only THIS env's + // profile is cleared; other envs' sessions are untouched (R10). The + // active-client pointer goes too — it's account-scoped, so leaving it + // would bleed into the next sign-in on this env. + *prof = config.Profile{} if err := cfg.Save(); err != nil { return &exitError{code: 1, err: err} } - printerFor(cmd).Successf("Signed out.") + + // Then revoke the token server-side so a copied/leaked credential stops + // authenticating after sign-out (RFC-0001 §7.5 / R2, backend#887). + // Best-effort by contract: on failure (offline / already-revoked) the + // local session is already cleared — the user is logged out (cli#112). + client := newAPIClient(env) + client.Token = token + if rerr := client.RevokeToken(cmd.Context()); rerr != nil { + p.Hintf("Signed out locally, but couldn't revoke the token server-side (%v). Revoke from the dashboard if this was a shared machine.", rerr) + return nil + } + p.Successf("Signed out.") return nil }, } @@ -193,18 +223,18 @@ func newAuthStatusCmd() *cobra.Command { p.Hintf("Not signed in. Run `tracebloc login`.") return nil } - env := cfg.Env - if env == "" { - env = api.EnvProd - } + prof := cfg.Current() p.Section("tracebloc auth") p.Field("status", "signed in") - p.Field("backend", env) - if cfg.Email != "" { - p.Field("account", cfg.Email) + p.Field("backend", cfg.CurrentEnv) + if prof.Email != "" { + p.Field("account", prof.Email) + } + if prof.ActiveClientID != "" { + p.Field("active client", prof.ActiveClientID) } - if cfg.ActiveClientID != "" { - p.Field("active client", cfg.ActiveClientID) + if prof.ExpiresAt != "" { + p.Field("expires", prof.ExpiresAt) } return nil }, diff --git a/internal/cli/auth_test.go b/internal/cli/auth_test.go index d17ce6ee..8b9b3fbc 100644 --- a/internal/cli/auth_test.go +++ b/internal/cli/auth_test.go @@ -76,11 +76,11 @@ func TestLogin_FullFlow(t *testing.T) { t.Errorf("expected 2 polls (pending then token), got %d", polls) } cfg, _ := config.Load() - if cfg.Token != "cat_abc" { - t.Errorf("stored token = %q, want cat_abc", cfg.Token) + if cfg.Current().Token != "cat_abc" { + t.Errorf("stored token = %q, want cat_abc", cfg.Current().Token) } - if cfg.Email != "ds@tracebloc.io" { - t.Errorf("stored email = %q, want ds@tracebloc.io", cfg.Email) + if cfg.Current().Email != "ds@tracebloc.io" { + t.Errorf("stored email = %q, want ds@tracebloc.io", cfg.Current().Email) } if !strings.Contains(out, "ds@tracebloc.io") { t.Errorf("expected output to show the account, got:\n%s", out) @@ -167,31 +167,106 @@ func TestLogin_Denied(t *testing.T) { } func TestLogout(t *testing.T) { - t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&config.Config{Token: "x", Email: "e@co", ActiveClientID: "7"}).Save(); err != nil { + // logout now revokes server-side (cli#112) — route it at a stub, not prod. + var revoked bool + withTestBackend(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/auth/revoke" || r.Method != http.MethodPost { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + return + } + revoked = true + if got := r.Header.Get("Authorization"); got != "Bearer x" { + t.Errorf("revoke auth header = %q, want %q", got, "Bearer x") + } + w.WriteHeader(http.StatusNoContent) // 204, like the real endpoint + }) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "e@co", ActiveClientID: "7"}, + }}).Save(); err != nil { t.Fatal(err) } out, err := runCmd(t, "logout") if err != nil { t.Fatal(err) } + if !revoked { + t.Error("logout did not call POST /auth/revoke") + } cfg, _ := config.Load() if cfg.SignedIn() { t.Error("expected to be signed out") } // The active-client pointer is account-scoped — it must not survive logout, // or it bleeds into the next account's session. - if cfg.ActiveClientID != "" { - t.Errorf("active_client_id = %q after logout, want cleared", cfg.ActiveClientID) + if cfg.Current().ActiveClientID != "" { + t.Errorf("active_client_id = %q after logout, want cleared", cfg.Current().ActiveClientID) + } + if !strings.Contains(out, "Signed out") { + t.Errorf("got:\n%s", out) + } +} + +// TestLogout_RevokeFailureStillClearsLocal pins the cli#112 contract: when the +// server-side revoke fails (offline / already-revoked / 5xx), logout must still +// succeed and clear local state — never leave the user unable to log out locally. +func TestLogout_RevokeFailureStillClearsLocal(t *testing.T) { + withTestBackend(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) // revoke fails + }) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "e@co", ActiveClientID: "7"}, + }}).Save(); err != nil { + t.Fatal(err) + } + out, err := runCmd(t, "logout") + if err != nil { + t.Fatalf("logout must succeed even when revoke fails: %v", err) + } + cfg, _ := config.Load() + if cfg.SignedIn() || cfg.Current().ActiveClientID != "" { + t.Errorf("local state must be cleared even when revoke fails: %+v", cfg) } if !strings.Contains(out, "Signed out") { t.Errorf("got:\n%s", out) } } +// TestLogout_RevokesAgainstSessionEnv pins that logout revokes against the +// session's own env (the current profile's env), not a hardcoded prod — so the +// token is killed on the host it was issued for (cli#112 / Bugbot, carried to v2). +func TestLogout_RevokesAgainstSessionEnv(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "stg", Profiles: map[string]*config.Profile{ + "stg": {Token: "x"}, + }}).Save(); err != nil { + t.Fatal(err) + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(srv.Close) + + var gotEnv string + orig := newAPIClient + newAPIClient = func(env string) *api.Client { + gotEnv = env + return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} + } + t.Cleanup(func() { newAPIClient = orig }) + + if _, err := runCmd(t, "logout"); err != nil { + t.Fatal(err) + } + if gotEnv != "stg" { + t.Errorf("logout revoked against env %q, want the session env %q (not prod)", gotEnv, "stg") + } +} + func TestAuthStatus_SignedIn(t *testing.T) { t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&config.Config{Env: "dev", Token: "x", Email: "ds@co"}).Save(); err != nil { + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "ds@co"}, + }}).Save(); err != nil { t.Fatal(err) } out, err := runCmd(t, "auth", "status") diff --git a/internal/cli/client.go b/internal/cli/client.go index 328bbc32..74ad73a8 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -104,6 +104,17 @@ func clientPrompter() prompter { return nil } +// sessionEnv resolves the backend env for the signed-in session: the env saved +// at login, falling back (legacy / empty config) to $CLIENT_ENV then prod. Shared +// by authedClient and logout so every authenticated call — including the revoke +// on sign-out — talks to the host the token was actually issued for. +func sessionEnv(cfg *config.Config) string { + if cfg.CurrentEnv != "" { + return cfg.CurrentEnv + } + return api.ResolveEnv("") +} + // authedClient loads the signed-in config and returns a token-bearing API // client, or an error telling the user to log in. func authedClient() (*api.Client, *config.Config, error) { @@ -114,20 +125,43 @@ func authedClient() (*api.Client, *config.Config, error) { if !cfg.SignedIn() { return nil, nil, errors.New("not signed in — run `tracebloc login` first") } - env := cfg.Env - if env == "" { - env = api.ResolveEnv("") - } - client := newAPIClient(env) - client.Token = cfg.Token + client := newAPIClient(sessionEnv(cfg)) + client.Token = cfg.Current().Token return client, cfg, nil } -func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clientCreateOpts) error { +func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clientCreateOpts) (err error) { + // Always leave a full provision trace on disk, even on a quiet/headless run + // (RFC-0001 §8.5). On any failure, point at the (idempotent) resume command + // + `cluster doctor`, so a zero-prompt connect that breaks isn't a dead end. + ilog, logPath := newInstallLog() + defer ilog.Close() + ilog.Logf("client create: name=%q location=%q", opts.name, opts.location) + defer func() { + if err != nil { + ilog.Logf("FAILED: %v", err) + p.Newline() + p.Hintf("Provisioning didn't complete. Re-running is safe — on the same cluster it adopts the existing client instead of minting a duplicate (idempotent):") + p.Hintf(" %s", resumeCommand(opts)) + p.Hintf("Diagnose auth / cluster problems with: tracebloc cluster doctor") + if logPath != "" { + p.Hintf("Full log: %s", logPath) + } + return + } + // Success/cancel: the terminal outcome was already logged at its own + // branch (minted / adopted / cancelled), so don't blanket-log "done" + // here — a declined confirm must not read as a successful provision. + if logPath != "" { + p.Detailf("full log: %s", logPath) + } + }() + client, cfg, err := authedClient() if err != nil { return &exitError{code: 1, err: err} } + ilog.Logf("authenticated; provisioning against the signed-in account") name, location := opts.name, opts.location @@ -151,6 +185,10 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien return mapClientErr(err) } } + // Reflect the resolved (possibly prompted) name + location back into opts, so + // the failure-path resume command includes them — opts otherwise carries only + // the flags, omitting anything the user typed at a prompt (Bugbot). + opts.name, opts.location = name, location // Read the cluster anchor (kube-system UID) so create is get-or-create keyed on // it — re-running on the same cluster adopts the existing client instead of @@ -161,6 +199,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien if cidErr != nil { p.Hintf("Couldn't read the target cluster's identity — provisioning without a cluster anchor, so re-running won't be idempotent. Point --kubeconfig/--context at the reachable cluster to enable that.") } + ilog.Logf("cluster anchor: %q (read err: %v)", clusterID, cidErr) // Derive the namespace slug from the name, avoiding collisions with existing // clients (best-effort: if the list call fails we still derive a base slug). @@ -191,6 +230,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien return mapClientErr(cerr) } if !ok { + ilog.Logf("cancelled by user at the confirm prompt") p.Hintf("Cancelled.") return nil } @@ -225,7 +265,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien return &exitError{code: 1, err: err} } - cfg.ActiveClientID = strconv.Itoa(pc.ID) + cfg.Current().ActiveClientID = strconv.Itoa(pc.ID) p.Newline() if adopted { @@ -234,6 +274,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien // where the backend instead matches a live in-cluster TB_CLIENT_ID whose // cluster_id is still null and the CLI backfills it via PATCH, is the // installer's orchestration — #838 — not done here.) + ilog.Logf("adopted existing client id=%d namespace=%s", pc.ID, pc.Namespace) p.Successf("This cluster is already registered as client %q (namespace %s) — adopted it.", pc.Name, pc.Namespace) p.Hintf("No new credential issued; the existing one stands. This machine is set to enroll as client %d.", pc.ID) if opts.credentialFile != "" { @@ -242,7 +283,12 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien // installer reconciles the existing release rather than expecting a fresh // credential (#838). if werr := writeClientCredential(opts.credentialFile, []string{ - "TRACEBLOC_CLIENT_ID=" + strconv.Itoa(pc.ID), + // TRACEBLOC_CLIENT_ID is the *auth username* the client pod sends to + // api-token-auth (cred → helm clientId → secret CLIENT_ID → + // controller getenv("CLIENT_ID") as username). The backend + // authenticates an EdgeDevice by its UUID username, NOT the numeric + // dashboard id — so write pc.Username, not pc.ID (id is display-only). + "TRACEBLOC_CLIENT_ID=" + pc.Username, "TB_NAMESPACE=" + pc.Namespace, "TRACEBLOC_CLIENT_ADOPTED=1", }); werr != nil { @@ -263,7 +309,9 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien p.Successf("Provisioned client %q (namespace %s).", pc.Name, pc.Namespace) if opts.credentialFile != "" { if werr := writeClientCredential(opts.credentialFile, []string{ - "TRACEBLOC_CLIENT_ID=" + strconv.Itoa(pc.ID), + // The auth username (UUID), NOT the numeric dashboard id — see the + // adopt path above. api-token-auth authenticates by username. + "TRACEBLOC_CLIENT_ID=" + pc.Username, "TRACEBLOC_CLIENT_PASSWORD=" + password, "TB_NAMESPACE=" + pc.Namespace, }); werr != nil { @@ -276,16 +324,54 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien // Print the credential FIRST — it's the only copy (the backend stores only // the hash), so a later config-save failure must never cost it. p.Section("Machine credential — needed by the installer to connect this client") - p.Field("client id", strconv.Itoa(pc.ID)) - p.Field("username", pc.Username) + // The installer's "Client ID" prompt takes the auth username (UUID); + // that IS TRACEBLOC_CLIENT_ID; the numeric id is a dashboard reference only. + p.Field("client id", pc.Username) + p.Field("dashboard id", strconv.Itoa(pc.ID)) // human reference at ai.tracebloc.io/clients — NOT an installer input p.Field("password", password) } + ilog.Logf("minted client id=%d namespace=%s", pc.ID, pc.Namespace) if serr := cfg.Save(); serr != nil { p.Hintf("Couldn't save the active-client pointer (%v) — run `tracebloc client use %d` to set it.", serr, pc.ID) } return nil } +// resumeCommand reconstructs the `tracebloc client create` invocation to retry a +// failed provision. Re-running is idempotent (RFC-0001 §7.2): on the same cluster +// it adopts the existing client rather than minting a duplicate. +func resumeCommand(opts clientCreateOpts) string { + parts := []string{"tracebloc client create"} + if opts.name != "" { + parts = append(parts, "--name "+shellArg(opts.name)) + } + if opts.location != "" { + parts = append(parts, "--location "+shellArg(opts.location)) + } + if opts.kubeconfigPath != "" { + parts = append(parts, "--kubeconfig "+shellArg(opts.kubeconfigPath)) + } + if opts.contextOverride != "" { + parts = append(parts, "--context "+shellArg(opts.contextOverride)) + } + if opts.credentialFile != "" { + parts = append(parts, "--credential-file "+shellArg(opts.credentialFile)) + } + if opts.yes { + parts = append(parts, "--yes") + } + return strings.Join(parts, " ") +} + +// shellArg single-quotes an argument containing whitespace so the resume command +// stays copy-pasteable for values like "Lab One". +func shellArg(s string) string { + if strings.ContainsAny(s, " \t") { + return "'" + s + "'" + } + return s +} + // writeClientCredential writes the machine credential to path (mode 0600) as a // shell-sourceable env file — the installer (#838) sources it to feed the chart, // so the secret lands in a 0600 file, never the terminal (RFC §9 never-show). The @@ -365,7 +451,7 @@ func runClientList(ctx context.Context, p *ui.Printer) error { p.Section("Clients in your account") for _, c := range clients { marker := "" - if strconv.Itoa(c.ID) == cfg.ActiveClientID { + if strconv.Itoa(c.ID) == cfg.Current().ActiveClientID { marker = " (active)" } p.Field(strconv.Itoa(c.ID)+marker, @@ -385,7 +471,7 @@ func runClientUse(ctx context.Context, p *ui.Printer, id string) error { } for _, c := range clients { if strconv.Itoa(c.ID) == id { - cfg.ActiveClientID = id + cfg.Current().ActiveClientID = id if serr := cfg.Save(); serr != nil { return &exitError{code: 1, err: serr} } diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index dcdcc1cf..cf07021c 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -28,7 +28,9 @@ func withClientBackend(t *testing.T, h http.HandlerFunc) { srv := httptest.NewServer(h) t.Cleanup(srv.Close) t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&config.Config{Env: "dev", Token: "tok"}).Save(); err != nil { + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok"}, + }}).Save(); err != nil { t.Fatal(err) } orig := newAPIClient @@ -79,8 +81,8 @@ func TestClientCreate_Success(t *testing.T) { t.Errorf("create body = %+v", body) } cfg, _ := config.Load() - if cfg.ActiveClientID != "5" { - t.Errorf("active client = %q, want 5", cfg.ActiveClientID) + if cfg.Current().ActiveClientID != "5" { + t.Errorf("active client = %q, want 5", cfg.Current().ActiveClientID) } if !strings.Contains(out.String(), "u-123") { t.Errorf("output missing username:\n%s", out.String()) @@ -140,8 +142,8 @@ func TestClientUse(t *testing.T) { t.Fatal(err) } cfg, _ := config.Load() - if cfg.ActiveClientID != "7" { - t.Errorf("active = %q, want 7", cfg.ActiveClientID) + if cfg.Current().ActiveClientID != "7" { + t.Errorf("active = %q, want 7", cfg.Current().ActiveClientID) } if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "99"); err == nil { t.Error("expected an error for an unknown client id") @@ -277,6 +279,12 @@ func TestClientCreate_AnchorMint(t *testing.T) { if !strings.Contains(out.String(), "Machine credential") { t.Errorf("mint should print the credential, got:\n%s", out.String()) } + // The printed "client id" is what the installer's Client ID prompt consumes — + // it must be the UUID username (u-5), the same value written to the credential + // file, not the numeric dashboard id. Assert the username is shown as the id. + if !strings.Contains(out.String(), "client id") || !strings.Contains(out.String(), "u-5") { + t.Errorf("mint should print the username (u-5) as the client id, got:\n%s", out.String()) + } } func TestClientCreate_AdoptIdempotent(t *testing.T) { @@ -322,8 +330,8 @@ func TestClientCreate_AdoptIdempotent(t *testing.T) { t.Error("password should still be sent in the adopt POST body") } cfg, _ := config.Load() - if cfg.ActiveClientID != "8" { - t.Errorf("active client = %q, want 8 (adopted id)", cfg.ActiveClientID) + if cfg.Current().ActiveClientID != "8" { + t.Errorf("active client = %q, want 8 (adopted id)", cfg.Current().ActiveClientID) } } @@ -357,8 +365,12 @@ func TestClientCreate_CredentialFileMint(t *testing.T) { t.Errorf("credential file mode = %o, want 600", perm) } kv := parseEnvFile(t, credPath) - if kv["TRACEBLOC_CLIENT_ID"] != "5" || kv["TB_NAMESPACE"] != "my-ns" || kv["TRACEBLOC_CLIENT_PASSWORD"] == "" { - t.Errorf("credential file = %v (want id=5, ns=my-ns, non-empty password)", kv) + // TRACEBLOC_CLIENT_ID must be the UUID *username* (here "u-5"), NOT the numeric + // dashboard id (5): it becomes the pod's CLIENT_ID, which controller.py sends to + // api-token-auth as the login username. The backend authenticates an EdgeDevice + // by its username, so writing the id crash-loops the client on "Unable to log in". + if kv["TRACEBLOC_CLIENT_ID"] != "u-5" || kv["TB_NAMESPACE"] != "my-ns" || kv["TRACEBLOC_CLIENT_PASSWORD"] == "" { + t.Errorf("credential file = %v (want id=u-5 [the username, not id 5], ns=my-ns, non-empty password)", kv) } // never-show, the real invariant: the minted password VALUE must not appear // in stdout under any label (the string checks above are just a proxy). @@ -446,10 +458,12 @@ func TestClientCreate_CredentialFileAdopt(t *testing.T) { t.Fatalf("adopt: %v", err) } kv := parseEnvFile(t, credPath) - // adopt emits id + namespace + the ADOPTED marker, but NO password (the - // existing one stands; it's write-only on the backend). - if kv["TRACEBLOC_CLIENT_ID"] != "8" || kv["TB_NAMESPACE"] != "ex-ns" || kv["TRACEBLOC_CLIENT_ADOPTED"] != "1" { - t.Errorf("adopt credential file = %v (want id=8, ns=ex-ns, ADOPTED=1)", kv) + // adopt emits the username + namespace + the ADOPTED marker, but NO password + // (the existing one stands; it's write-only on the backend). Same invariant as + // the mint path: TRACEBLOC_CLIENT_ID is the UUID username ("u-8"), not id 8 — + // it's the login username the adopted client reconnects with. + if kv["TRACEBLOC_CLIENT_ID"] != "u-8" || kv["TB_NAMESPACE"] != "ex-ns" || kv["TRACEBLOC_CLIENT_ADOPTED"] != "1" { + t.Errorf("adopt credential file = %v (want id=u-8 [the username, not id 8], ns=ex-ns, ADOPTED=1)", kv) } if _, hasPw := kv["TRACEBLOC_CLIENT_PASSWORD"]; hasPw { t.Errorf("adopt must not write a password (none issued), got:\n%v", kv) diff --git a/internal/cli/coverage_test.go b/internal/cli/coverage_test.go index 0150ffad..2939324e 100644 --- a/internal/cli/coverage_test.go +++ b/internal/cli/coverage_test.go @@ -128,14 +128,14 @@ func TestClassifyPushOutcome(t *testing.T) { // JSON contract. (Bugbot #49) func TestRunDatasetPush_OutputJSONEarlyFailureEmitsJSON(t *testing.T) { var jsonBuf, human bytes.Buffer - a := runDatasetPushArgs{ + a := runDataIngestArgs{ LocalPath: "./x", Spec: push.SpecArgs{Table: "../bad", Category: "image_classification", Intent: "train"}, Printer: ui.New(&human, ui.WithColor(false)), OutputJSON: true, JSONOut: &jsonBuf, } - err := runDatasetPush(context.Background(), &human, &human, a) + err := runDataIngest(context.Background(), &human, &human, a) var ee *exitError if !errors.As(err, &ee) || ee.Code() != 2 { @@ -202,7 +202,7 @@ func TestExitError_Methods(t *testing.T) { // and never reaches kubeconfig/cluster resolution. func TestRunDatasetRm_InvalidTableExitsTwo(t *testing.T) { var buf bytes.Buffer - err := runDatasetRm(context.Background(), runDatasetRmArgs{ + err := runDataDelete(context.Background(), runDataDeleteArgs{ Table: "../bad", Printer: ui.New(&buf, ui.WithColor(false)), }) diff --git a/internal/cli/dataset.go b/internal/cli/data.go similarity index 95% rename from internal/cli/dataset.go rename to internal/cli/data.go index 012fd2cf..b91c8bb5 100644 --- a/internal/cli/dataset.go +++ b/internal/cli/data.go @@ -20,34 +20,38 @@ import ( "github.com/tracebloc/cli/internal/ui" ) -// newDatasetCmd wires the `tracebloc dataset` subtree. The dominant -// verb is `push`, completed in Phase 3 (tracebloc/client#151) across +// newDataCmd wires the `tracebloc data` subtree. The dominant +// verb is `ingest`, completed in Phase 3 (tracebloc/client#151) across // PR-a (pre-flight: spec synth, validation, layout walk, cluster // discovery) and PR-b (this one: ephemeral stage Pod + tar-over- -// exec stream + progress bar + SIGINT-safe cleanup). `dataset rm` -// (#30) removes a pushed dataset's in-cluster artifacts; `dataset +// exec stream + progress bar + SIGINT-safe cleanup). `data delete` +// (#30) removes an ingested dataset's in-cluster artifacts; `data // list` lists the ingested datasets. -func newDatasetCmd() *cobra.Command { +// +// Aliases: "dataset" is kept for one deprecation cycle so existing +// scripts continue to work. +func newDataCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "dataset", - Short: "Manage datasets in the parent client release", + Use: "data", + Aliases: []string{"dataset"}, + Short: "Manage datasets in the parent client release", Long: `Commands for staging and managing datasets on the cluster's shared PVC. -` + "`dataset push`" + ` stages a local dataset to the cluster's shared +` + "`data ingest`" + ` stages a local dataset to the cluster's shared PVC, submits the ingestion run to jobs-manager, and watches the ingestor Job to completion (streaming logs + the final summary). ` + "`tracebloc cluster info`" + ` is the pre-flight you'd typically run -before the first push.`, +before the first ingest.`, } - cmd.AddCommand(newDatasetPushCmd()) - cmd.AddCommand(newDatasetListCmd()) - cmd.AddCommand(newDatasetRmCmd()) + cmd.AddCommand(newDataIngestCmd()) + cmd.AddCommand(newDataListCmd()) + cmd.AddCommand(newDataDeleteCmd()) return cmd } -// newDatasetPushCmd implements `tracebloc dataset push `. +// newDataIngestCmd implements `tracebloc data ingest `. // // Phase 3 scope (now complete across PR-a + PR-b): // @@ -64,7 +68,10 @@ before the first push.`, // Phase 4 (`tracebloc/client#152`) hooks the submit-to-jobs-manager // step into the bottom of this command, replacing the "manually // kick off helm ingestor" workaround in the success message. -func newDatasetPushCmd() *cobra.Command { +// +// Aliases: "push" is kept for one deprecation cycle so existing +// scripts continue to work. +func newDataIngestCmd() *cobra.Command { var ( // Kubeconfig flags — same conventions as `cluster info`. // Promoting these to persistent on the root is a v0.2 @@ -117,8 +124,9 @@ func newDatasetPushCmd() *cobra.Command { ) cmd := &cobra.Command{ - Use: "push ", - Short: "Stage a local dataset to the cluster's shared PVC", + Use: "ingest ", + Aliases: []string{"push"}, + Short: "Stage a local dataset to the cluster's shared PVC", Long: `Stages a local dataset to the parent client release's shared PVC, submits an ingestion run to jobs-manager, and watches the ingestor Job to completion. Supports 9 task categories (image classification, @@ -161,7 +169,7 @@ Exit codes: } // Guided mode: on a terminal (and unless --no-input), prompt // for whatever's still missing. Off a TTY / with --no-input, - // prompter stays nil and runDatasetPush keeps flag-only + // prompter stays nil and runDataIngest keeps flag-only // behavior. interactive := !noInput && !outputJSON && isInteractiveTTY() var pr prompter @@ -178,8 +186,8 @@ Exit codes: printer = printerForWriter(cmd, cmd.ErrOrStderr()) jsonOut = cmd.OutOrStdout() } - return runDatasetPush(cmd.Context(), humanOut, cmd.ErrOrStderr(), - runDatasetPushArgs{ + return runDataIngest(cmd.Context(), humanOut, cmd.ErrOrStderr(), + runDataIngestArgs{ LocalPath: localPath, Kubeconfig: kubeconfigPath, Context: contextOverride, @@ -268,11 +276,11 @@ Exit codes: return cmd } -// runDatasetPushArgs collects every parameter runDatasetPush needs, +// runDataIngestArgs collects every parameter runDataIngest needs, // so the body stays testable without going through cobra. The cobra // RunE wrapper above is the ONLY caller in production; tests // construct one of these directly. -type runDatasetPushArgs struct { +type runDataIngestArgs struct { LocalPath string Kubeconfig string Context string @@ -289,7 +297,7 @@ type runDatasetPushArgs struct { Printer *ui.Printer // Interactive guided mode (#28). When Interactive is true, - // runDatasetPush prompts (via Prompter) for any missing core inputs + // runDataIngest prompts (via Prompter) for any missing core inputs // before validation. CategorySet records whether --category was // passed explicitly (its non-empty default would otherwise look // like a deliberate choice). Prompter is nil off a TTY / --no-input. @@ -313,7 +321,7 @@ type runDatasetPushArgs struct { // expandHome expands a leading ~ or ~/… to $HOME, leaving every other // path (relative, absolute, empty) untouched. It mirrors // cluster.expandPath — kept as a small local copy rather than coupling -// the dataset path-handling to the cluster package's internals; if a +// the data path-handling to the cluster package's internals; if a // third caller appears, promote both to a shared pathutil. func expandHome(path string) string { if path == "" || path[0] != '~' { @@ -331,7 +339,7 @@ func expandHome(path string) string { return filepath.Join(home, path[1:]) } -// runDatasetPush is the full Phase 3 implementation: pre-flight +// runDataIngest is the full Phase 3 implementation: pre-flight // checks, then either --dry-run stop or stage Pod + tar stream + // cleanup. Phase 4 (#152) will hook submit-to-jobs-manager after // the staging step. @@ -340,7 +348,7 @@ func expandHome(path string) string { // need the cluster runs before any that does, so a customer with // a bad label-column or oversized dataset gets the diagnostic in // milliseconds without a kubeconfig round-trip. -func runDatasetPush(ctx context.Context, out, errOut io.Writer, a runDatasetPushArgs) (err error) { +func runDataIngest(ctx context.Context, out, errOut io.Writer, a runDataIngestArgs) (err error) { // In --output-json mode, guarantee stdout always carries a JSON // object. The dry-run + post-submit paths emit a result and set // jsonEmitted; this defer covers every early-failure return (bad @@ -358,11 +366,11 @@ func runDatasetPush(ctx context.Context, out, errOut io.Writer, a runDatasetPush } }() - // Intro header: brand + a plain-English explainer of what a push + // Intro header: brand + a plain-English explainer of what an ingest // does, so a first-time user understands it before any prompts. // Routed through a.Printer, so --output-json keeps it on stderr and // --plain/non-TTY degrade cleanly. (#31) - a.Printer.Banner("tracebloc", "dataset push") + a.Printer.Banner("tracebloc", "data ingest") a.Printer.Para(strings.TrimSpace(` This uploads a dataset from your machine into your tracebloc workspace so models can be trained on it. Your files are sent to the Kubernetes cluster your @@ -377,7 +385,7 @@ contributors train against it without ever seeing the raw files.`)) if a.Interactive && a.Prompter != nil { if err := runInteractive(a.Printer, a.Prompter, &a, a.CategorySet); err != nil { if errors.Is(err, errInteractiveCancelled) { - a.Printer.Infof("Cancelled — nothing was pushed.") + a.Printer.Infof("Cancelled — nothing was ingested.") return nil } return &exitError{code: 3, err: fmt.Errorf("interactive setup: %w", err)} @@ -424,7 +432,7 @@ contributors train against it without ever seeing the raw files.`)) case push.IsCLISupported(a.Spec.Category): // supported case push.IsKnown(a.Spec.Category): - // A recognized category dataset push doesn't implement yet — image + // A recognized category data ingest doesn't implement yet — image // (semantic_segmentation / instance_segmentation) or text // (causal_language_modeling). Routed here (not the default branch) so the // user gets the registry's per-category pending-support reason, not a diff --git a/internal/cli/dataset_rm.go b/internal/cli/data_delete.go similarity index 85% rename from internal/cli/dataset_rm.go rename to internal/cli/data_delete.go index ecc83f6a..974630a0 100644 --- a/internal/cli/dataset_rm.go +++ b/internal/cli/data_delete.go @@ -12,10 +12,10 @@ import ( "github.com/tracebloc/cli/internal/ui" ) -// runDatasetRmArgs is the resolved input to runDatasetRm — same shape -// convention as runDatasetPushArgs, so the command's RunE stays a thin +// runDataDeleteArgs is the resolved input to runDataDelete — same shape +// convention as runDataIngestArgs, so the command's RunE stays a thin // flag-to-struct adapter and the logic is unit-testable. -type runDatasetRmArgs struct { +type runDataDeleteArgs struct { Table string Kubeconfig string Context string @@ -26,11 +26,14 @@ type runDatasetRmArgs struct { Prompter prompter // nil off a TTY or when --yes is set } -// newDatasetRmCmd implements `tracebloc dataset rm ` — the -// in-cluster teardown of a previously-pushed dataset. See +// newDataDeleteCmd implements `tracebloc data delete
` — the +// in-cluster teardown of a previously-ingested dataset. See // internal/push.Teardown for the mechanism and the design note on the // approach (CLI-direct vs a server-side delete endpoint). -func newDatasetRmCmd() *cobra.Command { +// +// Aliases: "rm" is kept for one deprecation cycle so existing +// scripts continue to work. +func newDataDeleteCmd() *cobra.Command { var ( kubeconfigPath string contextOverride string @@ -40,9 +43,10 @@ func newDatasetRmCmd() *cobra.Command { ) cmd := &cobra.Command{ - Use: "rm
", - Short: "Delete a pushed dataset's in-cluster artifacts (table + PVC files)", - Long: `Removes the in-cluster artifacts a previous ` + "`dataset push`" + ` created + Use: "delete
", + Aliases: []string{"rm"}, + Short: "Delete an ingested dataset's in-cluster artifacts (table + PVC files)", + Long: `Removes the in-cluster artifacts a previous ` + "`data ingest`" + ` created for a table: the MySQL table in ` + push.IngestionDatabase + ` and the dataset's directories on the shared PVC. Destructive and not undoable. @@ -62,7 +66,7 @@ Exit codes: if !yes && isInteractiveTTY() { pr = surveyPrompter{} } - return runDatasetRm(cmd.Context(), runDatasetRmArgs{ + return runDataDelete(cmd.Context(), runDataDeleteArgs{ Table: args[0], Kubeconfig: kubeconfigPath, Context: contextOverride, @@ -89,16 +93,16 @@ Exit codes: return cmd } -// runDatasetRm discovers the cluster, shows the teardown plan, confirms, -// then removes the in-cluster artifacts. The flow mirrors runDatasetPush +// runDataDelete discovers the cluster, shows the teardown plan, confirms, +// then removes the in-cluster artifacts. The flow mirrors runDataIngest // (validate → discover → plan/pre-flight → act) so the two commands feel // like siblings. -func runDatasetRm(ctx context.Context, a runDatasetRmArgs) error { +func runDataDelete(ctx context.Context, a runDataDeleteArgs) error { p := a.Printer - p.Banner("tracebloc", "delete a pushed dataset") - p.Para(`This permanently removes a dataset you pushed earlier: it drops the table from + p.Banner("tracebloc", "delete an ingested dataset") + p.Para(`This permanently removes a dataset you ingested earlier: it drops the table from the cluster and deletes the dataset's files on the shared storage. It can't be -undone — re-pushing the data is the only way back.`) +undone — re-ingesting the data is the only way back.`) // 1. Validate the name before we build any PVC path from it // (push.PlanTeardown panics on an unsafe name by design). @@ -200,7 +204,7 @@ undone — re-pushing the data is the only way back.`) if res.DroppedTable { return &exitError{code: 7, err: fmt.Errorf( "teardown incomplete — the table %s.%s was dropped, but removing its files failed: %w; "+ - "re-run `tracebloc dataset rm %s`, or delete the leftover staging dirs on the node", + "re-run `tracebloc data delete %s`, or delete the leftover staging dirs on the node", plan.Database, plan.Table, err, a.Table)} } return &exitError{code: 7, err: fmt.Errorf("teardown failed: %w", err)} diff --git a/internal/cli/dataset_list.go b/internal/cli/data_list.go similarity index 73% rename from internal/cli/dataset_list.go rename to internal/cli/data_list.go index a0fb36fb..0d6029a6 100644 --- a/internal/cli/dataset_list.go +++ b/internal/cli/data_list.go @@ -14,10 +14,10 @@ import ( "github.com/tracebloc/cli/internal/ui" ) -// runDatasetListArgs is the resolved input to runDatasetList — same -// shape convention as the other dataset verbs, keeping the RunE a thin +// runDataListArgs is the resolved input to runDataList — same +// shape convention as the other data verbs, keeping the RunE a thin // flag-to-struct adapter. -type runDatasetListArgs struct { +type runDataListArgs struct { Kubeconfig string Context string Namespace string @@ -26,12 +26,12 @@ type runDatasetListArgs struct { JSONOut io.Writer } -// newDatasetListCmd implements `tracebloc dataset list` — a read-only +// newDataListCmd implements `tracebloc data list` — a read-only // listing of the datasets ingested into the cluster. The kubeconfig -// flags are all zero-value-safe, so the minimal `tracebloc dataset list` +// flags are all zero-value-safe, so the minimal `tracebloc data list` // runs against the current context + its namespace; the flags only // override that (same convention as `cluster info`). -func newDatasetListCmd() *cobra.Command { +func newDataListCmd() *cobra.Command { var ( kubeconfigPath string contextOverride string @@ -42,11 +42,11 @@ func newDatasetListCmd() *cobra.Command { cmd := &cobra.Command{ Use: "list", Short: "List datasets ingested in the cluster", - Long: `Lists the datasets pushed + ingested into the parent client release — + Long: `Lists the datasets ingested into the parent client release — the tables in ` + push.IngestionDatabase + ` on the cluster. With no flags it uses your current kubeconfig context and its namespace; -the flags below override that, same as ` + "`cluster info`" + ` and ` + "`dataset push`" + `. +the flags below override that, same as ` + "`cluster info`" + ` and ` + "`data ingest`" + `. For the full catalog (with metadata), see the dashboard at https://ai.tracebloc.io/metadata. @@ -58,14 +58,14 @@ Exit codes: Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { // In --output-json mode, human output (the banner) goes to - // stderr so stdout carries only the JSON — same split as push. + // stderr so stdout carries only the JSON — same split as ingest. printer := printerFor(cmd) var jsonOut io.Writer if outputJSON { printer = printerForWriter(cmd, cmd.ErrOrStderr()) jsonOut = cmd.OutOrStdout() } - return runDatasetList(cmd.Context(), runDatasetListArgs{ + return runDataList(cmd.Context(), runDataListArgs{ Kubeconfig: kubeconfigPath, Context: contextOverride, Namespace: nsOverride, @@ -88,14 +88,14 @@ Exit codes: return cmd } -// runDatasetList discovers the cluster, enumerates the ingested tables, -// and renders them. Mirrors the other dataset verbs' discovery so the +// runDataList discovers the cluster, enumerates the ingested tables, +// and renders them. Mirrors the other data verbs' discovery so the // exit-code contract is consistent. -func runDatasetList(ctx context.Context, a runDatasetListArgs) (err error) { +func runDataList(ctx context.Context, a runDataListArgs) (err error) { // In --output-json mode, guarantee stdout always carries JSON: the // success path emits the listing and sets jsonEmitted; this defer // covers the early-failure returns (kubeconfig, no release, query) - // with a JSON error object, mirroring dataset push. (Bugbot #53) + // with a JSON error object, mirroring data ingest. (Bugbot #53) jsonEmitted := false defer func() { if a.OutputJSON && err != nil && !jsonEmitted { @@ -104,7 +104,7 @@ func runDatasetList(ctx context.Context, a runDatasetListArgs) (err error) { if errors.As(err, &ee) { code = ee.Code() } - writeDatasetListErrorJSON(a.JSONOut, err, code) + writeDataListErrorJSON(a.JSONOut, err, code) } }() @@ -134,20 +134,20 @@ func runDatasetList(ctx context.Context, a runDatasetListArgs) (err error) { } if a.OutputJSON { - writeDatasetListJSON(a.JSONOut, resolved.Namespace, release.ReleaseName, tables) + writeDataListJSON(a.JSONOut, resolved.Namespace, release.ReleaseName, tables) jsonEmitted = true return nil } - renderDatasetList(p, resolved.Namespace, tables) + renderDataList(p, resolved.Namespace, tables) return nil } -// renderDatasetList prints the human-facing listing. Split out so it's +// renderDataList prints the human-facing listing. Split out so it's // unit-testable with a buffer-backed Printer. -func renderDatasetList(p *ui.Printer, namespace string, tables []string) { +func renderDataList(p *ui.Printer, namespace string, tables []string) { p.Section(fmt.Sprintf("Datasets in %s (%d)", namespace, len(tables))) if len(tables) == 0 { - p.Infof("No datasets yet — push one with `tracebloc dataset push`.") + p.Infof("No datasets yet — ingest one with `tracebloc data ingest`.") return } for _, t := range tables { @@ -155,19 +155,19 @@ func renderDatasetList(p *ui.Printer, namespace string, tables []string) { } } -// datasetListJSON is the --output-json shape (owned by the CLI layer). -type datasetListJSON struct { +// dataListJSON is the --output-json shape (owned by the CLI layer). +type dataListJSON struct { Namespace string `json:"namespace"` Release string `json:"release"` Count int `json:"count"` Datasets []string `json:"datasets"` } -func writeDatasetListJSON(w io.Writer, namespace, release string, tables []string) { +func writeDataListJSON(w io.Writer, namespace, release string, tables []string) { if tables == nil { tables = []string{} // emit [] not null } - res := datasetListJSON{ + res := dataListJSON{ Namespace: namespace, Release: release, Count: len(tables), @@ -180,10 +180,10 @@ func writeDatasetListJSON(w io.Writer, namespace, release string, tables []strin _, _ = fmt.Fprintln(w, string(b)) } -// writeDatasetListErrorJSON emits a minimal JSON error object for +// writeDataListErrorJSON emits a minimal JSON error object for // --output-json runs that fail before the listing is produced, so -// stdout is never empty on failure (parallels dataset push). (Bugbot #53) -func writeDatasetListErrorJSON(w io.Writer, e error, code int) { +// stdout is never empty on failure (parallels data ingest). (Bugbot #53) +func writeDataListErrorJSON(w io.Writer, e error, code int) { res := struct { Status string `json:"status"` Error string `json:"error"` diff --git a/internal/cli/dataset_list_test.go b/internal/cli/data_list_test.go similarity index 64% rename from internal/cli/dataset_list_test.go rename to internal/cli/data_list_test.go index 7e5d2461..715f8731 100644 --- a/internal/cli/dataset_list_test.go +++ b/internal/cli/data_list_test.go @@ -13,17 +13,17 @@ import ( "github.com/tracebloc/cli/internal/ui" ) -// TestRunDatasetList_OutputJSONEarlyFailureEmitsJSON: with --output-json, +// TestRunDataList_OutputJSONEarlyFailureEmitsJSON: with --output-json, // a failure before the listing (here a broken kubeconfig, exit 3) still // writes a JSON error object to stdout — the stdout-always-JSON contract -// that #49 established for dataset push. (Bugbot #53) -func TestRunDatasetList_OutputJSONEarlyFailureEmitsJSON(t *testing.T) { +// that #49 established for data ingest. (Bugbot #53) +func TestRunDataList_OutputJSONEarlyFailureEmitsJSON(t *testing.T) { bad := filepath.Join(t.TempDir(), "broken.yaml") if err := os.WriteFile(bad, []byte("}{ not valid kubeconfig"), 0o644); err != nil { t.Fatal(err) } var jsonBuf, human bytes.Buffer - err := runDatasetList(context.Background(), runDatasetListArgs{ + err := runDataList(context.Background(), runDataListArgs{ Kubeconfig: bad, OutputJSON: true, Printer: ui.New(&human, ui.WithColor(false)), @@ -43,25 +43,25 @@ func TestRunDatasetList_OutputJSONEarlyFailureEmitsJSON(t *testing.T) { } } -// TestRenderDatasetList_Empty: the empty listing shows the count and -// points the user at `dataset push`. -func TestRenderDatasetList_Empty(t *testing.T) { +// TestRenderDataList_Empty: the empty listing shows the count and +// points the user at `data ingest`. +func TestRenderDataList_Empty(t *testing.T) { var buf bytes.Buffer - renderDatasetList(ui.New(&buf, ui.WithColor(false)), "ap-workspace", nil) + renderDataList(ui.New(&buf, ui.WithColor(false)), "ap-workspace", nil) out := buf.String() if !strings.Contains(out, "Datasets in ap-workspace (0)") { t.Errorf("missing header/count:\n%s", out) } - if !strings.Contains(out, "dataset push") { - t.Errorf("empty state should point at `dataset push`:\n%s", out) + if !strings.Contains(out, "data ingest") { + t.Errorf("empty state should point at `data ingest`:\n%s", out) } } -// TestRenderDatasetList_Items: a populated listing shows the count and +// TestRenderDataList_Items: a populated listing shows the count and // every table name. -func TestRenderDatasetList_Items(t *testing.T) { +func TestRenderDataList_Items(t *testing.T) { var buf bytes.Buffer - renderDatasetList(ui.New(&buf, ui.WithColor(false)), "tracebloc-templates", []string{"reg_train", "churn_test"}) + renderDataList(ui.New(&buf, ui.WithColor(false)), "tracebloc-templates", []string{"reg_train", "churn_test"}) out := buf.String() for _, want := range []string{"Datasets in tracebloc-templates (2)", "reg_train", "churn_test"} { if !strings.Contains(out, want) { @@ -70,13 +70,13 @@ func TestRenderDatasetList_Items(t *testing.T) { } } -// TestWriteDatasetListJSON: valid JSON with the expected fields, and a +// TestWriteDataListJSON: valid JSON with the expected fields, and a // nil dataset slice marshals as [] (not null) so scripts get an array. -func TestWriteDatasetListJSON(t *testing.T) { +func TestWriteDataListJSON(t *testing.T) { var buf bytes.Buffer - writeDatasetListJSON(&buf, "ns1", "tracebloc", []string{"a", "b"}) + writeDataListJSON(&buf, "ns1", "tracebloc", []string{"a", "b"}) - var got datasetListJSON + var got dataListJSON if err := json.Unmarshal(buf.Bytes(), &got); err != nil { t.Fatalf("not JSON: %v\n%s", err, buf.String()) } @@ -88,7 +88,7 @@ func TestWriteDatasetListJSON(t *testing.T) { } buf.Reset() - writeDatasetListJSON(&buf, "ns1", "tracebloc", nil) + writeDataListJSON(&buf, "ns1", "tracebloc", nil) if !strings.Contains(buf.String(), `"datasets": []`) { t.Errorf("nil datasets should marshal as []:\n%s", buf.String()) } diff --git a/internal/cli/dataset_test.go b/internal/cli/data_test.go similarity index 69% rename from internal/cli/dataset_test.go rename to internal/cli/data_test.go index b8415b5c..299849ca 100644 --- a/internal/cli/dataset_test.go +++ b/internal/cli/data_test.go @@ -30,20 +30,21 @@ func imgcLayout(t *testing.T) string { return root } -// execDatasetPush drives the full cobra dispatch for the push -// command and returns the exit code + captured stdout/stderr. +// execDataIngest drives the full cobra dispatch for the ingest +// command using the canonical `data ingest` form and returns the +// exit code + captured stdout/stderr. // Mirrors the execIngestValidate helper from ingest_test.go — same // rationale about not sharing *cobra.Command across cases (cobra // holds flag state on the command tree). // -// kubeconfigPath is required because every push invocation tries +// kubeconfigPath is required because every ingest invocation tries // kubeconfig load before any cluster work; tests that want to // stop EARLIER (at schema validation or layout walk) still need a // kubeconfig path that resolves predictably. We feed in a path // that's guaranteed to fail os.Stat so the kubeconfig branch // errors out consistently when reached — and tests assert on the // EARLIER stage's exit code, which fires before kubeconfig. -func execDatasetPush(t *testing.T, args []string) (exitCode int, stdout, stderr string) { +func execDataIngest(t *testing.T, args []string) (exitCode int, stdout, stderr string) { t.Helper() root := NewRootCmd(BuildInfo{Version: "test"}) var so, se bytes.Buffer @@ -54,7 +55,7 @@ func execDatasetPush(t *testing.T, args []string) (exitCode int, stdout, stderr // "fall through" the local pre-checks into kubeconfig load // get a deterministic exit 3 (not a flaky "depends on whether // you have a real kubeconfig" outcome). - cmdArgs := append([]string{"dataset", "push", + cmdArgs := append([]string{"data", "ingest", "--kubeconfig=/tmp/tracebloc-cli-test-nonexistent-" + t.Name()}, args...) root.SetArgs(cmdArgs) @@ -63,7 +64,7 @@ func execDatasetPush(t *testing.T, args []string) (exitCode int, stdout, stderr return ExitCodeFromError(err), so.String(), se.String() } -// TestDatasetPush_UnsupportedCategory_ExitsTwo: the CLI-side category +// TestDataIngest_UnsupportedCategory_ExitsTwo: the CLI-side category // gate runs before schema validation so a customer who passes a // not-yet-supported category gets an actionable message (exit 2) // rather than the schema's confusing missing-property error. Today's @@ -71,7 +72,7 @@ func execDatasetPush(t *testing.T, args []string) (exitCode int, stdout, stderr // family; the other image categories (which need annotation/mask // sidecar staging), the text family, and nonsense values are gated // out here. Bugbot review-on-self caught the missing gate on PR-a. -func TestDatasetPush_UnsupportedCategory_ExitsTwo(t *testing.T) { +func TestDataIngest_UnsupportedCategory_ExitsTwo(t *testing.T) { root := imgcLayout(t) for _, badCategory := range []string{ "semantic_segmentation", // blocked on the ingestor (data-ingestors#136) @@ -79,7 +80,7 @@ func TestDatasetPush_UnsupportedCategory_ExitsTwo(t *testing.T) { "definitely-not-a-category", // nonsense; gate catches this too } { t.Run(badCategory, func(t *testing.T) { - code, _, _ := execDatasetPush(t, []string{ + code, _, _ := execDataIngest(t, []string{ root, "--table=t1", "--category=" + badCategory, @@ -93,18 +94,18 @@ func TestDatasetPush_UnsupportedCategory_ExitsTwo(t *testing.T) { } } -// TestDatasetPush_KnownUnsupportedCategory_PendingNote pins the Bugbot fix +// TestDataIngest_KnownUnsupportedCategory_PendingNote pins the Bugbot fix // (v0.4.0 RC): a registry-known but CLI-unsupported NON-image category // (causal_language_modeling) must get the registry's pending-support note, not -// the misleading "isn't a recognized task category" message. execDatasetPush +// the misleading "isn't a recognized task category" message. execDataIngest // discards the error and SilenceErrors swallows it, so run the command here and // inspect the returned error directly. -func TestDatasetPush_KnownUnsupportedCategory_PendingNote(t *testing.T) { +func TestDataIngest_KnownUnsupportedCategory_PendingNote(t *testing.T) { root := imgcLayout(t) rootCmd := NewRootCmd(BuildInfo{Version: "test"}) rootCmd.SetOut(&bytes.Buffer{}) rootCmd.SetErr(&bytes.Buffer{}) - rootCmd.SetArgs([]string{"dataset", "push", + rootCmd.SetArgs([]string{"data", "ingest", "--kubeconfig=/tmp/tracebloc-cli-test-nonexistent-" + t.Name(), root, "--table=t1", "--category=causal_language_modeling", "--intent=train", "--label-column=label"}) @@ -124,17 +125,17 @@ func TestDatasetPush_KnownUnsupportedCategory_PendingNote(t *testing.T) { } } -// TestDatasetPush_TraversalTableName_ExitsTwo is the security +// TestDataIngest_TraversalTableName_ExitsTwo is the security // regression pin at the CLI layer. --table=../../etc must be // rejected with exit 2 BEFORE any spec synthesis or cluster work — // the table name flows into the /data/shared/
/ PVC path, // and a traversal value would let PR-b's stage Pod escape that // subtree. Bugbot flagged this on PR #8 commit 4240097. -func TestDatasetPush_TraversalTableName_ExitsTwo(t *testing.T) { +func TestDataIngest_TraversalTableName_ExitsTwo(t *testing.T) { root := imgcLayout(t) for _, bad := range []string{"../../etc", "../foo", "foo/bar"} { t.Run(bad, func(t *testing.T) { - code, _, _ := execDatasetPush(t, []string{ + code, _, _ := execDataIngest(t, []string{ root, "--table=" + bad, "--category=image_classification", @@ -148,12 +149,12 @@ func TestDatasetPush_TraversalTableName_ExitsTwo(t *testing.T) { } } -// TestDatasetPush_MissingIntent_ExitsTwo: pins the "intent is +// TestDataIngest_MissingIntent_ExitsTwo: pins the "intent is // required" diagnostic path — different schema violation but the // same exit-code class. -func TestDatasetPush_MissingIntent_ExitsTwo(t *testing.T) { +func TestDataIngest_MissingIntent_ExitsTwo(t *testing.T) { root := imgcLayout(t) - code, _, stderr := execDatasetPush(t, []string{ + code, _, stderr := execDataIngest(t, []string{ root, "--table=t1", "--category=image_classification", @@ -168,7 +169,7 @@ func TestDatasetPush_MissingIntent_ExitsTwo(t *testing.T) { } } -// TestDatasetPush_NonexistentLocalPath_ExitsThree: the layout walk +// TestDataIngest_NonexistentLocalPath_ExitsThree: the layout walk // runs AFTER schema validation, so an invalid local path with // otherwise-valid flags surfaces at the walk stage with exit 3 // (the "local input or kubeconfig" code). @@ -179,8 +180,8 @@ func TestDatasetPush_MissingIntent_ExitsTwo(t *testing.T) { // ingest_test.go's TestIngestValidate_UnreadableFileExitsThree // pattern; the error-content surface is exercised at the package // level (internal/push.Discover's own tests). -func TestDatasetPush_NonexistentLocalPath_ExitsThree(t *testing.T) { - code, _, _ := execDatasetPush(t, []string{ +func TestDataIngest_NonexistentLocalPath_ExitsThree(t *testing.T) { + code, _, _ := execDataIngest(t, []string{ "/tmp/tracebloc-cli-test-no-such-dir-" + t.Name(), "--table=t1", "--category=image_classification", @@ -192,12 +193,12 @@ func TestDatasetPush_NonexistentLocalPath_ExitsThree(t *testing.T) { } } -// TestDatasetPush_MissingLabelsCSV_ExitsThree: most likely "real +// TestDataIngest_MissingLabelsCSV_ExitsThree: most likely "real // world" wrong-layout case — customer has images but forgot // labels.csv. Pins the exit-code contract for the common failure // mode; the diagnostic-text content is covered by // internal/push.TestDiscover_MissingLabelsCSV. -func TestDatasetPush_MissingLabelsCSV_ExitsThree(t *testing.T) { +func TestDataIngest_MissingLabelsCSV_ExitsThree(t *testing.T) { root := t.TempDir() imagesDir := filepath.Join(root, "images") if err := os.MkdirAll(imagesDir, 0o755); err != nil { @@ -208,7 +209,7 @@ func TestDatasetPush_MissingLabelsCSV_ExitsThree(t *testing.T) { t.Fatalf("write img: %v", err) } - code, _, _ := execDatasetPush(t, []string{ + code, _, _ := execDataIngest(t, []string{ root, "--table=t1", "--category=image_classification", @@ -220,14 +221,14 @@ func TestDatasetPush_MissingLabelsCSV_ExitsThree(t *testing.T) { } } -// TestDatasetPush_BadKubeconfig_ExitsThree: schema + layout both +// TestDataIngest_BadKubeconfig_ExitsThree: schema + layout both // pass; kubeconfig load fails because the injected path doesn't // exist. The exit-code contract matches `cluster info`'s — same // class of failure (3 = local input problem) surfaces with the // same code regardless of which command tripped it. -func TestDatasetPush_BadKubeconfig_ExitsThree(t *testing.T) { +func TestDataIngest_BadKubeconfig_ExitsThree(t *testing.T) { root := imgcLayout(t) - code, _, _ := execDatasetPush(t, []string{ + code, _, _ := execDataIngest(t, []string{ root, "--table=t1", "--category=image_classification", @@ -239,10 +240,10 @@ func TestDatasetPush_BadKubeconfig_ExitsThree(t *testing.T) { } } -// TestDatasetPush_RequiresExactlyOneArg: cobra-level Args check +// TestDataIngest_RequiresExactlyOneArg: cobra-level Args check // pins the command signature. Two positional args, or zero, should // fail before the runner even fires. -func TestDatasetPush_RequiresExactlyOneArg(t *testing.T) { +func TestDataIngest_RequiresExactlyOneArg(t *testing.T) { cases := []struct { name string args []string @@ -265,10 +266,69 @@ func TestDatasetPush_RequiresExactlyOneArg(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - code, _, _ := execDatasetPush(t, c.args) + code, _, _ := execDataIngest(t, c.args) if code == 0 { t.Errorf("expected non-zero exit for %s, got 0", c.name) } }) } } + +// TestAliasResolution verifies that the deprecated aliases still dispatch +// to the same handlers as the canonical names: +// - "dataset" → same as "data" +// - "push" → same as "ingest" +// - "rm" → same as "delete" +// +// We use --help invocations because they complete without cluster access; +// the exit code 0 + non-empty output is sufficient to confirm the alias +// resolved correctly. +func TestAliasResolution(t *testing.T) { + cases := []struct { + name string + args []string + want string // substring expected in the combined output + }{ + { + name: "dataset alias resolves", + args: []string{"dataset", "--help"}, + want: "ingest", + }, + { + name: "dataset push alias resolves", + args: []string{"dataset", "push", "--help"}, + want: "Stages a local dataset", + }, + { + name: "data ingest canonical", + args: []string{"data", "ingest", "--help"}, + want: "Stages a local dataset", + }, + { + name: "dataset rm alias resolves", + args: []string{"dataset", "rm", "--help"}, + want: "Removes the in-cluster artifacts", + }, + { + name: "data delete canonical", + args: []string{"data", "delete", "--help"}, + want: "Removes the in-cluster artifacts", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + rootCmd := NewRootCmd(BuildInfo{Version: "test"}) + var out bytes.Buffer + rootCmd.SetOut(&out) + rootCmd.SetErr(&out) + rootCmd.SetArgs(c.args) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("Execute() = %v, want nil (help should not error)", err) + } + combined := out.String() + if !strings.Contains(combined, c.want) { + t.Errorf("output missing %q:\n%s", c.want, combined) + } + }) + } +} diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 71a484fb..9fae112d 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -2,11 +2,14 @@ package cli import ( "context" - "fmt" + "errors" + "net/http" "github.com/spf13/cobra" + "github.com/tracebloc/cli/internal/api" "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/config" "github.com/tracebloc/cli/internal/doctor" "github.com/tracebloc/cli/internal/ui" ) @@ -75,19 +78,31 @@ func runClusterDoctor( ) error { p.Banner("tracebloc", "cluster doctor") + // Auth / config checks run FIRST and don't need a cluster — so `doctor` can + // diagnose a failed provision (bad/expired token, wrong env, no active + // client) even before any cluster is reachable (RFC-0001 §8.5). + authStatus := runAuthChecks(ctx, p) + resolved, err := cluster.Load(cluster.KubeconfigOptions{ Path: kubeconfigPath, Context: contextOverride, Namespace: nsOverride, }) if err != nil { - // 3 = kubeconfig file/parse problem (same class as cluster info). - return &exitError{code: 3, err: fmt.Errorf("loading kubeconfig: %w", err)} + // 3 = kubeconfig file/parse problem (same class as cluster info). The + // auth section above already ran; if IT also failed, escalate to 2 so + // automation doesn't read a real auth failure as a kubeconfig-only one. + p.Section("Cluster") + p.Errorf("Kubeconfig — couldn't load it: %v", err) + p.Hintf(" point --kubeconfig / --context at your cluster, or fix ~/.kube/config") + return &exitError{code: kubeconfigExitCode(authStatus), err: nil} } cs, err := cluster.NewClientset(resolved) if err != nil { - return &exitError{code: 3, err: err} + p.Section("Cluster") + p.Errorf("Kubeconfig — %v", err) + return &exitError{code: kubeconfigExitCode(authStatus), err: nil} } p.Section("Kubeconfig") @@ -116,7 +131,9 @@ func runClusterDoctor( } p.Newline() - switch doctor.Worst(results) { + // Overall verdict folds in the auth section, so an auth ✖/⚠ counts even when + // the cluster itself is healthy. + switch worseStatus(authStatus, doctor.Worst(results)) { case doctor.StatusFail: p.Errorf("Problems found — fix the ✖ items above.") p.Hintf("For deeper triage, send tracebloc a support bundle: ./install-k8s.sh --diagnose") @@ -127,7 +144,95 @@ func runClusterDoctor( p.Warnf("Completed with warnings — review the ⚠ items above.") return nil default: - p.Successf("All checks passed — the cluster looks healthy.") + p.Successf("All checks passed — auth and cluster look healthy.") return nil } } + +// runAuthChecks reports on the CLI's own auth/config state (~/.tracebloc): are +// we signed in, to which env, is an active client selected, and does the backend +// still accept the token. It's the half of `cluster doctor` that diagnoses a +// failed *provision* rather than a sick cluster (RFC-0001 §8.5). Returns the +// worst status seen so the caller can fold it into the overall verdict. +func runAuthChecks(ctx context.Context, p *ui.Printer) doctor.Status { + p.Section("Auth & config") + + cfg, err := config.Load() + if err != nil { + p.Errorf("Config — couldn't read the CLI config: %v", err) + p.Hintf(" check ~/.tracebloc/config.json, or run `tracebloc login` to recreate it") + return doctor.StatusFail + } + if !cfg.SignedIn() { + p.Errorf("Sign-in — not signed in") + p.Hintf(" run `tracebloc login` (add --env dev|stg|prod for a non-prod backend)") + return doctor.StatusFail + } + + env := cfg.CurrentEnv + prof := cfg.Current() + if prof.Email != "" { + p.Successf("Sign-in — signed in to %s as %s", env, prof.Email) + } else { + p.Successf("Sign-in — signed in to %s", env) + } + + worst := doctor.StatusOK + if prof.ActiveClientID == "" { + p.Warnf("Active client — none selected for %s", env) + p.Hintf(" run `tracebloc client use ` (or `tracebloc client create`) to set the client this machine enrolls as") + worst = doctor.StatusWarn + } else { + p.Successf("Active client — %s", prof.ActiveClientID) + } + + // Live token check. Best-effort: an explicit 401 is a failure (expired / + // revoked → must re-login); a network/proxy error is only a warning, since + // we can't conclude the token itself is bad. + p.Detailf("verifying the token against %s …", api.BaseURL(env)) + client := newAPIClient(env) + client.Token = prof.Token + if _, werr := client.WhoAmI(ctx); werr != nil { + var ae *api.APIError + var ue *api.UpgradeRequiredError + switch { + case errors.As(werr, &ae) && ae.StatusCode == http.StatusUnauthorized: + p.Errorf("Backend auth — %s rejected the token (401)", api.BaseURL(env)) + p.Hintf(" your session expired or was revoked — run `tracebloc login`") + return doctor.StatusFail + case errors.As(werr, &ue): + // 426: the server enforces a newer CLI. That's a hard, actionable + // failure ("upgrade"), not a transient "couldn't verify" warning. + p.Errorf("Backend auth — this CLI is too old for %s (HTTP 426)", api.BaseURL(env)) + p.Hintf(" %s", ue.Error()) + return doctor.StatusFail + default: + p.Warnf("Backend auth — couldn't verify the token: %v", werr) + p.Hintf(" the backend may be unreachable from here — check your network / HTTP(S)_PROXY") + return worseStatus(worst, doctor.StatusWarn) + } + } + p.Successf("Backend auth — token valid at %s", api.BaseURL(env)) + return worst +} + +// worseStatus returns the more severe of two doctor statuses (Fail > Warn > OK). +func worseStatus(a, b doctor.Status) doctor.Status { + if a == doctor.StatusFail || b == doctor.StatusFail { + return doctor.StatusFail + } + if a == doctor.StatusWarn || b == doctor.StatusWarn { + return doctor.StatusWarn + } + return doctor.StatusOK +} + +// kubeconfigExitCode is 3 ("kubeconfig could not be loaded") unless the auth +// section also failed — then it escalates to 2 ("a check failed"), so a bad +// token isn't masked behind a kubeconfig-only exit code (Bugbot). +func kubeconfigExitCode(authStatus doctor.Status) int { + if authStatus == doctor.StatusFail { + return 2 + } + return 3 +} diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go new file mode 100644 index 00000000..87892cf6 --- /dev/null +++ b/internal/cli/doctor_test.go @@ -0,0 +1,155 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/tracebloc/cli/internal/api" + "github.com/tracebloc/cli/internal/config" + "github.com/tracebloc/cli/internal/doctor" + "github.com/tracebloc/cli/internal/ui" +) + +// stubBackend points the newAPIClient seam at an httptest server for one test. +func stubBackend(t *testing.T, h http.HandlerFunc) { + t.Helper() + srv := httptest.NewServer(h) + t.Cleanup(srv.Close) + orig := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = orig }) +} + +// cli#101: `cluster doctor` auth/config/token checks (RFC-0001 §8.5). These pin +// runAuthChecks — the half of doctor that diagnoses a failed *provision*. + +func TestRunAuthChecks_NotSignedIn(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { + t.Errorf("not signed in → want Fail, got %v", st) + } + if !strings.Contains(out.String(), "Auth & config") || !strings.Contains(out.String(), "not signed in") { + t.Errorf("missing auth section / not-signed-in line:\n%s", out.String()) + } +} + +func TestRunAuthChecks_TokenValid(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) + }) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusOK { + t.Errorf("valid token + active client → want OK, got %v;\n%s", st, out.String()) + } + if !strings.Contains(out.String(), "token valid") { + t.Errorf("missing token-valid line:\n%s", out.String()) + } +} + +func TestRunAuthChecks_TokenRejected401(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"detail":"Invalid token."}`)) + }) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { + t.Errorf("token rejected (401) → want Fail, got %v", st) + } + if !strings.Contains(out.String(), "rejected the token (401)") { + t.Errorf("missing 401 line:\n%s", out.String()) + } +} + +func TestRunAuthChecks_NoActiveClientWarns(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x"}, // signed in, but no active client selected + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) + }) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusWarn { + t.Errorf("valid token but no active client → want Warn, got %v;\n%s", st, out.String()) + } + if !strings.Contains(out.String(), "Active client — none") { + t.Errorf("missing no-active-client warning:\n%s", out.String()) + } +} + +// TestClusterDoctor_KubeconfigFailEscalatesWhenAuthFails pins the Bugbot fix: a +// kubeconfig load failure normally exits 3, but if the auth section ALSO failed +// (here: not signed in) it escalates to 2 so a bad token isn't masked as a +// kubeconfig-only problem. +func TestClusterDoctor_KubeconfigFailEscalatesWhenAuthFails(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // not signed in → auth Fail + var out bytes.Buffer + err := runClusterDoctor(context.Background(), ui.New(&out), "/nonexistent-kubeconfig-xyz", "", "") + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("kubeconfig-fail + auth-fail → want exit 2, got %v", err) + } +} + +// TestClusterDoctor_KubeconfigFailStays3WhenAuthOK: with auth healthy, a +// kubeconfig failure keeps the documented exit-3 contract. +func TestClusterDoctor_KubeconfigFailStays3WhenAuthOK(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok → auth OK + }) + var out bytes.Buffer + err := runClusterDoctor(context.Background(), ui.New(&out), "/nonexistent-kubeconfig-xyz", "", "") + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 3 { + t.Fatalf("kubeconfig-fail + auth-OK → want exit 3 (contract), got %v", err) + } +} + +// TestRunAuthChecks_426IsHardFailure pins the Bugbot fix: a 426 (server enforces +// a newer CLI) from the live token check is a hard "upgrade" failure, not a +// transient "couldn't verify — check your network" warning. +func TestRunAuthChecks_426IsHardFailure(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUpgradeRequired) // 426 + _, _ = w.Write([]byte(`{"error":"upgrade_required","min_version":"0.9.0"}`)) + }) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { + t.Errorf("426 from the token check → want Fail (not a transient Warn), got %v", st) + } + if !strings.Contains(out.String(), "too old") || !strings.Contains(out.String(), "426") { + t.Errorf("426 should report a clear 'too old / upgrade' failure, got:\n%s", out.String()) + } +} diff --git a/internal/cli/installlog.go b/internal/cli/installlog.go new file mode 100644 index 00000000..8c45a232 --- /dev/null +++ b/internal/cli/installlog.go @@ -0,0 +1,62 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "time" + + "github.com/tracebloc/cli/internal/config" +) + +// installLog is an append-only, timestamped record of a connect/provision run, +// always written to ~/.tracebloc/install-.log regardless of --verbose. The +// connect flow is zero-prompt on a headless box, so a failed run must leave a +// full trace on disk to inspect or send to support even when the terminal stayed +// quiet (RFC-0001 §8.5). +// +// A nil *installLog is a no-op on every method, so callers never have to guard: +// logging must never be what fails a provision. +type installLog struct { + f *os.File +} + +// newInstallLog creates ~/.tracebloc/install-.log (mode 0600 — it can carry +// hostnames and paths). It returns the log (nil if it couldn't be opened) and +// the path it used, so the caller can surface the path without ever failing the +// command over logging. +func newInstallLog() (*installLog, string) { + dir, err := config.Dir() + if err != nil { + return nil, "" + } + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, "" + } + path := filepath.Join(dir, "install-"+time.Now().UTC().Format("20060102-150405")+".log") + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) + if err != nil { + // No file was created — return an empty path so the caller never + // advertises a "Full log:" location that doesn't exist (Bugbot). + return nil, "" + } + l := &installLog{f: f} + l.Logf("tracebloc connect/provision log") + return l, path +} + +// Logf appends a UTC-timestamped line. Safe on a nil receiver (no-op). +func (l *installLog) Logf(format string, a ...any) { + if l == nil || l.f == nil { + return + } + _, _ = fmt.Fprintf(l.f, "%s %s\n", time.Now().UTC().Format(time.RFC3339), fmt.Sprintf(format, a...)) +} + +// Close closes the underlying file. Safe on a nil receiver (no-op). +func (l *installLog) Close() { + if l == nil || l.f == nil { + return + } + _ = l.f.Close() +} diff --git a/internal/cli/interactive.go b/internal/cli/interactive.go index bb6478d9..9ad71d01 100644 --- a/internal/cli/interactive.go +++ b/internal/cli/interactive.go @@ -17,8 +17,8 @@ import ( // promptCategories is the ordered list offered by the interactive // category picker. It derives from the push registry's CLI-supported -// set — the exact categories runDatasetPush's gate accepts — so the -// picker can't drift from what `dataset push` actually supports. +// set — the exact categories runDataIngest's gate accepts — so the +// picker can't drift from what `data ingest` actually supports. // semantic_/instance_segmentation are excluded (CLISupported=false) // until they're implemented. var promptCategories = push.SupportedCategoryIDs() @@ -30,7 +30,7 @@ var promptCategories = push.SupportedCategoryIDs() // uses to let cluster code run against a fake clientset. // errInteractiveCancelled is returned when the user declines the // confirm prompt or hits Ctrl-C. It's control flow, not a failure: -// runDatasetPush maps it to a clean exit (0) with a "Cancelled" note. +// runDataIngest maps it to a clean exit (0) with a "Cancelled" note. var errInteractiveCancelled = errors.New("cancelled by user") type prompter interface { @@ -102,7 +102,7 @@ func isInteractiveTTY() bool { return term.IsTerminal(int(os.Stdin.Fd())) && term.IsTerminal(int(os.Stdout.Fd())) } -// runInteractive fills the gaps in a's core push fields by prompting, +// runInteractive fills the gaps in a's core ingest fields by prompting, // then returns. It only prompts for what's still missing, so flags the // user already passed win. categorySet says whether --category was set // explicitly (vs left at its non-empty default), which would otherwise @@ -110,8 +110,8 @@ func isInteractiveTTY() bool { // // Mutates a through the pointer. PR-b adds category-specific prompts // (target-size, schema, number-of-keypoints) + a confirm screen. -func runInteractive(p *ui.Printer, pr prompter, a *runDatasetPushArgs, categorySet bool) error { - p.PromptHeader("Let's set up your dataset push") +func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, categorySet bool) error { + p.PromptHeader("Let's set up your data ingest") p.Hintf("Press Enter to accept a default; Ctrl-C to cancel.") prompted := false @@ -177,11 +177,11 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDatasetPushArgs, categoryS } prompted = prompted || cp - // Confirm only when we actually prompted something — a push that's + // Confirm only when we actually prompted something — an ingest that's // fully specified by flags (on a TTY) isn't nagged with a confirm. if prompted { renderReview(p, a) - ok, err := pr.Confirm("Proceed with the push?", true) + ok, err := pr.Confirm("Proceed with the ingest?", true) if err != nil { return err } @@ -195,7 +195,7 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDatasetPushArgs, categoryS // promptCategorySpecific prompts for the inputs a particular category // needs beyond the core fields, filling only the gaps. Returns whether // it prompted anything (so the caller knows to show the confirm). -func promptCategorySpecific(p *ui.Printer, pr prompter, a *runDatasetPushArgs) (bool, error) { +func promptCategorySpecific(p *ui.Printer, pr prompter, a *runDataIngestArgs) (bool, error) { cat := a.Spec.Category prompted := false switch { @@ -257,9 +257,9 @@ func promptCategorySpecific(p *ui.Printer, pr prompter, a *runDatasetPushArgs) ( return prompted, nil } -// renderReview prints the assembled push inputs before the confirm +// renderReview prints the assembled ingest inputs before the confirm // prompt, so the user sees exactly what's about to happen. -func renderReview(p *ui.Printer, a *runDatasetPushArgs) { +func renderReview(p *ui.Printer, a *runDataIngestArgs) { p.Section("Review") p.Field("path", a.LocalPath) p.Field("category", a.Spec.Category) diff --git a/internal/cli/interactive_test.go b/internal/cli/interactive_test.go index d2c9295d..d1daf298 100644 --- a/internal/cli/interactive_test.go +++ b/internal/cli/interactive_test.go @@ -61,7 +61,7 @@ func TestRunInteractive_FillsAllWhenEmpty(t *testing.T) { "Intent": "test", "Label column": "churned", }} - a := &runDatasetPushArgs{Spec: push.SpecArgs{Category: "image_classification"}} + a := &runDataIngestArgs{Spec: push.SpecArgs{Category: "image_classification"}} if err := runInteractive(discardPrinter(), f, a, false); err != nil { t.Fatalf("runInteractive: %v", err) @@ -92,7 +92,7 @@ func TestRunInteractive_ShowsExampleHints(t *testing.T) { "Path to your dataset directory": "./d", "Destination table name": "churn_train", }} - a := &runDatasetPushArgs{Spec: push.SpecArgs{Category: "tabular_regression"}} + a := &runDataIngestArgs{Spec: push.SpecArgs{Category: "tabular_regression"}} var buf bytes.Buffer p := ui.New(&buf, ui.WithColor(false)) @@ -118,7 +118,7 @@ func TestRunInteractive_SkipsProvidedValues(t *testing.T) { f := &fakePrompter{answers: map[string]string{}} // text_classification has no category-specific prompts, so with all // core fields set + an explicit --category, nothing is asked. - a := &runDatasetPushArgs{ + a := &runDataIngestArgs{ LocalPath: "./data", Spec: push.SpecArgs{ Category: "text_classification", Table: "t", Intent: "train", LabelColumn: "label", @@ -136,7 +136,7 @@ func TestRunInteractive_SkipsProvidedValues(t *testing.T) { // the optional resolution left blank means auto-detect. func TestRunInteractive_Keypoint(t *testing.T) { f := &fakePrompter{answers: map[string]string{"Number of keypoints per sample": "17"}} - a := &runDatasetPushArgs{ + a := &runDataIngestArgs{ LocalPath: "./kp", Spec: push.SpecArgs{Category: "keypoint_detection", Table: "kp_train", Intent: "train", LabelColumn: "image_label"}, } @@ -155,7 +155,7 @@ func TestRunInteractive_Keypoint(t *testing.T) { // (regression-class) and leaves the schema to inference. func TestRunInteractive_TabularRegression(t *testing.T) { f := &fakePrompter{answers: map[string]string{"Label policy": "passthrough"}} - a := &runDatasetPushArgs{ + a := &runDataIngestArgs{ LocalPath: "./tab", Spec: push.SpecArgs{Category: "tabular_regression", Table: "reg_train", Intent: "train", LabelColumn: "Target"}, } @@ -180,7 +180,7 @@ func TestRunInteractive_Cancel(t *testing.T) { } // path is prompted (→ prompted=true → a confirm is shown); the rest // is pre-set so we reach the confirm cleanly. - a := &runDatasetPushArgs{Spec: push.SpecArgs{ + a := &runDataIngestArgs{Spec: push.SpecArgs{ Category: "image_classification", Table: "t", Intent: "train", LabelColumn: "label", }} if err := runInteractive(discardPrinter(), f, a, true); !errors.Is(err, errInteractiveCancelled) { @@ -195,7 +195,7 @@ func TestRunInteractive_MLMSkipsLabel(t *testing.T) { "Destination table name": "mlm_train", "Intent": "train", }} - a := &runDatasetPushArgs{ + a := &runDataIngestArgs{ LocalPath: "./data", Spec: push.SpecArgs{Category: "masked_language_modeling"}, } @@ -216,7 +216,7 @@ func TestRunInteractive_MLMSkipsLabel(t *testing.T) { // push.ValidateTableName, so an unsafe name surfaces as an error. func TestRunInteractive_RejectsBadTable(t *testing.T) { f := &fakePrompter{answers: map[string]string{"Destination table name": "../bad"}} - a := &runDatasetPushArgs{ + a := &runDataIngestArgs{ LocalPath: "./data", Spec: push.SpecArgs{Category: "image_classification", Intent: "train", LabelColumn: "label"}, } diff --git a/internal/cli/root.go b/internal/cli/root.go index 0bf6f8c0..2b7868dd 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -8,9 +8,12 @@ package cli import ( "io" + "os" + "strings" "github.com/spf13/cobra" + "github.com/tracebloc/cli/internal/api" "github.com/tracebloc/cli/internal/ui" ) @@ -32,6 +35,10 @@ type BuildInfo struct { // global state has historically been a source of test interference, // and constructing fresh trees per call is cheap. func NewRootCmd(info BuildInfo) *cobra.Command { + // Record the CLI version for the User-Agent sent on every backend request + // (RFC-0001 §14 R11 / backend#888): "tracebloc-cli/ (/)". + api.SetUserAgent(info.Version) + root := &cobra.Command{ Use: "tracebloc", Short: "tracebloc — interactive data ingestion for your cluster", @@ -43,7 +50,7 @@ developer's workstation. The dominant workflow: - tracebloc dataset push ./my-data \ + tracebloc data ingest ./my-data \ --table cats_dogs_train \ --category image_classification \ --intent train \ @@ -54,7 +61,7 @@ on the cluster's shared PVC, submitting the ingestion request, watching the resulting Job, and reporting the outcome. Customers never touch Helm, never edit YAML, never run kubectl cp manually. -This binary implements the full v0.1 ingestion path: ` + "`dataset push`" + ` +This binary implements the full v0.1 ingestion path: ` + "`data ingest`" + ` (the dominant workflow above), ` + "`ingest validate`" + ` for a local schema check, ` + "`cluster info`" + ` for discovery diagnostics, plus ` + "`version`" + ` and ` + "`completion`" + `. See @@ -75,12 +82,17 @@ what's planned next.`, // CI / log capture where stdout might still look like a terminal. root.PersistentFlags().Bool("plain", false, "disable color and decorative output (also honors $NO_COLOR)") + // --verbose streams the per-step detail (device-flow → provision → install) + // that's hidden by default; also enabled by $TRACEBLOC_LOG_LEVEL=debug. The + // default output stays quiet — a handful of ✔ lines (RFC-0001 §8.5). + root.PersistentFlags().Bool("verbose", false, + "stream detailed step-by-step progress (also via $TRACEBLOC_LOG_LEVEL=debug)") // Subcommands. New phases append here. root.AddCommand(newVersionCmd(info)) root.AddCommand(newIngestCmd()) root.AddCommand(newClusterCmd()) - root.AddCommand(newDatasetCmd()) + root.AddCommand(newDataCmd()) // RFC-0001 (backend#830): browser sign-in + client provisioning. root.AddCommand(newLoginCmd()) root.AddCommand(newLogoutCmd()) @@ -97,9 +109,9 @@ what's planned next.`, p := printerFor(cmd) p.Banner("tracebloc", "interactive data ingestion for your cluster") p.Section("Get started") - p.Infof("tracebloc dataset push — stage + ingest a dataset interactively (or use --help to see flags)") - p.Infof("tracebloc dataset list — list datasets ingested in the cluster") - p.Infof("tracebloc dataset rm
— delete a pushed dataset (its table + files)") + p.Infof("tracebloc data ingest — stage + ingest a dataset interactively (or use --help to see flags)") + p.Infof("tracebloc data list — list datasets ingested in the cluster") + p.Infof("tracebloc data delete
— delete an ingested dataset (its table + files)") p.Infof("tracebloc cluster info — check the CLI can reach your cluster") p.Infof("tracebloc ingest validate f.yaml — validate an ingest.yaml locally") p.Newline() @@ -122,8 +134,26 @@ func printerFor(cmd *cobra.Command) *ui.Printer { // dataset push's --output-json mode, which routes human output to // stderr so stdout carries only the JSON result. func printerForWriter(cmd *cobra.Command, w io.Writer) *ui.Printer { + var opts []ui.Option if plain, _ := cmd.Flags().GetBool("plain"); plain { - return ui.New(w, ui.WithColor(false)) + opts = append(opts, ui.WithColor(false)) + } + if verboseRequested(cmd) { + opts = append(opts, ui.WithVerbose(true)) + } + return ui.New(w, opts...) +} + +// verboseRequested reports whether the user asked for verbose output, via the +// --verbose flag or $TRACEBLOC_LOG_LEVEL (debug/trace/verbose). The flag wins; +// the env var lets a headless / scripted run opt in without editing the command. +func verboseRequested(cmd *cobra.Command) bool { + if v, err := cmd.Flags().GetBool("verbose"); err == nil && v { + return true + } + switch strings.ToLower(os.Getenv("TRACEBLOC_LOG_LEVEL")) { + case "debug", "trace", "verbose": + return true } - return ui.New(w) + return false } diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 6ac7f028..218be56d 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -41,7 +41,7 @@ func TestRootCmd_HomeScreen(t *testing.T) { if err := root.Execute(); err != nil { t.Fatalf("bare root failed: %v\n%s", err, out.String()) } - for _, want := range []string{"tracebloc", "dataset push", "dataset list", "dataset rm", "cluster info"} { + for _, want := range []string{"tracebloc", "data ingest", "data list", "data delete", "cluster info"} { if !strings.Contains(out.String(), want) { t.Errorf("home screen missing %q:\n%s", want, out.String()) } diff --git a/internal/cli/verbose_install_test.go b/internal/cli/verbose_install_test.go new file mode 100644 index 00000000..2c1f241c --- /dev/null +++ b/internal/cli/verbose_install_test.go @@ -0,0 +1,228 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/tracebloc/cli/internal/api" + "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/config" + "github.com/tracebloc/cli/internal/ui" +) + +// loginStub wires a minimal happy-path device flow via the auth_test seam. +func loginStub(t *testing.T) { + withTestBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/device/code": + _, _ = w.Write([]byte(`{"device_code":"dc","user_code":"X","verification_uri":"https://x/activate","interval":5}`)) + case "/device/token": + _, _ = w.Write([]byte(`{"token":"cat_v"}`)) + case "/userinfo/": + _, _ = w.Write([]byte(`{"email":"e@co","account":"A"}`)) + } + }) +} + +// cli#101 (RFC-0001 §8.5): --verbose streams the device-flow detail; the default +// output stays quiet. + +func TestLogin_VerboseStreamsDetail(t *testing.T) { + loginStub(t) + out, err := runCmd(t, "--verbose", "login") + if err != nil { + t.Fatalf("login: %v", err) + } + if !strings.Contains(out, "requesting a device code") { + t.Errorf("--verbose should stream the device-flow detail, got:\n%s", out) + } +} + +func TestLogin_QuietByDefault(t *testing.T) { + loginStub(t) + out, err := runCmd(t, "login") + if err != nil { + t.Fatalf("login: %v", err) + } + if strings.Contains(out, "requesting a device code") { + t.Errorf("default output should stay quiet (no verbose detail), got:\n%s", out) + } +} + +// TestClientCreate_FailurePrintsResumeAndWritesInstallLog pins the §8.5 failure +// path: a failed provision prints the (idempotent) resume command + the doctor +// pointer, and every run leaves an install-.log on disk. +func TestClientCreate_FailurePrintsResumeAndWritesInstallLog(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok"}, + }}).Save(); err != nil { + t.Fatal(err) + } + // List succeeds; the provision POST 500s → create fails. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`[]`)) + return + } + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(srv.Close) + origClient := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = origClient }) + origCID := readClusterID + readClusterID = func(context.Context, cluster.KubeconfigOptions) (string, error) { + return "", errors.New("no cluster (test)") + } + t.Cleanup(func() { readClusterID = origCID }) + + var out bytes.Buffer + err := runClientCreate(context.Background(), ui.New(&out), nil, + clientCreateOpts{name: "My Client", location: "DE", yes: true}) + if err == nil { + t.Fatal("expected the provision to fail (POST 500)") + } + // Resume hint: the idempotent re-run command (name with a space gets quoted) + // + the doctor pointer. + if !strings.Contains(out.String(), "tracebloc client create --name 'My Client' --location DE") { + t.Errorf("missing / incorrect resume command:\n%s", out.String()) + } + if !strings.Contains(out.String(), "cluster doctor") { + t.Errorf("missing `cluster doctor` pointer:\n%s", out.String()) + } + // An install-.log is written, recording the failure. + logs, _ := filepath.Glob(filepath.Join(dir, "install-*.log")) + if len(logs) == 0 { + t.Fatal("no install-*.log written") + } + raw, _ := os.ReadFile(logs[0]) + if !strings.Contains(string(raw), "FAILED") { + t.Errorf("install log should record the failure:\n%s", raw) + } +} + +// TestClientCreate_CancelLogsCancelledNotDone pins the Bugbot fix: declining the +// confirm prompt is a user abort, not a successful provision — the install log +// must record "cancelled", never "done". +func TestClientCreate_CancelLogsCancelledNotDone(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok"}, + }}).Save(); err != nil { + t.Fatal(err) + } + posted := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + posted = true + } + _, _ = w.Write([]byte(`[]`)) + })) + t.Cleanup(srv.Close) + origClient := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = origClient }) + origCID := readClusterID + readClusterID = func(context.Context, cluster.KubeconfigOptions) (string, error) { + return "", errors.New("no cluster (test)") + } + t.Cleanup(func() { readClusterID = origCID }) + + confirmNo := false + pr := &fakePrompter{answers: map[string]string{}, confirm: &confirmNo} + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), pr, + clientCreateOpts{name: "Lab", location: "DE"}); err != nil { + t.Fatalf("declining the confirm should be a clean exit, got: %v", err) + } + if posted { + t.Error("no client should be POSTed when the user declines") + } + logs, _ := filepath.Glob(filepath.Join(dir, "install-*.log")) + if len(logs) == 0 { + t.Fatal("no install-*.log written") + } + raw, _ := os.ReadFile(logs[0]) + if !strings.Contains(string(raw), "cancelled") { + t.Errorf("install log should record the cancel, got:\n%s", raw) + } + if strings.Contains(string(raw), "done") { + t.Errorf("a cancelled run must NOT be logged as 'done':\n%s", raw) + } +} + +// TestClientCreate_ResumeCommandIncludesPromptedValues pins the Bugbot fix: when +// name/location come from interactive prompts (not flags), a failed provision's +// resume command must still include them — opts alone would omit them. +func TestClientCreate_ResumeCommandIncludesPromptedValues(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok"}, + }}).Save(); err != nil { + t.Fatal(err) + } + // list ok; the provision POST 500s after the user confirms. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`[]`)) + return + } + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(srv.Close) + origClient := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = origClient }) + origCID := readClusterID + readClusterID = func(context.Context, cluster.KubeconfigOptions) (string, error) { + return "", errors.New("no cluster (test)") + } + t.Cleanup(func() { readClusterID = origCID }) + + confirmYes := true + pr := &fakePrompter{answers: map[string]string{ + "Client name": "Prompted Lab", + "Location zone (e.g. DE)": "FR", + }, confirm: &confirmYes} + var out bytes.Buffer + // No name/location flags — both come from the prompts. + if err := runClientCreate(context.Background(), ui.New(&out), pr, clientCreateOpts{}); err == nil { + t.Fatal("expected the provision to fail (POST 500)") + } + if !strings.Contains(out.String(), "--name 'Prompted Lab' --location FR") { + t.Errorf("resume command should carry the PROMPTED name + location, got:\n%s", out.String()) + } +} + +// TestNewInstallLog_NoPathWhenFileOpenFails pins the Bugbot fix: when the log +// file can't be opened, newInstallLog returns an empty path (not a path to a +// file that was never written), so the failure hint won't advertise it. +func TestNewInstallLog_NoPathWhenFileOpenFails(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("runs as root — directory perms don't restrict file creation") + } + dir := t.TempDir() + if err := os.Chmod(dir, 0o500); err != nil { // read-only dir → OpenFile fails + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(dir, 0o700) }) // restore so TempDir cleanup can remove it + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + + l, path := newInstallLog() + if l != nil { + l.Close() + } + if path != "" { + t.Errorf("OpenFile failed → path must be empty (no file created), got %q", path) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 5145fad2..da625574 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,20 @@ -// Package config persists the tracebloc CLI's user state — the backend -// environment, the user token from `tracebloc login`, and the active client -// for this machine — at ~/.tracebloc/config.json (mode 0600, it holds a -// token). RFC-0001 (backend#830). +// Package config persists the tracebloc CLI's user state at +// ~/.tracebloc/config.json (mode 0600, it holds tokens). RFC-0001 (backend#830). +// +// The on-disk format is v2 (RFC-0001 Appendix C.8): env-scoped profiles, so a +// user signed into dev / stg / prod keeps an independent token + active-client +// pointer per env. `login --env X` switches current_env without touching the +// other profiles — fixing the v1 bug (R10) where the single flat +// {env,token,active_client_id} record let a `login --env` strand the previous +// env's active_client_id and silently target the wrong client. +// +// { "version": 2, "current_env": "prod", +// "profiles": { +// "dev": { "email", "token", "expires_at", "active_client_id" }, +// "stg": { … }, "prod": { … } } } +// +// A v1 file (the flat cli#83 schema) auto-migrates to v2 on first read, with no +// data loss — its single record is wrapped under profiles[env]. package config import ( @@ -13,13 +26,62 @@ import ( "path/filepath" ) -// Config is the on-disk CLI state. Every field is omitempty so a -// partially-configured file stays small and forward-compatible. -type Config struct { - Env string `json:"env,omitempty"` // dev|stg|prod (mirrors CLIENT_ENV) +// schemaVersion is the current on-disk format. Anything lower (incl. a v1 file +// with no "version" key, which decodes as 0) is migrated on Load. +const schemaVersion = 2 + +// defaultEnv is the env a v1 record migrates under when its env was unset — the +// historical v1 default (mirrors api.EnvProd; kept literal to avoid importing api +// into this lower-level package). +const defaultEnv = "prod" + +// Profile is one env's signed-in state. Every field is omitempty so a +// partially-configured profile stays small and forward-compatible. +type Profile struct { Email string `json:"email,omitempty"` // who is signed in (display only) Token string `json:"token,omitempty"` // user token from device login - ActiveClientID string `json:"active_client_id,omitempty"` // client this machine enrolls as + ExpiresAt string `json:"expires_at,omitempty"` // token expiry (RFC 3339), when known + ActiveClientID string `json:"active_client_id,omitempty"` // client this machine enrolls as, for THIS env +} + +// Config is the on-disk CLI state: env-scoped profiles plus the current env. +type Config struct { + Version int `json:"version"` + CurrentEnv string `json:"current_env,omitempty"` + Profiles map[string]*Profile `json:"profiles,omitempty"` +} + +// Profile returns env's profile, creating an empty one (stored in the map) if +// absent. The returned pointer is live: mutate it then Save() to persist. +func (c *Config) Profile(env string) *Profile { + if c.Profiles == nil { + c.Profiles = map[string]*Profile{} + } + p := c.Profiles[env] + if p == nil { + p = &Profile{} + c.Profiles[env] = p + } + return p +} + +// Current returns the profile for the current env. When no env is selected it +// returns a fresh empty profile (a read-only "not signed in" view) rather than +// nil, so callers can read fields without a guard. +func (c *Config) Current() *Profile { + if c.CurrentEnv == "" { + return &Profile{} + } + return c.Profile(c.CurrentEnv) +} + +// SignedIn reports whether the current env has a stored token. +func (c *Config) SignedIn() bool { + if c.CurrentEnv == "" { + return false + } + p := c.Profiles[c.CurrentEnv] + return p != nil && p.Token != "" } // Dir is the config directory: $TRACEBLOC_CONFIG_DIR if set (tests / ops @@ -44,8 +106,8 @@ func Path() (string, error) { return filepath.Join(dir, "config.json"), nil } -// Load reads the config. A missing file is NOT an error — it returns an empty -// Config (a fresh machine that has never run `login`). +// Load reads the config, migrating a v1 file to v2 in memory. A missing file is +// NOT an error — it returns an empty v2 Config (a machine that's never run login). func Load() (*Config, error) { path, err := Path() if err != nil { @@ -53,21 +115,80 @@ func Load() (*Config, error) { } data, err := os.ReadFile(path) if errors.Is(err, fs.ErrNotExist) { - return &Config{}, nil + return &Config{Version: schemaVersion, Profiles: map[string]*Profile{}}, nil } if err != nil { return nil, fmt.Errorf("reading %s: %w", path, err) } + + // Detect the on-disk schema. v1 (cli#83) had no "version" key and a flat + // {env,email,token,active_client_id}; it decodes here as version 0. Migrate + // only a GENUINE v1 record: an old version AND no v2 `profiles` object — so a + // v2-shaped file with a missing/wrong version is still parsed as v2 and never + // has its profiles silently dropped by migrateV1. + var probe struct { + Version int `json:"version"` + Profiles json.RawMessage `json:"profiles"` + } + if err := json.Unmarshal(data, &probe); err != nil { + return nil, fmt.Errorf("parsing %s: %w", path, err) + } + if probe.Version < schemaVersion && len(probe.Profiles) == 0 { + return migrateV1(data, path) + } var c Config if err := json.Unmarshal(data, &c); err != nil { return nil, fmt.Errorf("parsing %s: %w", path, err) } + if c.Profiles == nil { + c.Profiles = map[string]*Profile{} + } return &c, nil } +// migrateV1 wraps a v1 flat record under profiles[env] (RFC-0001 Appendix C.8), +// with no data loss. The first Save rewrites the file as v2. +func migrateV1(data []byte, path string) (*Config, error) { + var v1 struct { + Env string `json:"env"` + Email string `json:"email"` + Token string `json:"token"` + ActiveClientID string `json:"active_client_id"` + } + if err := json.Unmarshal(data, &v1); err != nil { + return nil, fmt.Errorf("parsing %s: %w", path, err) + } + c := &Config{Version: schemaVersion, Profiles: map[string]*Profile{}} + // A signed-in v1 record carries over under its env; an empty / logged-out v1 + // file just becomes an empty v2 (nothing to migrate). + if v1.Token != "" || v1.Email != "" || v1.ActiveClientID != "" { + env := v1.Env + if env == "" { + env = defaultEnv + } + c.CurrentEnv = env + c.Profiles[env] = &Profile{ + Email: v1.Email, + Token: v1.Token, + ActiveClientID: v1.ActiveClientID, + } + } + return c, nil +} + // Save writes the config 0600 (creating the dir 0700), atomically: a temp file -// in the same dir then a rename, so a crash mid-write can't truncate the token. +// in the same dir then a rename, so a crash mid-write can't truncate a token. func (c *Config) Save() error { + c.Version = schemaVersion + // Prune fully-empty profiles (e.g. the current env's profile after logout + // clears it) so the file stays tidy; an absent profile reads as "not signed + // in" for that env. + for env, p := range c.Profiles { + if p == nil || *p == (Profile{}) { + delete(c.Profiles, env) + } + } + dir, err := Dir() if err != nil { return err @@ -105,8 +226,8 @@ func (c *Config) Save() error { return nil } -// Clear removes the config file (full sign-out + reset). A missing file is not -// an error. +// Clear removes the config file (full sign-out + reset, all envs). A missing +// file is not an error. func Clear() error { path, err := Path() if err != nil { @@ -117,6 +238,3 @@ func Clear() error { } return nil } - -// SignedIn reports whether a user token is stored. -func (c *Config) SignedIn() bool { return c.Token != "" } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 53dd368b..e49934ae 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,12 +2,16 @@ package config import ( "os" + "path/filepath" + "strings" "testing" ) func TestSaveLoadRoundTrip(t *testing.T) { t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - in := &Config{Env: "dev", Email: "a@b.com", Token: "tok123", ActiveClientID: "edge_1"} + in := &Config{CurrentEnv: "dev", Profiles: map[string]*Profile{ + "dev": {Email: "a@b.com", Token: "tok123", ActiveClientID: "edge_1"}, + }} if err := in.Save(); err != nil { t.Fatal(err) } @@ -15,17 +19,19 @@ func TestSaveLoadRoundTrip(t *testing.T) { if err != nil { t.Fatal(err) } - if *out != *in { - t.Fatalf("round-trip mismatch: %+v != %+v", out, in) + if out.CurrentEnv != "dev" || !out.SignedIn() { + t.Fatalf("round-trip: %+v", out) } - if !out.SignedIn() { - t.Error("SignedIn should be true when a token is present") + p := out.Profile("dev") + if p.Email != "a@b.com" || p.Token != "tok123" || p.ActiveClientID != "edge_1" { + t.Errorf("round-trip profile mismatch: %+v", p) } } func TestSaveIs0600(t *testing.T) { t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&Config{Token: "secret"}).Save(); err != nil { + c := &Config{CurrentEnv: "prod", Profiles: map[string]*Profile{"prod": {Token: "secret"}}} + if err := c.Save(); err != nil { t.Fatal(err) } p, _ := Path() @@ -34,7 +40,7 @@ func TestSaveIs0600(t *testing.T) { t.Fatal(err) } if fi.Mode().Perm() != 0o600 { - t.Errorf("config mode = %v, want 0600 (it holds a token)", fi.Mode().Perm()) + t.Errorf("config mode = %v, want 0600 (it holds tokens)", fi.Mode().Perm()) } } @@ -51,7 +57,7 @@ func TestLoadMissingIsEmpty(t *testing.T) { func TestClear(t *testing.T) { t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - _ = (&Config{Token: "x"}).Save() + _ = (&Config{CurrentEnv: "prod", Profiles: map[string]*Profile{"prod": {Token: "x"}}}).Save() if err := Clear(); err != nil { t.Fatal(err) } @@ -62,3 +68,134 @@ func TestClear(t *testing.T) { t.Errorf("Clear on a missing file should be nil, got %v", err) } } + +// TestMigrateV1ToV2 pins the v1 (flat cli#83 schema) → v2 migration: the single +// record is wrapped under profiles[env], no data loss, and the next Save rewrites +// the file as v2 on disk. +func TestMigrateV1ToV2(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + v1 := `{"env":"dev","email":"a@b.com","token":"tok123","active_client_id":"7"}` + if err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(v1), 0o600); err != nil { + t.Fatal(err) + } + c, err := Load() + if err != nil { + t.Fatal(err) + } + if c.Version != 2 { + t.Errorf("version = %d, want 2", c.Version) + } + if c.CurrentEnv != "dev" { + t.Errorf("current_env = %q, want dev", c.CurrentEnv) + } + if p := c.Profile("dev"); p.Token != "tok123" || p.Email != "a@b.com" || p.ActiveClientID != "7" { + t.Errorf("migrated dev profile = %+v, want token/email/active carried over", p) + } + if !c.SignedIn() { + t.Error("a migrated v1 record with a token should be signed in") + } + // Persisting rewrites the file as v2. + if err := c.Save(); err != nil { + t.Fatal(err) + } + raw, _ := os.ReadFile(filepath.Join(dir, "config.json")) + if !strings.Contains(string(raw), `"version": 2`) || !strings.Contains(string(raw), `"profiles"`) { + t.Errorf("on-disk file is not v2 after save:\n%s", raw) + } +} + +// TestMigrateV1EmptyEnvDefaultsProd: a v1 token with no env migrates under the +// historical default (prod), not a profile keyed by the empty string. +func TestMigrateV1EmptyEnvDefaultsProd(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(`{"token":"t"}`), 0o600); err != nil { + t.Fatal(err) + } + c, err := Load() + if err != nil { + t.Fatal(err) + } + if c.CurrentEnv != "prod" || c.Profile("prod").Token != "t" { + t.Errorf("empty-env v1 should migrate under prod, got current=%q profiles=%+v", c.CurrentEnv, c.Profiles) + } +} + +// TestProfilesAreEnvScoped_NoClobber is the R10 fix: `login --env X` switches +// current_env and writes X's profile WITHOUT touching the other envs' active +// client pointers. Simulates dev → prod → dev and asserts dev's active client +// survives (the v1 flat schema clobbered it). +func TestProfilesAreEnvScoped_NoClobber(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + + // Sign into dev, pick a dev client. + c, _ := Load() + c.CurrentEnv = "dev" + c.Profile("dev").Token = "dev-tok" + c.Profile("dev").ActiveClientID = "11" + if err := c.Save(); err != nil { + t.Fatal(err) + } + + // `login --env prod`: switch env, write prod's profile. + c, _ = Load() + c.CurrentEnv = "prod" + c.Profile("prod").Token = "prod-tok" + c.Profile("prod").ActiveClientID = "22" + if err := c.Save(); err != nil { + t.Fatal(err) + } + + // `login --env dev` again: switch back. + c, _ = Load() + c.CurrentEnv = "dev" + c.Profile("dev").Token = "dev-tok2" + if err := c.Save(); err != nil { + t.Fatal(err) + } + + c, _ = Load() + if got := c.Profile("dev").ActiveClientID; got != "11" { + t.Errorf("dev active_client_id = %q, want 11 (clobbered across envs — R10)", got) + } + if got := c.Profile("prod").ActiveClientID; got != "22" { + t.Errorf("prod active_client_id = %q, want 22 (other env touched)", got) + } +} + +// TestSignedInRequiresCurrentEnvToken: a profile that exists for a non-current +// env doesn't count as signed in; only the current env's token does. +func TestSignedInRequiresCurrentEnvToken(t *testing.T) { + c := &Config{CurrentEnv: "dev", Profiles: map[string]*Profile{"prod": {Token: "x"}}} + if c.SignedIn() { + t.Error("signed-in should be false when only a non-current env has a token") + } + c.Profile("dev").Token = "y" + if !c.SignedIn() { + t.Error("signed-in should be true once the current env has a token") + } +} + +// TestLoadV2ShapedWithoutVersion_NotMigrated pins the Bugbot fix: a config that +// already has v2 `profiles` but a missing/old version must be parsed as v2, not +// misread as v1 and migrated (which reads only flat fields → would drop profiles). +func TestLoadV2ShapedWithoutVersion_NotMigrated(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + // No "version" key, but a v2-shaped profiles object. + raw := `{"current_env":"dev","profiles":{"dev":{"token":"keep-me","active_client_id":"7"}}}` + if err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(raw), 0o600); err != nil { + t.Fatal(err) + } + c, err := Load() + if err != nil { + t.Fatal(err) + } + if !c.SignedIn() { + t.Fatalf("v2-shaped config without a version was misread as v1 (profiles dropped): %+v", c) + } + if p := c.Profile("dev"); p.Token != "keep-me" || p.ActiveClientID != "7" { + t.Errorf("dev profile not preserved: %+v", p) + } +} diff --git a/internal/push/category.go b/internal/push/category.go index 73f0ffdf..24e23f02 100644 --- a/internal/push/category.go +++ b/internal/push/category.go @@ -70,6 +70,8 @@ var categoryRegistry = []CategorySpec{ UnsupportedNote: "not implemented"}, {ID: "causal_language_modeling", Family: FamilyText, Label: "Causal language modeling", CLISupported: false, UnsupportedNote: "schema-recognized (data-ingestors#805); `tracebloc ingest` discover/build for its raw-.txt / prompt\\tcompletion `texts` layout is pending"}, + {ID: "seq2seq", Family: FamilyText, Label: "Sequence-to-sequence", CLISupported: false, + UnsupportedNote: "schema-recognized; `tracebloc ingest` discover/build for its raw-.txt / source\\ttarget `texts` layout is pending"}, {ID: "token_classification", Family: FamilyText, Label: "Token classification", CLISupported: false, UnsupportedNote: "schema-recognized; the CLI doesn't stage its per-token-label `texts` layout yet"}, } diff --git a/internal/push/category_registry_test.go b/internal/push/category_registry_test.go index 17ce1f3c..c37e94bc 100644 --- a/internal/push/category_registry_test.go +++ b/internal/push/category_registry_test.go @@ -17,7 +17,7 @@ func TestRegistryKnownCategories(t *testing.T) { "image_classification", "object_detection", "keypoint_detection", "semantic_segmentation", "instance_segmentation", "text_classification", "token_classification", - "masked_language_modeling", "causal_language_modeling", + "masked_language_modeling", "causal_language_modeling", "seq2seq", "tabular_classification", "tabular_regression", "time_series_forecasting", "time_to_event_prediction", } @@ -44,9 +44,9 @@ func TestSupportedCategories(t *testing.T) { t.Errorf("SupportedCategoryIDs returned %q but IsCLISupported is false", id) } } - // segmentation + causal_language_modeling are known but not yet pushable, - // and must explain why. - for _, id := range []string{"semantic_segmentation", "instance_segmentation", "causal_language_modeling", "token_classification"} { + // segmentation + the self-supervised text categories (CLM, seq2seq) + + // token_classification are known but not yet pushable, and must explain why. + for _, id := range []string{"semantic_segmentation", "instance_segmentation", "causal_language_modeling", "seq2seq", "token_classification"} { if !IsKnown(id) { t.Errorf("%s should be known", id) } diff --git a/internal/schema/ingest.v1.json b/internal/schema/ingest.v1.json index b5332b99..06c69185 100644 --- a/internal/schema/ingest.v1.json +++ b/internal/schema/ingest.v1.json @@ -37,7 +37,8 @@ "time_series_forecasting", "time_to_event_prediction", "masked_language_modeling", - "causal_language_modeling" + "causal_language_modeling", + "seq2seq" ], "description": "Task category. Drives convention defaults: validators, data_format, default columns, default file extensions, default validator set." }, @@ -87,7 +88,7 @@ "texts": { "type": "string", "minLength": 1, - "description": "Directory holding text files referenced by the labels CSV. Required for text_classification, token_classification, and causal_language_modeling (raw .txt: plain text, or a tab-separated prompt\\tcompletion pair)." + "description": "Directory holding text files referenced by the labels CSV. Required for text_classification, token_classification, causal_language_modeling (raw .txt: plain text, or a tab-separated prompt\\tcompletion pair), and seq2seq (raw .txt: a tab-separated source\\ttarget pair)." }, "sequences": { @@ -345,6 +346,14 @@ }, "then": { "required": ["texts"] } }, + { + "description": "seq2seq requires `texts` (raw .txt samples, each a tab-separated source\\ttarget pair). It is self-supervised, so unlike text/token classification it does NOT require `label`.", + "if": { + "properties": { "category": { "const": "seq2seq" } }, + "required": ["category"] + }, + "then": { "required": ["texts"] } + }, { "description": "tabular and time-series categories require `schema`.", "if": { @@ -421,7 +430,8 @@ "category": { "enum": [ "masked_language_modeling", - "causal_language_modeling" + "causal_language_modeling", + "seq2seq" ] } }, diff --git a/internal/ui/ui.go b/internal/ui/ui.go index e87122f1..7821fe8c 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -29,8 +29,9 @@ import ( // a command's call tree; it is not safe for concurrent writes to the // same underlying writer (neither is fmt.Fprintf). type Printer struct { - w io.Writer - color bool + w io.Writer + color bool + verbose bool } // Option customizes a Printer at construction. This is the functional- @@ -47,6 +48,13 @@ func WithColor(on bool) Option { return func(p *Printer) { p.color = on } } +// WithVerbose enables verbose output: Detailf lines print only when on. Wire it +// to a --verbose flag / $TRACEBLOC_LOG_LEVEL so the default happy path stays +// quiet (~6 status lines) while a streamed step-by-step view is opt-in. +func WithVerbose(on bool) Option { + return func(p *Printer) { p.verbose = on } +} + // New returns a Printer writing to w. By default it colorizes only when // w is a real terminal and the NO_COLOR env var is unset // (https://no-color.org). Options are applied after auto-detection, so @@ -143,6 +151,21 @@ func (p *Printer) Infof(format string, a ...any) { p.out(" %s %s\n", p.paint("·", color.Faint), fmt.Sprintf(format, a...)) } +// Detailf prints an indented, dim step-detail line — but ONLY in verbose mode +// (WithVerbose). Use for the streamed device-flow → provision → install trace +// (§8.5 R-verbose) that would be noise by default; the quiet path skips it. +func (p *Printer) Detailf(format string, a ...any) { + if !p.verbose { + return + } + p.out(" %s %s\n", p.paint("·", color.Faint), fmt.Sprintf(format, a...)) +} + +// Verbose reports whether this Printer is in verbose mode, so a caller can guard +// expensive detail (e.g. formatting a large value) it would otherwise build then +// discard. +func (p *Printer) Verbose() bool { return p.verbose } + // Errorf prints a bold-red ✖ error line. Unlike common.sh's error(), // it does NOT exit — surfacing the message is the UI's job; the command // still returns an *exitError so main() owns the process exit code. diff --git a/internal/ui/verbose_test.go b/internal/ui/verbose_test.go new file mode 100644 index 00000000..0c6e78a2 --- /dev/null +++ b/internal/ui/verbose_test.go @@ -0,0 +1,27 @@ +package ui + +import ( + "bytes" + "strings" + "testing" +) + +// TestDetailfVerboseGating: Detailf is silent by default and prints only under +// WithVerbose — the --verbose contract (RFC-0001 §8.5: the default stays quiet). +func TestDetailfVerboseGating(t *testing.T) { + var quiet bytes.Buffer + New(&quiet).Detailf("hidden %d", 1) + if quiet.Len() != 0 { + t.Errorf("Detailf must be silent without WithVerbose, got %q", quiet.String()) + } + + var loud bytes.Buffer + p := New(&loud, WithVerbose(true)) + if !p.Verbose() { + t.Error("Verbose() should report true under WithVerbose") + } + p.Detailf("shown %d", 2) + if !strings.Contains(loud.String(), "shown 2") { + t.Errorf("verbose Detailf should print, got %q", loud.String()) + } +} diff --git a/scripts/install.sh b/scripts/install.sh index 1af70f54..b76b8ba3 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -10,7 +10,10 @@ # 2. Resolves the latest release tag (or honors --version) # 3. Downloads tracebloc--- from the GitHub Release # 4. Verifies SHA256 against the release's SHA256SUMS file -# 5. (Optional) Verifies cosign signature if cosign is on PATH +# 5. Verifies the cosign signature — MANDATORY (RFC-0001 R8). If cosign isn't +# installed it bootstraps a pinned, checksum-verified one; if it can't, it +# FAILS CLOSED (never silently skips, never trusts the same-channel SHA256 +# alone). Override only with TRACEBLOC_ALLOW_UNVERIFIED=1. # 6. Installs to /usr/local/bin/tracebloc (falls back to $HOME/.local/bin # with PATH advice if /usr/local/bin isn't writable) # @@ -29,6 +32,19 @@ RELEASE_VERSION="${RELEASE_VERSION:-latest}" GITHUB_REPO="tracebloc/cli" BINARY_NAME="tracebloc" +# Cosign signature verification is MANDATORY on the default path (RFC-0001 R8, +# backend#889). The previous build silently SKIPPED it when cosign was absent — +# the default on a fresh box — degrading to a SHA256 fetched over the same +# channel as the binary, which an on-path attacker also controls. We now require +# a signature: if cosign isn't present we bootstrap a pinned, checksum-verified +# one; if we can't, we FAIL CLOSED. This explicit opt-out is the only way past, +# for the genuinely-constrained operator, and it shouts. +ALLOW_UNVERIFIED="${TRACEBLOC_ALLOW_UNVERIFIED:-0}" +# Pin kept in lockstep with the release workflow's cosign-installer and the +# client installer's COSIGN_VERSION. +COSIGN_VERSION="${COSIGN_VERSION:-v2.4.1}" +COSIGN_BIN="" + usage() { cat </dev/null 2>&1; then + sha256sum "$1" | awk '{print $1}' + elif command -v shasum >/dev/null 2>&1; then + shasum -a 256 "$1" | awk '{print $1}' + else + return 1 + fi +} + +# -------------------------------------------------------------------- +# Resolve a usable cosign into $COSIGN_BIN. Prefer one already on PATH; +# otherwise download the pinned release binary for this OS/arch and verify it +# against cosign's own published checksums before trusting it (a cosign we +# can't vouch for is no better than none). Returns non-zero if cosign can be +# neither found nor safely bootstrapped — the caller then fails closed. +# -------------------------------------------------------------------- +ensure_cosign() { + if command -v cosign >/dev/null 2>&1; then + COSIGN_BIN="cosign" + return 0 + fi + + # cosign publishes assets named cosign-- (arch in amd64/arm64); + # 386/arm have no official cosign build, so bootstrapping isn't possible there. + cosign_arch="" + case "$ARCH" in + amd64) cosign_arch="amd64" ;; + arm64) cosign_arch="arm64" ;; + *) return 1 ;; + esac + + # COSIGN_VERSION is env-overridable and gets interpolated into the Sigstore + # download URL, so it needs the same semver + path-traversal gate as the + # release tag — a crafted value must not redirect which release path we fetch. + validate_version_tag "$COSIGN_VERSION" "cosign version" \ + "Set COSIGN_VERSION to a published cosign release tag (e.g. v2.4.1)." + + cbase="https://github.com/sigstore/cosign/releases/download/${COSIGN_VERSION}" + casset="cosign-${OS}-${cosign_arch}" + cbin="$TMP/cosign" + csums="$TMP/cosign_checksums.txt" + + echo " cosign not found — bootstrapping pinned ${COSIGN_VERSION} to verify the signature..." + # --tlsv1.2 floor for the cosign bootstrap fetch, matching the client + # installer's curls — never negotiate below TLS 1.2 to pull the verifier we + # then trust to authenticate the release. + if ! curl -fsSL --tlsv1.2 "$cbase/$casset" -o "$cbin" 2>/dev/null; then return 1; fi + if ! curl -fsSL --tlsv1.2 "$cbase/cosign_checksums.txt" -o "$csums" 2>/dev/null; then return 1; fi + + cwant="$(grep " ${casset}\$" "$csums" | awk '{print $1}' | head -1)" + [ -n "$cwant" ] || return 1 + cgot="$(sha256_of "$cbin")" || return 1 + if [ "$cwant" != "$cgot" ]; then + echo "Error: bootstrapped cosign failed its own checksum — not using it." >&2 + return 1 + fi + chmod +x "$cbin" + COSIGN_BIN="$cbin" + return 0 +} + # -------------------------------------------------------------------- # Resolve the release tag if "latest". # -------------------------------------------------------------------- @@ -120,7 +202,7 @@ resolve_tag() { # Use the redirect-trail of /releases/latest to learn the tag — # avoids hitting the rate-limited /api/repos endpoint for the # zero-auth one-liner case. - redirect_url="$(curl -fsSI \ + redirect_url="$(curl -fsSI --tlsv1.2 \ "https://github.com/${GITHUB_REPO}/releases/latest" \ | awk '/^[Ll]ocation:/ { print $2 }' \ | tr -d '\r')" @@ -133,7 +215,44 @@ resolve_tag() { basename "$redirect_url" } +# -------------------------------------------------------------------- +# Validate the resolved tag before it flows into a download URL. +# +# --version / RELEASE_VERSION is returned by resolve_tag verbatim and then +# interpolated into BASE_URL=.../releases/download/${TAG}. An unvalidated value +# such as 'v1.2.3-../../heads/main' would let curl collapse the '..' and fetch +# from a path other than the intended release — a path-traversal lever in the +# most security-sensitive download in the installer. Constrain it to a release +# tag shape and refuse any '/' or '..' (RFC-0001 R8, backend#889). Matches the +# client bootstrap's gate (^v[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.]+)?$). +# validate_version_tag