diff --git a/cmd/kosli/main.go b/cmd/kosli/main.go index ef60bd0cc..a18161da4 100644 --- a/cmd/kosli/main.go +++ b/cmd/kosli/main.go @@ -7,6 +7,7 @@ import ( log "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/requests" + "github.com/kosli-dev/cli/internal/version" "github.com/spf13/cobra" _ "k8s.io/client-go/plugin/pkg/client/auth" ) @@ -43,6 +44,15 @@ func main() { func innerMain(cmd *cobra.Command, args []string) error { err := cmd.Execute() if err == nil { + // Cobra handles --version internally and bypasses all hooks, so we print + // the update notice here after the fact. + if cmd.Root().Flags().Changed("version") { + notice, _ := version.CheckForUpdate(version.GetVersion()) + if notice != "" { + _, _ = fmt.Fprint(logger.ErrOut, notice) + } + } + return nil } diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 655e610c1..bf9f1fe5e 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -329,13 +329,17 @@ func newRootCmd(out, errOut io.Writer, args []string) (*cobra.Command, error) { // Skip when: // - "version" subcommand: runs the check synchronously itself // - "__complete*": Cobra shell-completion commands fire on every Tab press - // - --version flag: Cobra handles it internally and skips PersistentPostRun, - // so the goroutine result would always be silently discarded - if cmd.Name() != "version" && !strings.HasPrefix(cmd.Name(), "__") && !cmd.Root().Flags().Changed("version") { - go func() { - notice, _ := version.CheckForUpdate(version.GetVersion()) - updateNoticeCh <- notice - }() + // Note: --version is handled by Cobra before any hooks run, so it never + // reaches this point; innerMain handles the notice for that case. + if cmd.Name() != "version" && !strings.HasPrefix(cmd.Name(), "__") { + f := cmd.Flags().Lookup("output") + // skip version checks if not using table output (programmatic usage) + if f == nil || f.Value.String() == "table" { + go func() { + notice, _ := version.CheckForUpdate(version.GetVersion()) + updateNoticeCh <- notice + }() + } } if global.ApiToken == "DRY_RUN" { diff --git a/cmd/kosli/root_test.go b/cmd/kosli/root_test.go index aa65f130b..9c4d13f56 100644 --- a/cmd/kosli/root_test.go +++ b/cmd/kosli/root_test.go @@ -1,8 +1,12 @@ package main import ( + "bytes" + "fmt" + "io" "testing" + "github.com/kosli-dev/cli/internal/version" "github.com/stretchr/testify/suite" ) @@ -30,3 +34,63 @@ func (suite *RootCommandTestSuite) TestConfigProcessing() { func TestRootCommandTestSuite(t *testing.T) { suite.Run(t, new(RootCommandTestSuite)) } + +type UpdateNoticeTestSuite struct { + suite.Suite + defaultKosliArguments string +} + +func (suite *UpdateNoticeTestSuite) SetupTest() { + global = &GlobalOpts{ + ApiToken: "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ImNkNzg4OTg5In0.e8i_lA_QrEhFncb05Xw6E_tkCHU9QfcY4OLTVUCHffY", + Org: "docs-cmd-test-user", + Host: "http://localhost:8001", + } + suite.defaultKosliArguments = fmt.Sprintf("--host %s --org %s --api-token %s", + global.Host, global.Org, global.ApiToken) +} + +func (suite *UpdateNoticeTestSuite) TestVersionFlagPrintsNotice() { + const fakeNotice = "\nA new version of the Kosli CLI is available: v9.99.0 (you have v0.0.1)\nUpgrade: https://docs.kosli.com/getting_started/install/\n" + + var errBuf bytes.Buffer + origErrOut := logger.ErrOut + logger.ErrOut = &errBuf + defer func() { logger.ErrOut = origErrOut }() + + cmd, err := newRootCmd(io.Discard, &errBuf, []string{"--version"}) + suite.Require().NoError(err) + + var called bool + defer version.SetCheckForUpdateOverride(func(string) (string, error) { + called = true + return fakeNotice, nil + })() + + cmd.SetArgs([]string{"--version"}) + suite.NoError(innerMain(cmd, []string{"kosli", "--version"})) + suite.True(called, "expected CheckForUpdate override to be called for --version") + suite.Contains(errBuf.String(), "A new version") +} + +func (suite *UpdateNoticeTestSuite) TestVersionNoticeSkippedForJSON() { + const fakeNotice = "\nA new version of the Kosli CLI is available: v9.99.0 (you have v0.0.1)\nUpgrade: https://docs.kosli.com/getting_started/install/\n" + + defer version.SetCheckForUpdateOverride(func(string) (string, error) { return fakeNotice, nil })() + + // with --output json: no notice in stderr + _, _, _, stderr, err := executeCommandC( + fmt.Sprintf("list flows --output json %s", suite.defaultKosliArguments)) + suite.NoError(err) + suite.Empty(stderr) + + // with --output table: notice IS in stderr + _, _, _, stderr, err = executeCommandC( + fmt.Sprintf("list flows --output table %s", suite.defaultKosliArguments)) + suite.NoError(err) + suite.Contains(stderr, "A new version") +} + +func TestUpdateNoticeTestSuite(t *testing.T) { + suite.Run(t, new(UpdateNoticeTestSuite)) +} diff --git a/cmd/kosli/version_test.go b/cmd/kosli/version_test.go index 52b272427..0339857e0 100644 --- a/cmd/kosli/version_test.go +++ b/cmd/kosli/version_test.go @@ -20,11 +20,11 @@ func (suite *VersionTestSuite) TestVersionCmd() { { name: "default", cmd: "version", - golden: fmt.Sprintf("version.BuildInfo{Version:\"main\", GitCommit:\"\", GitTreeState:\"\", GoVersion:\"%s\"}\n", runtime.Version()), + golden: fmt.Sprintf("version.BuildInfo{Version:\"dev\", GitCommit:\"\", GitTreeState:\"\", GoVersion:\"%s\"}\n", runtime.Version()), }, { name: "short", cmd: "version --short", - golden: "main\n", + golden: "dev\n", }, } runTestCmd(suite.T(), tests) diff --git a/internal/version/update_check.go b/internal/version/update_check.go index 1cc9ea2e0..e8eb50130 100644 --- a/internal/version/update_check.go +++ b/internal/version/update_check.go @@ -7,6 +7,7 @@ import ( "net/http" "os" "strings" + "sync" "time" semver "github.com/Masterminds/semver/v3" @@ -14,15 +15,42 @@ import ( const ( githubLatestReleaseURL = "https://api.github.com/repos/kosli-dev/cli/releases/latest" - updateCheckTimeout = 2 * time.Second + updateCheckTimeout = 1 * time.Second // max timeout when checking version ) type githubRelease struct { TagName string `json:"tag_name"` } +// overrideCheckForUpdate may be set by tests (via SetCheckForUpdateOverride) +// to replace the real HTTP check. +var ( + overrideMu sync.RWMutex + overrideCheckForUpdate func(currentVersion string) (string, error) +) + +// SetCheckForUpdateOverride replaces the implementation used by CheckForUpdate +// with fn and returns a function that restores the previous value. Tests only. +func SetCheckForUpdateOverride(fn func(currentVersion string) (string, error)) func() { + overrideMu.Lock() + old := overrideCheckForUpdate + overrideCheckForUpdate = fn + overrideMu.Unlock() + return func() { + overrideMu.Lock() + overrideCheckForUpdate = old + overrideMu.Unlock() + } +} + // CheckForUpdate is the public entry point — uses the real GitHub URL func CheckForUpdate(currentVersion string) (string, error) { + overrideMu.RLock() + fn := overrideCheckForUpdate + overrideMu.RUnlock() + if fn != nil { + return fn(currentVersion) + } return checkForUpdateWithURL(currentVersion, githubLatestReleaseURL) } @@ -33,11 +61,13 @@ func CheckForUpdate(currentVersion string) (string, error) { // so it never blocks or fails a command. // Set KOSLI_NO_UPDATE_CHECK=1 to skip entirely. func checkForUpdateWithURL(currentVersion string, apiURL string) (string, error) { + // checks disabled — skip if os.Getenv("KOSLI_NO_UPDATE_CHECK") != "" { return "", nil } - if currentVersion == "" || strings.HasPrefix(currentVersion, "main") || strings.Contains(currentVersion, "+unreleased") { - return "", nil // dev build — skip + // dev build — skip + if currentVersion == "" || strings.HasPrefix(currentVersion, "dev") { + return "", nil } // context provides the timeout and not http.Client diff --git a/internal/version/update_check_test.go b/internal/version/update_check_test.go index 31cd503a9..5dc14b397 100644 --- a/internal/version/update_check_test.go +++ b/internal/version/update_check_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -53,15 +54,21 @@ func TestCheckForUpdate_OptOut(t *testing.T) { assert.Empty(t, notice) } +func TestCheckForUpdate_EmptyVersion(t *testing.T) { + notice, err := checkForUpdateWithURL("", "http://should-not-be-called") + assert.NoError(t, err) + assert.Empty(t, notice) +} + func TestCheckForUpdate_DevBuild(t *testing.T) { // dev builds should be skipped without any HTTP call - notice, err := checkForUpdateWithURL("main", "http://should-not-be-called") + notice, err := checkForUpdateWithURL("dev", "http://should-not-be-called") assert.NoError(t, err) assert.Empty(t, notice) } -func TestCheckForUpdate_UnreleasedBuild(t *testing.T) { - notice, err := checkForUpdateWithURL("v1.0.0+unreleased", "http://should-not-be-called") +func TestCheckForUpdate_DevBuildWithMetadata(t *testing.T) { + notice, err := checkForUpdateWithURL("dev+unreleased", "http://should-not-be-called") assert.NoError(t, err) assert.Empty(t, notice) } @@ -93,3 +100,24 @@ func TestCheckForUpdate_Non200(t *testing.T) { assert.NoError(t, err) assert.Empty(t, notice) } + +func TestSetCheckForUpdateOverride_Race(t *testing.T) { + fake := func(string) (string, error) { return "notice", nil } + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, _ = CheckForUpdate("v1.2.3") + }() + } + for i := 0; i < 50; i++ { + wg.Add(1) + go func() { + defer wg.Done() + restore := SetCheckForUpdateOverride(fake) + restore() + }() + } + wg.Wait() +} diff --git a/internal/version/version.go b/internal/version/version.go index d9721821c..6678b8f57 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -11,7 +11,7 @@ var ( // // Increment major number for new feature additions and behavioral changes. // Increment minor number for bug fixes and performance enhancements. - version = "main" // this is overwritten with a release tag in the makefile + version = "dev" // this is overwritten with a release tag in the makefile // metadata is extra build time data metadata = "" diff --git a/internal/version/version_test.go b/internal/version/version_test.go index 44a25fc7c..94024c9e6 100644 --- a/internal/version/version_test.go +++ b/internal/version/version_test.go @@ -18,7 +18,7 @@ type VersionTestSuite struct { // reset the variables before each test func (suite *VersionTestSuite) SetupTest() { - version = "main" + version = "dev" metadata = "" gitCommit = "" gitTreeState = "" @@ -37,18 +37,18 @@ func (suite *VersionTestSuite) TestGetVersion() { want string }{ { - name: "version is main when metadata is empty.", + name: "version is dev when metadata is empty.", args: args{ metadata: "", }, - want: "main", + want: "dev", }, { name: "version is suffixed with metadat when metadata is not empty.", args: args{ metadata: "bla", }, - want: "main+bla", + want: "dev+bla", }, { name: "default version is overwritten when provided and there is metadata.",