diff --git a/cmd/volcano/main.go b/cmd/volcano/main.go index 0664f2e..6d1dcbf 100644 --- a/cmd/volcano/main.go +++ b/cmd/volcano/main.go @@ -3,15 +3,77 @@ package main import ( "fmt" + "io" + "net/http" "os" + "github.com/spf13/cobra" + + "github.com/Kong/volcano-cli/internal/api" rootcmd "github.com/Kong/volcano-cli/internal/cmd/root" + upgradecmd "github.com/Kong/volcano-cli/internal/cmd/upgrade" cliruntime "github.com/Kong/volcano-cli/internal/runtime" ) func main() { - if err := rootcmd.New(cliruntime.Deps{}).Execute(); err != nil { - fmt.Fprintln(os.Stderr, "Error:", err) - os.Exit(1) + deps := cliruntime.Deps{} + os.Exit(run(rootcmd.New(deps), deps)) +} + +// run executes root and returns the process exit code. Extracted from main +// so this orchestration — 426 short-circuiting, error-before-notices +// ordering, and exit codes — is covered by tests instead of only by the +// individually-tested helper functions it calls. Uses root.ErrOrStderr() +// (defaults to os.Stderr, same as before this extraction) so tests can +// redirect it via root.SetErr. +func run(root *cobra.Command, deps cliruntime.Deps) int { + err := root.Execute() + stderr := root.ErrOrStderr() + + if err != nil && api.Status(err) == http.StatusUpgradeRequired { + // The 426 body's message already reads "cli version no longer + // supported; run `volcano upgrade`"; just add the concrete upgrade + // target when the API provided one, and stop — printing the + // suggestion/deprecation notice below too would repeat ourselves. + printDeprecationError(stderr, err, deps) + return 1 + } + + if err != nil { + // Print the failure first: stderr's first line should be the actual + // error, for both humans skimming and log parsers/scripts that treat + // line 1 as the failure reason. Any pending notice is secondary + // context, printed after — VOL-180 instructions observed from an + // earlier API call in this invocation are not cleared by whatever + // unrelated error ended the command (see api's recordInstructions). + printError(stderr, err, deps) + upgradecmd.PrintAPIInstructionNotices(root, deps) + return 1 + } + + // Success: covers the non-blocking suggestion, and a deprecated CLI + // succeeding on an exempt route (e.g. `login`) where the user still needs + // to know their CLI is deprecated even though this command was let + // through. Reads only in-process state (api.LastInstructions); adds no + // network call. + upgradecmd.PrintAPIInstructionNotices(root, deps) + return 0 +} + +// printDeprecationError renders a require_version_upgrade 426 error together with +// the concrete upgrade target, when the API provided one. +func printDeprecationError(w io.Writer, err error, deps cliruntime.Deps) { + fmt.Fprintln(w, "Error:", err) + if latest := api.LastInstructions().LatestVersion; latest != "" { + fmt.Fprintf(w, "Upgrade to %s: %s\n", latest, cliruntime.CommandPath(deps, "upgrade")) + } +} + +// printError renders a generic command error, appending a reauth hint when +// the API signaled the platform token needs re-authentication (VOL-180). +func printError(w io.Writer, err error, deps cliruntime.Deps) { + fmt.Fprintln(w, "Error:", err) + if api.LastInstructions().DeviceInstruction == api.DeviceInstructionReauth { + fmt.Fprintf(w, "Run `%s` to re-authenticate.\n", cliruntime.CommandPath(deps, "login")) } } diff --git a/cmd/volcano/main_test.go b/cmd/volcano/main_test.go new file mode 100644 index 0000000..a0f433e --- /dev/null +++ b/cmd/volcano/main_test.go @@ -0,0 +1,293 @@ +package main + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Kong/volcano-cli/internal/api" + rootcmd "github.com/Kong/volcano-cli/internal/cmd/root" + cliconfig "github.com/Kong/volcano-cli/internal/config" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +// withInstructions drives api.LastInstructions() through a real api.Client +// call against a test server, mirroring how production populates it from +// response headers (VOL-180). +func withInstructions(t *testing.T, latest, deviceInstruction string) { + t.Helper() + // recordInstructions is sticky (VOL-180): a field a response omits doesn't + // clear a value recorded by an earlier test. Reset explicitly so each test + // starts from the zero value regardless of execution order. + api.ResetLastInstructionsForTest() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if latest != "" { + // The server never sends X-Volcano-CLI-Latest-Version without a + // paired instruction (setLatestVersionHeader is only called from the + // suggest/deprecate branches) — recordInstructions relies on that + // contract, so this helper must too. + w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionRequireVersionUpgrade) + w.Header().Set("X-Volcano-CLI-Latest-Version", latest) + } + if deviceInstruction != "" { + w.Header().Set("X-Volcano-Device-Instruction", deviceInstruction) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":[],"has_more":false,"page":1,"limit":100,"total":0}`)) + })) + t.Cleanup(server.Close) + + client, err := api.NewClient(server.URL, "", api.WithHTTPClient(server.Client())) + require.NoError(t, err) + _, err = client.ListProjects(context.Background(), api.DefaultPage, api.DefaultLimit) + require.NoError(t, err) +} + +func TestPrintDeprecationError_WithLatestVersion(t *testing.T) { + withInstructions(t, "v1.5.0", "") + var out bytes.Buffer + + printDeprecationError(&out, &api.Error{StatusCode: http.StatusUpgradeRequired, Message: "cli version no longer supported; run `volcano upgrade`"}, cliruntime.Deps{}) + + assert.Contains(t, out.String(), "Error: HTTP 426: cli version no longer supported; run `volcano upgrade`") + assert.Contains(t, out.String(), "Upgrade to v1.5.0: volcano upgrade") +} + +func TestPrintDeprecationError_WithoutLatestVersion(t *testing.T) { + withInstructions(t, "", "") + var out bytes.Buffer + + printDeprecationError(&out, &api.Error{StatusCode: http.StatusUpgradeRequired, Message: "cli version no longer supported"}, cliruntime.Deps{}) + + assert.Contains(t, out.String(), "Error: HTTP 426: cli version no longer supported") + assert.NotContains(t, out.String(), "Upgrade to") +} + +func TestPrintDeprecationError_UsesCommandPathPrefix(t *testing.T) { + withInstructions(t, "v1.5.0", "") + var out bytes.Buffer + + printDeprecationError(&out, &api.Error{StatusCode: http.StatusUpgradeRequired}, cliruntime.Deps{CommandPathPrefix: "acme"}) + + assert.Contains(t, out.String(), "Upgrade to v1.5.0: acme upgrade") +} + +func TestPrintError_ReauthHint(t *testing.T) { + withInstructions(t, "", "reauth") + var out bytes.Buffer + + printError(&out, &api.Error{StatusCode: http.StatusUnauthorized, Message: "token expired"}, cliruntime.Deps{}) + + assert.Contains(t, out.String(), "Error: HTTP 401: token expired") + assert.Contains(t, out.String(), "Run `volcano login` to re-authenticate.") +} + +func TestPrintError_NoReauthHintWithoutSignal(t *testing.T) { + withInstructions(t, "", "") + var out bytes.Buffer + + printError(&out, &api.Error{StatusCode: http.StatusInternalServerError, Message: "boom"}, cliruntime.Deps{}) + + assert.Contains(t, out.String(), "Error: HTTP 500: boom") + assert.NotContains(t, out.String(), "re-authenticate") +} + +func TestPrintError_ReauthHintUsesCommandPathPrefix(t *testing.T) { + withInstructions(t, "", "reauth") + var out bytes.Buffer + + printError(&out, &api.Error{StatusCode: http.StatusUnauthorized}, cliruntime.Deps{CommandPathPrefix: "acme"}) + + assert.Contains(t, out.String(), "Run `acme login` to re-authenticate.") +} + +// runDeps builds cliruntime.Deps + a rootcmd.New(deps) wired to server via an +// in-memory config (no disk/env), so tests can drive run() through a real +// command instead of poking internal state. +func runDeps(server *httptest.Server) cliruntime.Deps { + return cliruntime.Deps{ + HTTPClient: server.Client(), + ConfigLoader: func() (*cliconfig.Config, error) { + return &cliconfig.Config{ + APIBaseURL: server.URL, + UserToken: "test-token", + IgnoreEnv: true, + }, nil + }, + } +} + +func TestRun_SuccessNoNotice(t *testing.T) { + api.ResetLastInstructionsForTest() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":[],"has_more":false,"page":1,"limit":100,"total":0}`)) + })) + defer server.Close() + + deps := runDeps(server) + root := rootcmd.New(deps) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs([]string{"projects", "list"}) + + code := run(root, deps) + + assert.Equal(t, 0, code) + assert.Empty(t, stderr.String(), "no notice pending, stderr must stay empty") +} + +func TestRun_SuccessWithSuggestionNotice(t *testing.T) { + api.ResetLastInstructionsForTest() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionSuggestionVersionUpgrade) + w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":[],"has_more":false,"page":1,"limit":100,"total":0}`)) + })) + defer server.Close() + + deps := runDeps(server) + root := rootcmd.New(deps) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs([]string{"projects", "list"}) + + code := run(root, deps) + + assert.Equal(t, 0, code) + assert.Contains(t, stderr.String(), "A newer Volcano CLI version is available: v1.5.0") +} + +func TestRun_SuccessWithDeprecationNoticeOnExemptRoute(t *testing.T) { + // The exact scenario the notice-printing path exists for (per + // printDeprecationWarning's doc comment): a deprecated CLI's request lands + // on an exempt route (e.g. `login`) and succeeds — the server sets the + // require_version_upgrade header but doesn't 426 it. The user still needs + // to learn their CLI is deprecated even though this command was let + // through. Only the deprecation-error (426) path had run()-level coverage + // before this; this is the success-path counterpart. + api.ResetLastInstructionsForTest() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionRequireVersionUpgrade) + w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":[],"has_more":false,"page":1,"limit":100,"total":0}`)) + })) + defer server.Close() + + deps := runDeps(server) + root := rootcmd.New(deps) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs([]string{"projects", "list"}) + + code := run(root, deps) + + // The command succeeded (no error to short-circuit on), so the exit code + // must be 0 even though the CLI is deprecated — only the 426 path (a + // non-exempt route) exits 1. + assert.Equal(t, 0, code) + assert.Contains(t, stderr.String(), "Volcano CLI") + assert.Contains(t, stderr.String(), "is no longer supported. Upgrade to v1.5.0 or later:") +} + +func TestRun_DeprecationErrorShortCircuitsWithoutDuplicateNotice(t *testing.T) { + api.ResetLastInstructionsForTest() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionRequireVersionUpgrade) + w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUpgradeRequired) + _, _ = w.Write([]byte(`{"error":"cli version no longer supported; run ` + "`volcano upgrade`" + `"}`)) + })) + defer server.Close() + + deps := runDeps(server) + root := rootcmd.New(deps) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs([]string{"projects", "list"}) + + code := run(root, deps) + + assert.Equal(t, 1, code) + text := stderr.String() + assert.Contains(t, text, "Error:") + assert.Contains(t, text, "Upgrade to v1.5.0: volcano upgrade") + // The 426 path must not ALSO print the generic non-blocking suggestion + // notice (a distinct phrase from the deprecation error's own message) — + // that would tell the user the same thing twice in different words. + assert.NotContains(t, text, "A newer Volcano CLI version is available", "deprecation error path must not duplicate the suggestion notice: %q", text) +} + +func TestRun_NonBlockingErrorPrintsErrorBeforeNotice(t *testing.T) { + // A command that fails for an unrelated reason (404 here) while also + // carrying a pending suggestion notice must print the actual error first. + api.ResetLastInstructionsForTest() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Volcano-CLI-Instruction", api.CLIInstructionSuggestionVersionUpgrade) + w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"project not found"}`)) + })) + defer server.Close() + + deps := runDeps(server) + root := rootcmd.New(deps) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs([]string{"projects", "get", "11111111-1111-1111-1111-111111111111"}) + + code := run(root, deps) + + assert.Equal(t, 1, code) + text := stderr.String() + require.Contains(t, text, "Error:") + require.Contains(t, text, "newer Volcano CLI version is available") + assert.Less(t, strings.Index(text, "Error:"), strings.Index(text, "newer Volcano CLI version is available"), + "the error line must come before the notice: %q", text) +} + +func TestRun_NonBlockingErrorWithReauthHint(t *testing.T) { + api.ResetLastInstructionsForTest() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Volcano-Device-Instruction", api.DeviceInstructionReauth) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"token expired"}`)) + })) + defer server.Close() + + deps := runDeps(server) + root := rootcmd.New(deps) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + root.SetArgs([]string{"projects", "list"}) + + code := run(root, deps) + + assert.Equal(t, 1, code) + text := stderr.String() + require.Contains(t, text, "Error:") + require.Contains(t, text, "Run `volcano login` to re-authenticate.") + assert.Less(t, strings.Index(text, "Error:"), strings.Index(text, "Run `volcano login`"), + "the error line must come before the reauth hint: %q", text) +} diff --git a/internal/api/client.go b/internal/api/client.go index 07f25f8..25f1056 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -70,6 +70,13 @@ func NewClient(apiURL, token string, opts ...Option) (*Client, error) { cfg.streamHTTPClient = &http.Client{} } + // Wrap both doers with the VOL-180 version protocol: every request + // (including unauthenticated ones, e.g. device/token exchange) reports this + // CLI's version/identity, and every response's instruction headers are + // recorded for LastInstructions regardless of which call observed them. + cfg.httpClient = versionProtocolDoer{next: cfg.httpClient} + cfg.streamHTTPClient = versionProtocolDoer{next: cfg.streamHTTPClient} + baseURL := generatedClientBaseURL(parsed) clientOpts := []apiclient.ClientOption{ apiclient.WithHTTPClient(cfg.httpClient), diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 3cd3a9a..95b5cbd 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/Kong/volcano-cli/internal/apiclient" + "github.com/Kong/volcano-cli/internal/version" ) func TestPollDeviceTokenUnexpectedStatusReturnsError(t *testing.T) { @@ -108,6 +109,36 @@ func TestWebSignupURLRejectsBadInput(t *testing.T) { } } +func TestNewClientSendsVersionProtocolHeadersAndRecordsInstructions(t *testing.T) { + resetInstructions(t) + oldVersion := version.Version + version.Version = "v1.2.3" + t.Cleanup(func() { version.Version = oldVersion }) + + var sawVersionHeader, sawUA string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawVersionHeader = r.Header.Get("X-Volcano-CLI-Version") + sawUA = r.Header.Get("User-Agent") + w.Header().Set("X-Volcano-CLI-Instruction", CLIInstructionSuggestionVersionUpgrade) + w.Header().Set("X-Volcano-CLI-Latest-Version", "v1.5.0") + writeAPIJSON(t, w, http.StatusOK, map[string]any{"data": []any{}, "has_more": false, "page": 1, "limit": 100, "total": 0}) + })) + defer server.Close() + + client, err := NewClient(server.URL, "token", WithHTTPClient(server.Client())) + require.NoError(t, err) + + _, err = client.ListProjects(context.Background(), DefaultPage, DefaultLimit) + require.NoError(t, err) + + assert.Equal(t, "v1.2.3", sawVersionHeader) + assert.Contains(t, sawUA, "volcano-cli/v1.2.3") + + got := LastInstructions() + assert.Equal(t, CLIInstructionSuggestionVersionUpgrade, got.CLIInstruction) + assert.Equal(t, "v1.5.0", got.LatestVersion) +} + func TestNewClientPreservesAPIURLPathPrefix(t *testing.T) { var sawPath string var sawQuery string diff --git a/internal/api/version_protocol.go b/internal/api/version_protocol.go new file mode 100644 index 0000000..f9c0dfd --- /dev/null +++ b/internal/api/version_protocol.go @@ -0,0 +1,140 @@ +package api + +import ( + "net/http" + "runtime" + "strings" + "sync" + + "github.com/Kong/volcano-cli/internal/apiclient" + "github.com/Kong/volcano-cli/internal/version" +) + +// Header names and instruction values for the VOL-180 CLI version protocol. +// These MUST match volcano-hosting's internal/constants/http.go and +// docs/cli/version-gating.md. Duplicated here because this is a separate +// repository with no shared Go module; there is intentionally no +// cryptographic binding on the request header — see "Security model" in that +// doc for why the claimed version is advisory input only and never a +// privilege (the CLI cannot make this header trustworthy, and doesn't need +// to: the API never grants anything based on it). +const ( + headerCLIVersion = "X-Volcano-CLI-Version" + headerCLIInstruction = "X-Volcano-CLI-Instruction" + headerCLILatestVersion = "X-Volcano-CLI-Latest-Version" + headerDeviceInstruction = "X-Volcano-Device-Instruction" + + // CLIInstructionSuggestionVersionUpgrade signals a newer CLI is published; non-blocking. + CLIInstructionSuggestionVersionUpgrade = "suggestion_version_upgrade" + // CLIInstructionRequireVersionUpgrade signals this CLI version is no longer + // supported. The API pairs it with an HTTP 426 on non-exempt routes. + CLIInstructionRequireVersionUpgrade = "require_version_upgrade" + + // CLIInstructionLowCreditWarning and CLIInstructionNotEnoughCredit are + // RESERVED for a future billing/credit gate. PrintAPIInstructionNotices + // already has cases for both, but they're unreached today: the API does + // not emit them yet — billing integration into this protocol needs its own + // design pass. Naming is locked in now, parallel to the version + // instructions, so a future server-side implementation doesn't need a + // wire-format rename. + CLIInstructionLowCreditWarning = "low_credit_warning" + CLIInstructionNotEnoughCredit = "not_enough_credit" + + // DeviceInstructionReauth signals the platform token needs re-authentication. + DeviceInstructionReauth = "reauth" +) + +// Instructions is a snapshot of the most recent VOL-180 protocol signals +// observed on any API response in this process. +type Instructions struct { + CLIInstruction string + LatestVersion string + DeviceInstruction string +} + +var ( + instructionsMu sync.RWMutex + lastInstructions Instructions +) + +// LastInstructions returns the most recently observed VOL-180 instructions +// from any API response in this process. It is the zero value if no API call +// has completed yet — e.g. local-only commands (init, help, version, +// completion) never populate this, since they never make a request. +// +// A CLI process runs exactly one command per invocation, so "most recent" is +// simply "from the request(s) this command made" — there is no cross-command +// state to reconcile. +func LastInstructions() Instructions { + instructionsMu.RLock() + defer instructionsMu.RUnlock() + return lastInstructions +} + +// recordInstructions updates the two instruction fields independently, and +// only when a response actually carries a value for that field — a response +// with no X-Volcano-CLI-Instruction header does not clear a CLIInstruction +// (and its paired LatestVersion) recorded from an earlier response in the +// same command invocation, and likewise for DeviceInstruction. A command +// that makes several API calls must not have an earlier real notice silently +// dropped just because a later, unrelated response didn't repeat the header. +// +// CLIInstruction and LatestVersion update together (never independently): +// they come from the same server-side gate decision, so a response that sets +// one is authoritative for both, even if that response's LatestVersion is +// itself empty (e.g. no latest configured). +func recordInstructions(header http.Header) { + cliInstruction := strings.TrimSpace(header.Get(headerCLIInstruction)) + latestVersion := strings.TrimSpace(header.Get(headerCLILatestVersion)) + deviceInstruction := strings.TrimSpace(header.Get(headerDeviceInstruction)) + + instructionsMu.Lock() + defer instructionsMu.Unlock() + if cliInstruction != "" { + lastInstructions.CLIInstruction = cliInstruction + lastInstructions.LatestVersion = latestVersion + } + if deviceInstruction != "" { + lastInstructions.DeviceInstruction = deviceInstruction + } +} + +// userAgent identifies this CLI build to the API — for support/debugging and +// as the VOL-180 pre-protocol/attribution signal — replacing the Go HTTP +// client's default user agent. +func userAgent() string { + return "volcano-cli/" + version.Version + " (" + runtime.GOOS + "/" + runtime.GOARCH + ")" +} + +// versionProtocolDoer wraps an apiclient.HttpRequestDoer to speak the VOL-180 +// CLI version protocol on every request: it reports this CLI's version and +// identity, and records whatever instructions the API returns so callers can +// act on them via LastInstructions. It never alters the request/response +// beyond adding headers — bodies and errors pass through untouched, and it +// never retries. +type versionProtocolDoer struct { + next apiclient.HttpRequestDoer +} + +func (d versionProtocolDoer) Do(req *http.Request) (*http.Response, error) { + req.Header.Set(headerCLIVersion, version.Version) + req.Header.Set("User-Agent", userAgent()) + resp, err := d.next.Do(req) + if resp != nil { + recordInstructions(resp.Header) + } + return resp, err +} + +// ResetLastInstructionsForTest clears the process-global instructions state. +// Test-only: production never needs this, since each CLI invocation is a +// fresh process. Call it between test cases that expect LastInstructions() to +// start from the zero value — recordInstructions is intentionally sticky (see +// its doc comment): a response that omits a field never clears a previously +// recorded value for it, so tests must reset explicitly rather than relying +// on an unrelated response to clear prior state. +func ResetLastInstructionsForTest() { + instructionsMu.Lock() + lastInstructions = Instructions{} + instructionsMu.Unlock() +} diff --git a/internal/api/version_protocol_test.go b/internal/api/version_protocol_test.go new file mode 100644 index 0000000..5c2d50f --- /dev/null +++ b/internal/api/version_protocol_test.go @@ -0,0 +1,189 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Kong/volcano-cli/internal/version" +) + +// resetInstructions clears process-global observed instructions between test +// cases (production never needs this: each CLI invocation is a fresh process). +func resetInstructions(t *testing.T) { + t.Helper() + ResetLastInstructionsForTest() + t.Cleanup(ResetLastInstructionsForTest) +} + +func TestVersionProtocolDoer_SetsVersionHeaderAndUserAgent(t *testing.T) { + resetInstructions(t) + oldVersion := version.Version + version.Version = "v1.4.2" + t.Cleanup(func() { version.Version = oldVersion }) + + var gotVersionHeader, gotUA string + inner := doerFunc(func(req *http.Request) (*http.Response, error) { + gotVersionHeader = req.Header.Get(headerCLIVersion) + gotUA = req.Header.Get("User-Agent") + return httptest.NewRecorder().Result(), nil + }) + + req := httptest.NewRequest(http.MethodGet, "/projects", http.NoBody) + _, err := versionProtocolDoer{next: inner}.Do(req) + require.NoError(t, err) + + assert.Equal(t, "v1.4.2", gotVersionHeader) + assert.Equal(t, "volcano-cli/v1.4.2 ("+runtime.GOOS+"/"+runtime.GOARCH+")", gotUA) +} + +func TestVersionProtocolDoer_RecordsInstructionHeaders(t *testing.T) { + resetInstructions(t) + + inner := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set(headerCLIInstruction, CLIInstructionSuggestionVersionUpgrade) + rec.Header().Set(headerCLILatestVersion, "v1.5.0") + rec.WriteHeader(http.StatusOK) + return rec.Result(), nil + }) + + req := httptest.NewRequest(http.MethodGet, "/projects", http.NoBody) + _, err := versionProtocolDoer{next: inner}.Do(req) + require.NoError(t, err) + + got := LastInstructions() + assert.Equal(t, CLIInstructionSuggestionVersionUpgrade, got.CLIInstruction) + assert.Equal(t, "v1.5.0", got.LatestVersion) + assert.Empty(t, got.DeviceInstruction) +} + +func TestVersionProtocolDoer_RecordsReauthInstruction(t *testing.T) { + resetInstructions(t) + + inner := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set(headerDeviceInstruction, DeviceInstructionReauth) + rec.WriteHeader(http.StatusUnauthorized) + return rec.Result(), nil + }) + + req := httptest.NewRequest(http.MethodGet, "/projects", http.NoBody) + _, err := versionProtocolDoer{next: inner}.Do(req) + require.NoError(t, err) + + assert.Equal(t, DeviceInstructionReauth, LastInstructions().DeviceInstruction) +} + +func TestVersionProtocolDoer_NoResponseIsNoOp(t *testing.T) { + resetInstructions(t) + + boom := assert.AnError + inner := doerFunc(func(*http.Request) (*http.Response, error) { + return nil, boom + }) + + req := httptest.NewRequest(http.MethodGet, "/projects", http.NoBody) + _, err := versionProtocolDoer{next: inner}.Do(req) + require.ErrorIs(t, err, boom) + assert.Equal(t, Instructions{}, LastInstructions()) +} + +func TestVersionProtocolDoer_LaterEmptyResponseDoesNotClearEarlierInstruction(t *testing.T) { + // A command can make several API calls; a later response that simply + // doesn't repeat the instruction header (e.g. an unrelated call) must not + // silently drop a real notice observed earlier in the same invocation. + resetInstructions(t) + + first := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set(headerCLIInstruction, CLIInstructionSuggestionVersionUpgrade) + rec.Header().Set(headerCLILatestVersion, "v1.5.0") + rec.WriteHeader(http.StatusOK) + return rec.Result(), nil + }) + second := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.WriteHeader(http.StatusOK) + return rec.Result(), nil + }) + + req := httptest.NewRequest(http.MethodGet, "/projects", http.NoBody) + _, err := versionProtocolDoer{next: first}.Do(req) + require.NoError(t, err) + require.Equal(t, CLIInstructionSuggestionVersionUpgrade, LastInstructions().CLIInstruction) + require.Equal(t, "v1.5.0", LastInstructions().LatestVersion) + + _, err = versionProtocolDoer{next: second}.Do(req) + require.NoError(t, err) + assert.Equal(t, CLIInstructionSuggestionVersionUpgrade, LastInstructions().CLIInstruction, "a later response with no instruction header must not clear an earlier real one") + assert.Equal(t, "v1.5.0", LastInstructions().LatestVersion, "LatestVersion stays paired with the preserved CLIInstruction") +} + +func TestVersionProtocolDoer_LaterNonEmptyResponseUpdatesInstruction(t *testing.T) { + // A genuinely different instruction (e.g. the policy changed, or a later + // call reports a different version comparison) must still take effect — + // preservation only protects against being cleared by an empty response. + resetInstructions(t) + + first := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set(headerCLIInstruction, CLIInstructionSuggestionVersionUpgrade) + rec.Header().Set(headerCLILatestVersion, "v1.5.0") + rec.WriteHeader(http.StatusOK) + return rec.Result(), nil + }) + second := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set(headerCLIInstruction, CLIInstructionRequireVersionUpgrade) + rec.Header().Set(headerCLILatestVersion, "v1.6.0") + rec.WriteHeader(http.StatusUpgradeRequired) + return rec.Result(), nil + }) + + req := httptest.NewRequest(http.MethodGet, "/projects", http.NoBody) + _, err := versionProtocolDoer{next: first}.Do(req) + require.NoError(t, err) + _, err = versionProtocolDoer{next: second}.Do(req) + require.NoError(t, err) + + assert.Equal(t, CLIInstructionRequireVersionUpgrade, LastInstructions().CLIInstruction) + assert.Equal(t, "v1.6.0", LastInstructions().LatestVersion) +} + +func TestVersionProtocolDoer_CLIInstructionAndDeviceInstructionTrackIndependently(t *testing.T) { + // A response that sets DeviceInstruction but not CLIInstruction (or vice + // versa) must not clear whichever one it didn't mention. + resetInstructions(t) + + first := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set(headerCLIInstruction, CLIInstructionSuggestionVersionUpgrade) + rec.WriteHeader(http.StatusOK) + return rec.Result(), nil + }) + second := doerFunc(func(*http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set(headerDeviceInstruction, DeviceInstructionReauth) + rec.WriteHeader(http.StatusUnauthorized) + return rec.Result(), nil + }) + + req := httptest.NewRequest(http.MethodGet, "/projects", http.NoBody) + _, err := versionProtocolDoer{next: first}.Do(req) + require.NoError(t, err) + _, err = versionProtocolDoer{next: second}.Do(req) + require.NoError(t, err) + + got := LastInstructions() + assert.Equal(t, CLIInstructionSuggestionVersionUpgrade, got.CLIInstruction, "unrelated device-instruction response must not clear the earlier CLI instruction") + assert.Equal(t, DeviceInstructionReauth, got.DeviceInstruction) +} + +type doerFunc func(*http.Request) (*http.Response, error) + +func (f doerFunc) Do(req *http.Request) (*http.Response, error) { return f(req) } diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index 936e119..e6c09cd 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -27,9 +27,6 @@ func New(deps cliruntime.Deps) *cobra.Command { Long: "volcano is the command-line client for the Volcano hosting platform.", SilenceUsage: true, SilenceErrors: true, - PersistentPreRun: func(cmd *cobra.Command, _ []string) { - upgradecmd.MaybePrintUpdateNotice(cmd, deps) - }, RunE: func(cmd *cobra.Command, _ []string) error { if showVersion { printVersion(cmd.OutOrStdout()) diff --git a/internal/cmd/upgrade/upgrade.go b/internal/cmd/upgrade/upgrade.go index c90ecf6..2275bdf 100644 --- a/internal/cmd/upgrade/upgrade.go +++ b/internal/cmd/upgrade/upgrade.go @@ -2,31 +2,16 @@ package upgrade import ( - "context" - "encoding/json" "fmt" - "os" - "path/filepath" - "strings" - "time" "github.com/spf13/cobra" + "github.com/Kong/volcano-cli/internal/api" cliruntime "github.com/Kong/volcano-cli/internal/runtime" "github.com/Kong/volcano-cli/internal/update" "github.com/Kong/volcano-cli/internal/version" ) -const ( - updateCheckTimeout = 2 * time.Second - updateCheckMaxAge = 24 * time.Hour -) - -type noticeCache struct { - CheckedAt time.Time `json:"checked_at"` - Latest string `json:"latest"` -} - // New returns the upgrade command. func New(deps cliruntime.Deps) *cobra.Command { var verifySignature bool @@ -53,156 +38,71 @@ func updateOptions(deps cliruntime.Deps) update.Options { } } -// MaybePrintUpdateNotice checks whether a newer CLI release is available and prints a non-blocking notice. -func MaybePrintUpdateNotice(cmd *cobra.Command, deps cliruntime.Deps) { - if shouldSkipUpdateCheck(cmd) { - return - } - if notice, ok := noticeFromCache(version.Version); ok { - if notice != nil { - printUpdateNotice(cmd, notice) - } - return - } - parent := cmd.Context() - if parent == nil { - parent = context.Background() - } - ctx, cancel := context.WithTimeout(parent, updateCheckTimeout) - defer cancel() - release, err := update.LatestRelease(ctx, updateOptions(deps)) - if err != nil { - return - } - latest := strings.TrimSpace(release.TagName) - if latest == "" { - return - } - writeNoticeCache(latest) - newer, err := update.NewerThan(latest, version.Version) - if err != nil || !newer { - return - } - notice := &update.Notice{Current: version.Version, Latest: latest} - printUpdateNotice(cmd, notice) -} - -func shouldSkipUpdateCheck(cmd *cobra.Command) bool { - if version.Version == "dev" { - return true - } - if cmd.Flags().Changed("help") || cmd.Flags().Changed("version") { - return true - } - // Skip when invoking the root command with no subcommand — cobra renders help, and - // we don't want a network check to delay or pollute that output. - if cmd.Parent() == nil && len(cmd.Flags().Args()) == 0 { - return true - } - for c := cmd; c != nil; c = c.Parent() { - if skipUpdateCheckCommand(c.Name()) { - return true - } - } - return false -} - -func skipUpdateCheckCommand(name string) bool { - switch name { - case "version", "upgrade", "help", "completion", "__complete", "__completeNoDesc": - return true - default: - return false +// PrintAPIInstructionNotices prints a non-blocking upgrade suggestion or a +// deprecation warning based on the most recent VOL-180 instructions observed +// on this invocation's API responses (api.LastInstructions). This replaces +// the previous GitHub-release polling notice (VOL-168): the API is now the +// sole source of truth for whether/how loudly to nudge an upgrade, and the +// check adds no extra network round-trip — it only reads headers the command +// already received. +// +// A command that made no API call (help, version, completion, local-only +// commands) observes a zero-value Instructions and prints nothing. +func PrintAPIInstructionNotices(cmd *cobra.Command, deps cliruntime.Deps) { + instructions := api.LastInstructions() + switch instructions.CLIInstruction { + case api.CLIInstructionRequireVersionUpgrade: + printDeprecationWarning(cmd, deps, instructions.LatestVersion) + case api.CLIInstructionSuggestionVersionUpgrade: + printUpgradeSuggestion(cmd, deps, instructions.LatestVersion) + case api.CLIInstructionNotEnoughCredit: + printNotEnoughCreditWarning(cmd) + case api.CLIInstructionLowCreditWarning: + printLowCreditWarning(cmd) } } -func printUpdateNotice(cmd *cobra.Command, notice *update.Notice) { - fmt.Fprintf(cmd.ErrOrStderr(), "A newer Volcano CLI version is available: %s (current %s). Run `volcano upgrade` to upgrade.\n", notice.Latest, notice.Current) -} - -func noticeFromCache(current string) (*update.Notice, bool) { - cache, err := readNoticeCache() - if err != nil || cache.Latest == "" { - return nil, false - } - // Use absolute age so future-dated CheckedAt values (clock skew, restored backups) - // are treated as stale rather than fresh-forever. - age := time.Since(cache.CheckedAt) - if age < 0 { - age = -age - } - if age > updateCheckMaxAge { - return nil, false - } - newer, err := update.NewerThan(cache.Latest, current) - if err != nil { - return nil, false - } - if !newer { - return nil, true +func printUpgradeSuggestion(cmd *cobra.Command, deps cliruntime.Deps, latest string) { + upgradeCmd := cliruntime.CommandPath(deps, "upgrade") + if latest != "" { + fmt.Fprintf(cmd.ErrOrStderr(), "A newer Volcano CLI version is available: %s (current %s). Run `%s` to upgrade.\n", latest, version.Version, upgradeCmd) + return } - return &update.Notice{Current: current, Latest: cache.Latest}, true + fmt.Fprintf(cmd.ErrOrStderr(), "A newer Volcano CLI version is available (current %s). Run `%s` to upgrade.\n", version.Version, upgradeCmd) } -func readNoticeCache() (*noticeCache, error) { - path, err := noticeCachePath() - if err != nil { - return nil, err - } - f, err := os.Open(path) - if err != nil { - return nil, err - } - defer func() { _ = f.Close() }() - var cache noticeCache - if err := json.NewDecoder(f).Decode(&cache); err != nil { - return nil, err +// printDeprecationWarning prints a hard warning for a version the API has +// deprecated. Gated routes already 426 this request (see api.Status(err) == +// http.StatusUpgradeRequired handling in cmd/volcano/main.go); this notice +// also fires on exempt routes (e.g. a deprecated CLI's successful `volcano +// login`), so the user learns their CLI is deprecated even when the specific +// command they ran was allowed through. +func printDeprecationWarning(cmd *cobra.Command, deps cliruntime.Deps, latest string) { + upgradeCmd := cliruntime.CommandPath(deps, "upgrade") + if latest != "" { + fmt.Fprintf(cmd.ErrOrStderr(), "Volcano CLI %s is no longer supported. Upgrade to %s or later:\n %s\n", version.Version, latest, upgradeCmd) + return } - return &cache, nil + fmt.Fprintf(cmd.ErrOrStderr(), "Volcano CLI %s is no longer supported. Run `%s` to upgrade.\n", version.Version, upgradeCmd) } -func writeNoticeCache(latest string) { - path, err := noticeCachePath() - if err != nil { - return - } - dir := filepath.Dir(path) - if err := os.MkdirAll(dir, 0o700); err != nil { - return - } - // Write to a sibling temp file and rename, so concurrent CLI invocations can't - // observe a half-written JSON file. - f, err := os.CreateTemp(dir, "update-check-*.json") - if err != nil { - return - } - tmpPath := f.Name() - cleanup := true - defer func() { - if cleanup { - _ = os.Remove(tmpPath) - } - }() - if err := json.NewEncoder(f).Encode(noticeCache{CheckedAt: time.Now().UTC(), Latest: latest}); err != nil { - _ = f.Close() - return - } - if err := f.Close(); err != nil { - return - } - if err := os.Chmod(tmpPath, 0o600); err != nil { - return - } - if err := os.Rename(tmpPath, path); err != nil { - return - } - cleanup = false +// printNotEnoughCreditWarning and printLowCreditWarning handle the reserved +// credit instructions (VOL-180 PR review discussion, +// api.CLIInstructionNotEnoughCredit / api.CLIInstructionLowCreditWarning). +// The API never emits +// these yet — billing integration needs its own design pass — so these paths +// are unreached today; they exist so that once the server starts sending the +// header, this is already wired and nothing else needs to change here. +// +// The printed notice is a placeholder, not the designed UX: the actual ask is +// an interactive prompt ("upgrade?" / "purchase extra credits?") that takes +// an action, which needs a real target (a URL? a command?) that doesn't +// exist yet. internal/confirm already has the prompt primitive for that once +// there's something concrete to confirm. +func printNotEnoughCreditWarning(cmd *cobra.Command) { + fmt.Fprintln(cmd.ErrOrStderr(), "Your project does not have enough credit to complete this request.") } -func noticeCachePath() (string, error) { - dir, err := os.UserCacheDir() - if err != nil { - return "", err - } - return filepath.Join(dir, "volcano", "update-check.json"), nil +func printLowCreditWarning(cmd *cobra.Command) { + fmt.Fprintln(cmd.ErrOrStderr(), "Your project is running low on credit.") } diff --git a/internal/cmd/upgrade/upgrade_test.go b/internal/cmd/upgrade/upgrade_test.go index 40b7791..2621acc 100644 --- a/internal/cmd/upgrade/upgrade_test.go +++ b/internal/cmd/upgrade/upgrade_test.go @@ -12,12 +12,12 @@ import ( "os" "path/filepath" "testing" - "time" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/Kong/volcano-cli/internal/api" cliruntime "github.com/Kong/volcano-cli/internal/runtime" "github.com/Kong/volcano-cli/internal/update" "github.com/Kong/volcano-cli/internal/version" @@ -100,231 +100,114 @@ func TestUpgradeCommandRejectsArgs(t *testing.T) { assert.ErrorContains(t, err, `unknown command "v1.2.3"`) } -func TestUpdateNoticePrintsForOutdatedVersion(t *testing.T) { - setUpgradeTestCacheDir(t) - oldVersion := version.Version - version.Version = "v1.2.3" - t.Cleanup(func() { version.Version = oldVersion }) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, "/releases/latest", r.URL.Path) - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.4"}) - })) - defer server.Close() - - cmd := noticeTestCommand("init") - var out bytes.Buffer - cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Contains(t, out.String(), "A newer Volcano CLI version is available: v1.2.4 (current v1.2.3). Run `volcano upgrade` to upgrade.") -} - -func TestUpdateNoticeSkipsVersionCommand(t *testing.T) { - setUpgradeTestCacheDir(t) - oldVersion := version.Version - version.Version = "v1.2.3" - t.Cleanup(func() { version.Version = oldVersion }) - - requests := 0 +// withInstructions drives api.LastInstructions() through a real api.Client +// call against a test server, exercising PrintAPIInstructionNotices exactly +// as production does (via observed response headers), with no reliance on +// api's unexported state. +func withInstructions(t *testing.T, cliInstruction, latest, deviceInstruction string) { + t.Helper() + // recordInstructions is sticky (VOL-180): a field a response omits doesn't + // clear a value recorded by an earlier test. Reset explicitly so each test + // starts from the zero value regardless of execution order. + api.ResetLastInstructionsForTest() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.4"}) + if cliInstruction != "" { + w.Header().Set("X-Volcano-CLI-Instruction", cliInstruction) + } + if latest != "" { + w.Header().Set("X-Volcano-CLI-Latest-Version", latest) + } + if deviceInstruction != "" { + w.Header().Set("X-Volcano-Device-Instruction", deviceInstruction) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":[],"has_more":false,"page":1,"limit":100,"total":0}`)) })) - defer server.Close() + t.Cleanup(server.Close) - cmd := noticeTestCommand("version") - var out bytes.Buffer - cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Empty(t, out.String()) - assert.Zero(t, requests) + client, err := api.NewClient(server.URL, "", api.WithHTTPClient(server.Client())) + require.NoError(t, err) + _, err = client.ListProjects(context.Background(), api.DefaultPage, api.DefaultLimit) + require.NoError(t, err) } -func TestUpdateNoticeSkipsImplicitRootHelp(t *testing.T) { - setUpgradeTestCacheDir(t) +func TestPrintAPIInstructionNotices_Suggestion(t *testing.T) { oldVersion := version.Version version.Version = "v1.2.3" t.Cleanup(func() { version.Version = oldVersion }) + withInstructions(t, api.CLIInstructionSuggestionVersionUpgrade, "v1.5.0", "") - requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.4"}) - })) - defer server.Close() - - cmd := &cobra.Command{Use: "volcano"} + cmd := &cobra.Command{Use: "projects"} var out bytes.Buffer cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Empty(t, out.String()) - assert.Zero(t, requests) -} + PrintAPIInstructionNotices(cmd, cliruntime.Deps{}) -func TestUpdateNoticeSkipsCompletionCommands(t *testing.T) { - setUpgradeTestCacheDir(t) - oldVersion := version.Version - version.Version = "v1.2.3" - t.Cleanup(func() { version.Version = oldVersion }) - - for _, name := range []string{"completion", "__complete", "__completeNoDesc"} { - t.Run(name, func(t *testing.T) { - requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.4"}) - })) - defer server.Close() - - cmd := noticeTestCommand(name) - var out bytes.Buffer - cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Empty(t, out.String()) - assert.Zero(t, requests) - }) - } + assert.Contains(t, out.String(), "A newer Volcano CLI version is available: v1.5.0 (current v1.2.3). Run `volcano upgrade` to upgrade.") } -func TestUpdateNoticeUsesFreshCache(t *testing.T) { - setUpgradeTestCacheDir(t) +func TestPrintAPIInstructionNotices_SuggestionWithoutLatestVersion(t *testing.T) { oldVersion := version.Version version.Version = "v1.2.3" t.Cleanup(func() { version.Version = oldVersion }) - writeUpgradeTestCache(t, "v1.2.4", time.Now().UTC()) + withInstructions(t, api.CLIInstructionSuggestionVersionUpgrade, "", "") - requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.5"}) - })) - defer server.Close() - - cmd := noticeTestCommand("init") + cmd := &cobra.Command{Use: "projects"} var out bytes.Buffer cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Contains(t, out.String(), "v1.2.4") - assert.NotContains(t, out.String(), "v1.2.5") - assert.Zero(t, requests) + PrintAPIInstructionNotices(cmd, cliruntime.Deps{}) + + assert.Contains(t, out.String(), "A newer Volcano CLI version is available (current v1.2.3). Run `volcano upgrade` to upgrade.") } -func TestUpdateNoticeUsesFreshUpToDateCache(t *testing.T) { - setUpgradeTestCacheDir(t) +func TestPrintAPIInstructionNotices_Deprecation(t *testing.T) { oldVersion := version.Version - version.Version = "v1.2.4" + version.Version = "v0.9.0" t.Cleanup(func() { version.Version = oldVersion }) - writeUpgradeTestCache(t, "v1.2.4", time.Now().UTC()) - - requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.5"}) - })) - defer server.Close() + withInstructions(t, api.CLIInstructionRequireVersionUpgrade, "v1.5.0", "") - cmd := noticeTestCommand("init") + cmd := &cobra.Command{Use: "login"} var out bytes.Buffer cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Empty(t, out.String()) - assert.Zero(t, requests) + PrintAPIInstructionNotices(cmd, cliruntime.Deps{}) + + assert.Contains(t, out.String(), "Volcano CLI v0.9.0 is no longer supported. Upgrade to v1.5.0 or later:\n volcano upgrade") } -func TestUpdateNoticeWritesCacheWhenAlreadyUpToDate(t *testing.T) { - setUpgradeTestCacheDir(t) - oldVersion := version.Version - version.Version = "v1.2.4" - t.Cleanup(func() { version.Version = oldVersion }) +func TestPrintAPIInstructionNotices_NotEnoughCredit(t *testing.T) { + // Reserved instruction (VOL-180 PR review discussion): the API never + // emits this yet, but the CLI-side handling is real and testable so a + // future billing-service integration needs no CLI change to start working. + withInstructions(t, api.CLIInstructionNotEnoughCredit, "", "") - requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.4"}) - })) - defer server.Close() - - cmd := noticeTestCommand("init") + cmd := &cobra.Command{Use: "functions"} var out bytes.Buffer cmd.SetErr(&out) - deps := cliruntime.Deps{HTTPClient: server.Client(), UpdateGitHubAPIURL: server.URL} - MaybePrintUpdateNotice(cmd, deps) - assert.Empty(t, out.String()) - assert.Equal(t, 1, requests) - - MaybePrintUpdateNotice(cmd, deps) - assert.Empty(t, out.String()) - assert.Equal(t, 1, requests) -} + PrintAPIInstructionNotices(cmd, cliruntime.Deps{}) -func TestUpdateNoticeRejectsFutureDatedCache(t *testing.T) { - setUpgradeTestCacheDir(t) - oldVersion := version.Version - version.Version = "v1.2.3" - t.Cleanup(func() { version.Version = oldVersion }) - // A future CheckedAt (clock skew, restored backup) must not be trusted as fresh — - // the notice check should re-fetch from the network instead. - writeUpgradeTestCache(t, "v1.2.4", time.Now().Add(48*time.Hour).UTC()) + assert.Contains(t, out.String(), "Your project does not have enough credit to complete this request.") +} - requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.5"}) - })) - defer server.Close() +func TestPrintAPIInstructionNotices_LowCreditWarning(t *testing.T) { + withInstructions(t, api.CLIInstructionLowCreditWarning, "", "") - cmd := noticeTestCommand("init") + cmd := &cobra.Command{Use: "functions"} var out bytes.Buffer cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Contains(t, out.String(), "v1.2.5") - assert.Equal(t, 1, requests) -} + PrintAPIInstructionNotices(cmd, cliruntime.Deps{}) -func TestUpdateNoticeRefreshesStaleCache(t *testing.T) { - setUpgradeTestCacheDir(t) - oldVersion := version.Version - version.Version = "v1.2.3" - t.Cleanup(func() { version.Version = oldVersion }) - writeUpgradeTestCache(t, "v1.2.4", time.Now().Add(-25*time.Hour).UTC()) + assert.Contains(t, out.String(), "Your project is running low on credit.") +} - requests := 0 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - requests++ - writeUpgradeJSON(t, w, update.Release{TagName: "v1.2.5"}) - })) - defer server.Close() +func TestPrintAPIInstructionNotices_UsesCommandPathPrefix(t *testing.T) { + withInstructions(t, api.CLIInstructionSuggestionVersionUpgrade, "v1.5.0", "") - cmd := noticeTestCommand("init") + cmd := &cobra.Command{Use: "projects"} var out bytes.Buffer cmd.SetErr(&out) - MaybePrintUpdateNotice(cmd, cliruntime.Deps{ - HTTPClient: server.Client(), - UpdateGitHubAPIURL: server.URL, - }) - assert.Contains(t, out.String(), "v1.2.5") - assert.Equal(t, 1, requests) + PrintAPIInstructionNotices(cmd, cliruntime.Deps{CommandPathPrefix: "acme"}) + + assert.Contains(t, out.String(), "Run `acme upgrade` to upgrade.") } func executeUpgradeCommand(t *testing.T, cmd *cobra.Command, args ...string) (string, error) { @@ -337,13 +220,6 @@ func executeUpgradeCommand(t *testing.T, cmd *cobra.Command, args ...string) (st return out.String(), err } -func noticeTestCommand(name string) *cobra.Command { - root := &cobra.Command{Use: "volcano"} - cmd := &cobra.Command{Use: name} - root.AddCommand(cmd) - return cmd -} - func newUpgradeTestServer(t *testing.T, binaryName string, binary []byte, checksums string) *httptest.Server { t.Helper() return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -371,22 +247,3 @@ func writeUpgradeJSON(t *testing.T, w http.ResponseWriter, value any) { w.Header().Set("Content-Type", "application/json") require.NoError(t, json.NewEncoder(w).Encode(value)) } - -func setUpgradeTestCacheDir(t *testing.T) string { - t.Helper() - dir := t.TempDir() - t.Setenv("HOME", dir) - t.Setenv("XDG_CACHE_HOME", filepath.Join(dir, ".cache")) - return dir -} - -func writeUpgradeTestCache(t *testing.T, latest string, checkedAt time.Time) { - t.Helper() - path, err := noticeCachePath() - require.NoError(t, err) - require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) - f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) - require.NoError(t, err) - defer func() { require.NoError(t, f.Close()) }() - require.NoError(t, json.NewEncoder(f).Encode(noticeCache{CheckedAt: checkedAt, Latest: latest})) -} diff --git a/internal/update/update.go b/internal/update/update.go index 99debe2..ad96832 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -56,12 +56,6 @@ type Options struct { RequireSignatureVerification bool } -// Notice describes an available upgrade. -type Notice struct { - Current string - Latest string -} - // Release is the subset of GitHub release metadata the CLI needs. type Release struct { TagName string `json:"tag_name"`