From a164d2548122c46472d17e93a525a68c42d5faf8 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Fri, 20 Mar 2026 22:27:31 -0700 Subject: [PATCH 1/5] refactor: remove vestigial CLIVersion field and SetVersion option The CLIVersion field and SetVersion functional option on ClientFactory were a workaround for a circular dependency that no longer exists. The API client reads version from context via slackcontext.Version(ctx), making this indirection unnecessary. Replace with direct version.Raw() calls. --- cmd/root.go | 6 +++--- internal/shared/clients.go | 15 --------------- main.go | 3 ++- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 80e8e059..8919b902 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -148,7 +148,7 @@ func Init(ctx context.Context) (*cobra.Command, *shared.ClientFactory) { // updateNotification will check for an update in the background and print a message after the command runs var updateNotification *update.UpdateNotification - clients = shared.NewClientFactory(shared.SetVersion(version.Raw())) + clients = shared.NewClientFactory() rootCmd := NewRootCommand(clients, updateNotification) // Support `--version` by setting root command's `Version` and custom template. @@ -265,7 +265,7 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob // Init clients that use flags clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, nil) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.CLIVersion) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) // Init System ID if systemID, err := clients.Config.SystemConfig.InitSystemID(ctx); err != nil { @@ -297,7 +297,7 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob clients.Config.LoadExperiments(ctx, clients.IO.PrintDebug) style.ToggleLipgloss(clients.Config.WithExperimentOn(experiment.Lipgloss)) // TODO(slackcontext) Consolidate storing CLI version to slackcontext - clients.Config.Version = clients.CLIVersion + clients.Config.Version = version.Raw() // The domain auths (token->domain) shouldn't change for the execution of the CLI so preload them into config! clients.Config.DomainAuthTokens = clients.Auth().MapAuthTokensToDomains(ctx) diff --git a/internal/shared/clients.go b/internal/shared/clients.go index 2dc43eb5..e61b5ab9 100644 --- a/internal/shared/clients.go +++ b/internal/shared/clients.go @@ -45,7 +45,6 @@ type ClientFactory struct { API func() api.APIInterface AppClient func() *app.Client Auth func() auth.AuthInterface - CLIVersion string Config *config.Config EventTracker tracking.TrackingManager HookExecutor hooks.HookExecutor @@ -92,14 +91,6 @@ func NewClientFactory(options ...func(*ClientFactory)) *ClientFactory { clients.Auth = clients.defaultAuthFunc clients.Browser = clients.defaultBrowserFunc - // TODO: Temporary hack to get around circular dependency in internal/api/client.go since that imports version - // Follows pattern demonstrated by the GitHub CLI here https://github.com/cli/cli/blob/5a46c1cab601a3394caa8de85adb14f909b811e9/pkg/cmd/factory/default.go#L29 - // Used by the APIClient for its userAgent - // Currently needed because trying to get the version of the CLI from pkg/version/version.go would cause a circular dependency - // We can get rid of this once we refactor the code relationship between pkg/ and internal/ - // userAgent can get Slack CLI version from context which is defined in main.go, this approach bypass circular dependency. The clients.CLIVersion is retained for future code refactor purpose and serve SetVersion function - clients.CLIVersion = "" - // Custom values set by functional options // Learn more: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis for _, option := range options { @@ -323,12 +314,6 @@ func DebugMode(c *ClientFactory) { c.Config.DebugEnabled = true } -// SetVersion is a functional option that sets the Cli version that the API Client references -// Learn more: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis -func SetVersion(version string) func(c *ClientFactory) { - return func(c *ClientFactory) { c.CLIVersion = version } -} - // getDevHostname returns the hostname of the given URL if it is dev or a numbered dev instance func getDevHostname(host string) string { if host == "" { diff --git a/main.go b/main.go index 601b4b26..ae223676 100644 --- a/main.go +++ b/main.go @@ -80,7 +80,8 @@ func main() { func recoveryFunc() { // in the event of a panic, log panic if r := recover(); r != nil { - var clients = shared.NewClientFactory(shared.SetVersion(version.Raw())) + var clients = shared.NewClientFactory() + clients.Config.Version = version.Raw() var ctx = context.Background() ctx = slackcontext.SetSessionID(ctx, uuid.New().String()) From 0147ef497fbf6008edb3ca84cb6433a9c813cd08 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sun, 22 Mar 2026 20:32:08 -0700 Subject: [PATCH 2/5] refactor: remove vestigial Config.Version field Consumers now call version.Raw() directly instead of reading from Config.Version, which was a redundant copy set in two places. --- cmd/auth/logout.go | 5 +++-- cmd/root.go | 3 --- internal/auth/auth.go | 3 ++- internal/config/config.go | 1 - internal/iostreams/logfile.go | 3 ++- internal/pkg/apps/delete.go | 3 ++- internal/tracking/tracking.go | 3 ++- internal/tracking/tracking_test.go | 3 ++- main.go | 3 +-- 9 files changed, 14 insertions(+), 13 deletions(-) diff --git a/cmd/auth/logout.go b/cmd/auth/logout.go index b5937f51..04718bd5 100644 --- a/cmd/auth/logout.go +++ b/cmd/auth/logout.go @@ -26,6 +26,7 @@ import ( "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/internal/style" + "github.com/slackapi/slack-cli/internal/version" "github.com/spf13/cobra" ) @@ -95,7 +96,7 @@ func handleAuthRemoval(ctx context.Context, clients *shared.ClientFactory, auth // Update the API Host and Logstash Host to be the selected/default auth clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, &auth) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.Config.Version) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) // First, try to revoke the xoxe-xoxp (auth) token credential var xoxpToken = auth.Token @@ -118,7 +119,7 @@ func handleAuthRemoval(ctx context.Context, clients *shared.ClientFactory, auth // Update the API Host and Logstash Host to be the selected/default auth clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, nil) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.Config.Version) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) return nil } diff --git a/cmd/root.go b/cmd/root.go index 4d40c542..04a2319a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -296,9 +296,6 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob // Init configurations clients.Config.LoadExperiments(ctx, clients.IO.PrintDebug) style.ToggleLipgloss(clients.Config.WithExperimentOn(experiment.Lipgloss)) - // TODO(slackcontext) Consolidate storing CLI version to slackcontext - clients.Config.Version = version.Raw() - // The domain auths (token->domain) shouldn't change for the execution of the CLI so preload them into config! clients.Config.DomainAuthTokens = clients.Auth().MapAuthTokensToDomains(ctx) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 61253336..e22d3a88 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -35,6 +35,7 @@ import ( "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" + "github.com/slackapi/slack-cli/internal/version" "github.com/spf13/afero" ) @@ -399,7 +400,7 @@ func (c *Client) SetSelectedAuth(ctx context.Context, auth types.SlackAuth, conf // Often set after standard selections but custom authentication must set this // unless the command is exiting right after, like 'login'. config.APIHostResolved = c.ResolveAPIHost(ctx, config.APIHostFlag, &auth) - config.LogstashHostResolved = c.ResolveLogstashHost(ctx, config.APIHostResolved, config.Version) + config.LogstashHostResolved = c.ResolveLogstashHost(ctx, config.APIHostResolved, version.Raw()) // Set environment variables for app development configurations and processes. if _, ok := os.LookupEnv("SLACK_API_URL"); !ok { diff --git a/internal/config/config.go b/internal/config/config.go index dfe985cf..9993d742 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -59,7 +59,6 @@ type Config struct { TeamFlag string TokenFlag string NoColor bool - Version string // Feature experiments ExperimentsFlag []string diff --git a/internal/iostreams/logfile.go b/internal/iostreams/logfile.go index bb321fdf..ea46d729 100644 --- a/internal/iostreams/logfile.go +++ b/internal/iostreams/logfile.go @@ -29,6 +29,7 @@ import ( "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" + "github.com/slackapi/slack-cli/internal/version" ) // Constants @@ -79,7 +80,7 @@ func (io *IOStreams) InitLogFile(ctx context.Context) error { "Command": goutils.RedactPII(strings.Join(os.Args[0:], " ")), "SessionID": sessionID, "Slack-CLI-TraceID": traceID, - "Slack-CLI Version": io.config.Version, + "Slack-CLI Version": version.Raw(), "Operating System (OS)": runtime.GOOS, "System ID": io.config.SystemID, "Project ID": io.config.ProjectID, diff --git a/internal/pkg/apps/delete.go b/internal/pkg/apps/delete.go index e3485b64..02bbcb76 100644 --- a/internal/pkg/apps/delete.go +++ b/internal/pkg/apps/delete.go @@ -23,6 +23,7 @@ import ( "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/internal/version" ) // Delete will delete the app for this teamDomain both remotely (API) and locally (project) @@ -81,7 +82,7 @@ func getAuthSession(ctx context.Context, clients *shared.ClientFactory, auth typ // get an auth without updating the default auth. It's less important for the Login command that terminals afterward, // because on start up, the root command resolves the auth's current APIHost. clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, &auth) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.Config.Version) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) authSession, err := clients.API().ValidateSession(ctx, token) if err != nil { diff --git a/internal/tracking/tracking.go b/internal/tracking/tracking.go index 38b1f203..eddebe5d 100644 --- a/internal/tracking/tracking.go +++ b/internal/tracking/tracking.go @@ -32,6 +32,7 @@ import ( "github.com/slackapi/slack-cli/internal/ioutils" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/style" + "github.com/slackapi/slack-cli/internal/version" ) // TrackingManager is an interface for tracking metrics and events related to CLI activity @@ -168,7 +169,7 @@ func (e *EventTracker) FlushToLogstash(ctx context.Context, cfg *config.Config, // In this case, the root command was not initialized, so none of the bootup routine executed (flags weren't parsed, config not initialized, etc.). return nil } - versionString, _ := strings.CutPrefix(cfg.Version, "v") + versionString, _ := strings.CutPrefix(version.Raw(), "v") eventData := e.cleanSessionData(e.getSessionData()) sessionID, err := slackcontext.SessionID(ctx) if err != nil { diff --git a/internal/tracking/tracking_test.go b/internal/tracking/tracking_test.go index c25d9670..7d9b8970 100644 --- a/internal/tracking/tracking_test.go +++ b/internal/tracking/tracking_test.go @@ -26,6 +26,7 @@ import ( "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" + "github.com/slackapi/slack-cli/internal/version" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -129,7 +130,7 @@ func Test_Tracking_FlushToLogstash(t *testing.T) { }, "should strip 'v' prefix from version string": { setup: func(cfg *config.Config) { - cfg.Version = "v4.2.0" + version.Version = "v4.2.0" }, assertOnRequest: func(t *testing.T, req *http.Request) { payload, err := io.ReadAll(req.Body) diff --git a/main.go b/main.go index 0bb26ccc..108d6e75 100644 --- a/main.go +++ b/main.go @@ -81,13 +81,12 @@ func recoveryFunc() { // in the event of a panic, log panic if r := recover(); r != nil { var clients = shared.NewClientFactory() - clients.Config.Version = version.Raw() var ctx = context.Background() ctx = slackcontext.SetSessionID(ctx, uuid.New().String()) // set host for logging - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.Config.Version) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) clients.IO.PrintError(ctx, "Recovered from panic: %s\n%s", r, string(debug.Stack())) os.Exit(int(iostreams.ExitError)) } From e229a966fe039be781c0702eda9bc4cffdf636c2 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Mon, 23 Mar 2026 14:35:05 -0700 Subject: [PATCH 3/5] refactor: migrate version.Raw() callers to slackcontext.Version(ctx) Replace direct version.Raw() calls with slackcontext.Version(ctx) across all callers for improved testability via context-based dependency injection. - slackcontext.Version(ctx) now falls back to version.Raw() when the context is missing the version, returning both the fallback value and an error so callers can log a warning - ResolveLogstashHost no longer takes a cliVersion parameter; it reads the version from context internally - Tracking tests use context-based version injection instead of mutating the global version.Version variable --- cmd/app/add_test.go | 2 +- cmd/app/delete_test.go | 2 +- cmd/app/uninstall_test.go | 2 +- cmd/auth/logout.go | 5 ++- cmd/auth/logout_test.go | 10 +++--- cmd/root.go | 2 +- cmd/sandbox/create_test.go | 28 ++++++++-------- cmd/sandbox/delete_test.go | 10 +++--- cmd/sandbox/list_test.go | 10 +++--- internal/auth/auth.go | 14 +++++--- internal/auth/auth_mock.go | 4 +-- internal/iostreams/logfile.go | 8 +++-- internal/pkg/apps/delete.go | 3 +- internal/pkg/apps/delete_test.go | 2 +- internal/slackcontext/slackcontext.go | 16 ++++++---- internal/slackcontext/slackcontext_test.go | 37 +++++++++------------- internal/tracking/tracking.go | 7 ++-- internal/tracking/tracking_test.go | 10 ++++-- main.go | 3 +- 19 files changed, 92 insertions(+), 83 deletions(-) diff --git a/cmd/app/add_test.go b/cmd/app/add_test.go index e3fd2182..a7ecddde 100644 --- a/cmd/app/add_test.go +++ b/cmd/app/add_test.go @@ -894,7 +894,7 @@ func prepareAddMocks(t *testing.T, clients *shared.ClientFactory, clientsMock *s clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything). Return("api host") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything). + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything). Return("logstash host") manifestMock := &app.ManifestMockObject{} diff --git a/cmd/app/delete_test.go b/cmd/app/delete_test.go index d8444d0b..4c2956cb 100644 --- a/cmd/app/delete_test.go +++ b/cmd/app/delete_test.go @@ -164,7 +164,7 @@ func prepareCommonDeleteMocks(t *testing.T, cf *shared.ClientFactory, cm *shared cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything). Return("api host") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything). + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything). Return("logstash host") // Mock list command diff --git a/cmd/app/uninstall_test.go b/cmd/app/uninstall_test.go index cfa1f355..169ba898 100644 --- a/cmd/app/uninstall_test.go +++ b/cmd/app/uninstall_test.go @@ -104,7 +104,7 @@ func prepareCommonUninstallMocks(ctx context.Context, clients *shared.ClientFact // Mock API calls clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything). Return("api host") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything). + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything). Return("logstash host") clientsMock.API.On("ValidateSession", mock.Anything, mock.Anything).Return(api.AuthSession{ diff --git a/cmd/auth/logout.go b/cmd/auth/logout.go index 04718bd5..bc7b9905 100644 --- a/cmd/auth/logout.go +++ b/cmd/auth/logout.go @@ -26,7 +26,6 @@ import ( "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/internal/style" - "github.com/slackapi/slack-cli/internal/version" "github.com/spf13/cobra" ) @@ -96,7 +95,7 @@ func handleAuthRemoval(ctx context.Context, clients *shared.ClientFactory, auth // Update the API Host and Logstash Host to be the selected/default auth clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, &auth) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved) // First, try to revoke the xoxe-xoxp (auth) token credential var xoxpToken = auth.Token @@ -119,7 +118,7 @@ func handleAuthRemoval(ctx context.Context, clients *shared.ClientFactory, auth // Update the API Host and Logstash Host to be the selected/default auth clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, nil) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved) return nil } diff --git a/cmd/auth/logout_test.go b/cmd/auth/logout_test.go index e81ff024..d8787d0e 100644 --- a/cmd/auth/logout_test.go +++ b/cmd/auth/logout_test.go @@ -37,7 +37,7 @@ func TestLogoutCommand(t *testing.T) { CmdArgs: []string{"--all"}, Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) { clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com") + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com") clientsMock.Auth.On("Auths", mock.Anything).Return(fakeAuthsByTeamSlice, nil) clientsMock.Auth.On("RevokeToken", mock.Anything, fakeAuthsByTeamSlice[0].Token).Return(nil) clientsMock.Auth.On("RevokeToken", mock.Anything, fakeAuthsByTeamSlice[0].RefreshToken).Return(nil) @@ -61,7 +61,7 @@ func TestLogoutCommand(t *testing.T) { CmdArgs: []string{"--team", "team1"}, Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) { clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com") + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com") clientsMock.Auth.On("Auths", mock.Anything).Return(fakeAuthsByTeamSlice, nil) clientsMock.IO.On("SelectPrompt", mock.Anything, "Select an authorization to revoke", mock.Anything, iostreams.MatchPromptConfig(iostreams.SelectPromptConfig{ Flag: clientsMock.Config.Flags.Lookup("team"), @@ -85,7 +85,7 @@ func TestLogoutCommand(t *testing.T) { CmdArgs: []string{"--team", "T2"}, Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) { clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com") + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com") clientsMock.IO.On("SelectPrompt", mock.Anything, "Select an authorization to revoke", mock.Anything, iostreams.MatchPromptConfig(iostreams.SelectPromptConfig{ Flag: clientsMock.Config.Flags.Lookup("team"), })).Return(iostreams.SelectPromptResponse{ @@ -134,7 +134,7 @@ func TestLogoutCommand(t *testing.T) { CmdArgs: []string{}, Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) { clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com") + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com") clientsMock.Auth.On("Auths", mock.Anything).Return(fakeAuthsByTeamSlice, nil) clientsMock.IO.On("SelectPrompt", mock.Anything, "Select an authorization to revoke", []string{ FormatAuthLabel(fakeAuthsByTeamSlice[0]), @@ -162,7 +162,7 @@ func TestLogoutCommand(t *testing.T) { CmdArgs: []string{}, Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) { clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com") + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com") clientsMock.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{ fakeAuthsByTeamSlice[0], }, nil) diff --git a/cmd/root.go b/cmd/root.go index 9bf982cc..4d472450 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -265,7 +265,7 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob // Init clients that use flags clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, nil) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved) // Init System ID if systemID, err := clients.Config.SystemConfig.InitSystemID(ctx); err != nil { diff --git a/cmd/sandbox/create_test.go b/cmd/sandbox/create_test.go index 33b6677e..36d0619a 100644 --- a/cmd/sandbox/create_test.go +++ b/cmd/sandbox/create_test.go @@ -46,7 +46,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "test-box", "test-box", "mypass", "", "", 0, "", int64(0), false). Return("T123", "https://test-box.slack.com", nil) cm.API.On("UsersInfo", mock.Anything, mock.Anything, mock.Anything).Return(&types.UserInfo{Profile: types.UserProfile{}}, nil) @@ -73,7 +73,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "My Test Box", "my-test-box", "pass", "", "", 0, "", int64(0), false). Return("T789", "https://my-test-box.slack.com", nil) @@ -98,7 +98,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "tmp-box", "tmp-box", "pass", "", "", 0, "", mock.MatchedBy(func(v int64) bool { return v > 0 }), false). Return("T111", "https://tmp-box.slack.com", nil) @@ -122,7 +122,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "err-box", "err-box", "pass", "", "", 0, "", int64(0), false). Return("", "", errors.New("api_error")) @@ -145,7 +145,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "tpl-box", "tpl-box", "pass", "", "", 1, "", int64(0), false). Return("T333", "https://tpl-box.slack.com", nil) @@ -171,7 +171,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "partner-box", "partner-box", "pass", "", "", 0, "", int64(0), true). Return("T555", "https://partner-box.slack.com", nil) @@ -197,7 +197,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.AddDefaultMocks() cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)} @@ -221,7 +221,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "date-box", "date-box", "pass", "", "", 0, "", archiveEpoch, false). Return("T222", "https://date-box.slack.com", nil) @@ -247,7 +247,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.AddDefaultMocks() cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)} @@ -271,7 +271,7 @@ func TestCreateCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.AddDefaultMocks() cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)} @@ -306,7 +306,7 @@ func setupCreateMocks(t *testing.T, ctx context.Context, cm *shared.ClientsMock, testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, name, domain, password, "", "", 0, "", archiveEpoch, partner). Return("T222", "https://"+domain+".slack.com", nil) cm.AddDefaultMocks() @@ -319,7 +319,7 @@ func setupCreateAuthOnly(t *testing.T, ctx context.Context, cm *shared.ClientsMo testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.AddDefaultMocks() cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)} cm.Config.LoadExperiments(ctx, cm.IO.PrintDebug) @@ -494,7 +494,7 @@ func Test_getTemplateID(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "tpl-box", "tpl-box", "pass", "", "", 1, "", int64(0), false). Return("T333", "https://tpl-box.slack.com", nil) cm.AddDefaultMocks() @@ -519,7 +519,7 @@ func Test_getTemplateID(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("CreateSandbox", mock.Anything, testToken, "tpl-box", "tpl-box", "pass", "", "", 1, "", int64(0), false). Return("T333", "https://tpl-box.slack.com", nil) cm.AddDefaultMocks() diff --git a/cmd/sandbox/delete_test.go b/cmd/sandbox/delete_test.go index c5b0a33f..454ba87f 100644 --- a/cmd/sandbox/delete_test.go +++ b/cmd/sandbox/delete_test.go @@ -40,7 +40,7 @@ func TestDeleteCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken, UserID: "U123"}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("DeleteSandbox", mock.Anything, testToken, "T123").Return(nil) cm.API.On("ListSandboxes", mock.Anything, testToken, "").Return([]types.Sandbox{}, nil) cm.API.On("UsersInfo", mock.Anything, mock.Anything, mock.Anything).Return(&types.UserInfo{Profile: types.UserProfile{}}, nil) @@ -67,7 +67,7 @@ func TestDeleteCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken, UserID: "U123"}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("DeleteSandbox", mock.Anything, testToken, "T123").Return(nil) sandboxes := []types.Sandbox{ { @@ -102,7 +102,7 @@ func TestDeleteCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to delete the sandbox?", false).Return(false, nil) cm.AddDefaultMocks() @@ -125,7 +125,7 @@ func TestDeleteCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken, UserID: "U123"}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to delete the sandbox?", false).Return(true, nil) cm.API.On("DeleteSandbox", mock.Anything, testToken, "E0123456").Return(nil) cm.API.On("ListSandboxes", mock.Anything, testToken, "").Return([]types.Sandbox{}, nil) @@ -152,7 +152,7 @@ func TestDeleteCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("DeleteSandbox", mock.Anything, testToken, "T123").Return(errors.New("api_error")) cm.AddDefaultMocks() diff --git a/cmd/sandbox/list_test.go b/cmd/sandbox/list_test.go index d0148af8..3f4c9905 100644 --- a/cmd/sandbox/list_test.go +++ b/cmd/sandbox/list_test.go @@ -35,7 +35,7 @@ func TestListCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("ListSandboxes", mock.Anything, testToken, "").Return([]types.Sandbox{}, nil) cm.API.On("UsersInfo", mock.Anything, mock.Anything, mock.Anything).Return(&types.UserInfo{Profile: types.UserProfile{}}, nil) @@ -55,7 +55,7 @@ func TestListCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") sandboxes := []types.Sandbox{ { TeamID: "T123", @@ -84,7 +84,7 @@ func TestListCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") sandboxes := []types.Sandbox{ { TeamID: "T456", @@ -113,7 +113,7 @@ func TestListCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("ListSandboxes", mock.Anything, testToken, "active").Return([]types.Sandbox{}, nil) cm.API.On("UsersInfo", mock.Anything, mock.Anything, mock.Anything).Return(&types.UserInfo{Profile: types.UserProfile{}}, nil) @@ -131,7 +131,7 @@ func TestListCommand(t *testing.T) { testToken := "xoxb-test-token" cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil) cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com") - cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") + cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli") cm.API.On("ListSandboxes", mock.Anything, testToken, ""). Return([]types.Sandbox(nil), errors.New("api_error")) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index e22d3a88..b9674aa4 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -33,9 +33,9 @@ import ( "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" - "github.com/slackapi/slack-cli/internal/version" "github.com/spf13/afero" ) @@ -80,8 +80,8 @@ type AuthInterface interface { // ResolveAPIHost returns the API Host based on the API Host Flag, Dev Flag, Project Config, and Stored Auth API Host. ResolveAPIHost(ctx context.Context, apiHostFlag string, customAuth *types.SlackAuth) string - // ResolveLogstashHost returns the error log stash host based on API Host and CLI version - ResolveLogstashHost(ctx context.Context, apiHost string, cliVersion string) string + // ResolveLogstashHost returns the error log stash host based on API Host and CLI version from context + ResolveLogstashHost(ctx context.Context, apiHost string) string // MapAuthTokensToDomains groups tokens by API host then delineates the host MapAuthTokensToDomains(ctx context.Context) string @@ -400,7 +400,7 @@ func (c *Client) SetSelectedAuth(ctx context.Context, auth types.SlackAuth, conf // Often set after standard selections but custom authentication must set this // unless the command is exiting right after, like 'login'. config.APIHostResolved = c.ResolveAPIHost(ctx, config.APIHostFlag, &auth) - config.LogstashHostResolved = c.ResolveLogstashHost(ctx, config.APIHostResolved, version.Raw()) + config.LogstashHostResolved = c.ResolveLogstashHost(ctx, config.APIHostResolved) // Set environment variables for app development configurations and processes. if _, ok := os.LookupEnv("SLACK_API_URL"); !ok { @@ -509,7 +509,11 @@ func (c *Client) ResolveAPIHost(ctx context.Context, apiHostFlag string, customA // TODO: how does this play together with ResolveAPIHost above? // ResolveLogstashHost returns the error log stash host based on API Host and CLI version -func (c *Client) ResolveLogstashHost(ctx context.Context, apiHost string, cliVersion string) string { +func (c *Client) ResolveLogstashHost(ctx context.Context, apiHost string) string { + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + c.io.PrintDebug(ctx, "Warning: %s", err.Error()) + } c.io.PrintDebug(ctx, "Resolving logstash host, %s, %s", apiHost, cliVersion) if localBuildGitSHAInVersionRegex.Match([]byte(cliVersion)) { return "https://dev.slackb.com/events/cli" diff --git a/internal/auth/auth_mock.go b/internal/auth/auth_mock.go index 8eff5ece..1a08e47d 100644 --- a/internal/auth/auth_mock.go +++ b/internal/auth/auth_mock.go @@ -76,8 +76,8 @@ func (m *AuthMock) ResolveAPIHost(ctx context.Context, apiHostFlag string, custo return args.String(0) } -func (m *AuthMock) ResolveLogstashHost(ctx context.Context, apiHost string, cliVersion string) string { - args := m.Called(ctx, apiHost, cliVersion) +func (m *AuthMock) ResolveLogstashHost(ctx context.Context, apiHost string) string { + args := m.Called(ctx, apiHost) return args.String(0) } diff --git a/internal/iostreams/logfile.go b/internal/iostreams/logfile.go index ea46d729..1510f391 100644 --- a/internal/iostreams/logfile.go +++ b/internal/iostreams/logfile.go @@ -29,7 +29,6 @@ import ( "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" - "github.com/slackapi/slack-cli/internal/version" ) // Constants @@ -74,13 +73,18 @@ func (io *IOStreams) InitLogFile(ctx context.Context) error { return err } + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + log.Printf("Warning: %s", err.Error()) + } + // Log the Slack-CLI version, User's OS, SessionID, TraceID // But format data before writing them to the log file formatAndWriteDataToLogFile(logger, map[string]string{ "Command": goutils.RedactPII(strings.Join(os.Args[0:], " ")), "SessionID": sessionID, "Slack-CLI-TraceID": traceID, - "Slack-CLI Version": version.Raw(), + "Slack-CLI Version": cliVersion, "Operating System (OS)": runtime.GOOS, "System ID": io.config.SystemID, "Project ID": io.config.ProjectID, diff --git a/internal/pkg/apps/delete.go b/internal/pkg/apps/delete.go index 02bbcb76..dcba44a0 100644 --- a/internal/pkg/apps/delete.go +++ b/internal/pkg/apps/delete.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" - "github.com/slackapi/slack-cli/internal/version" ) // Delete will delete the app for this teamDomain both remotely (API) and locally (project) @@ -82,7 +81,7 @@ func getAuthSession(ctx context.Context, clients *shared.ClientFactory, auth typ // get an auth without updating the default auth. It's less important for the Login command that terminals afterward, // because on start up, the root command resolves the auth's current APIHost. clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, &auth) - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved) authSession, err := clients.API().ValidateSession(ctx, token) if err != nil { diff --git a/internal/pkg/apps/delete_test.go b/internal/pkg/apps/delete_test.go index 117068c9..e3e1d23a 100644 --- a/internal/pkg/apps/delete_test.go +++ b/internal/pkg/apps/delete_test.go @@ -81,7 +81,7 @@ func TestAppsDelete(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api host") - clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash host") + clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash host") clientsMock.API.On("ValidateSession", mock.Anything, mock.Anything).Return(api.AuthSession{ TeamName: &tc.auth.TeamDomain, TeamID: &tc.auth.TeamID, diff --git a/internal/slackcontext/slackcontext.go b/internal/slackcontext/slackcontext.go index ccb86ec3..b92aff47 100644 --- a/internal/slackcontext/slackcontext.go +++ b/internal/slackcontext/slackcontext.go @@ -31,6 +31,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/internal/version" ) // contextKey is an unexported type to avoid context key collisions. @@ -170,15 +171,16 @@ func SetSystemID(ctx context.Context, systemID string) context.Context { return ctx } -// Version returns the CLI version associated with `ctx`, or -// `""` and `slackerror.ErrContextValueNotFound` if no version could be found. +// Version returns the CLI version associated with `ctx`. If the version is not +// found in the context, it falls back to version.Raw() and returns an error to +// signal that the context was not properly initialized. func Version(ctx context.Context) (string, error) { - var version, ok = ctx.Value(contextKeyVersion).(string) - if !ok || version == "" { - return "", slackerror.New(slackerror.ErrContextValueNotFound). - WithMessage("The value for Version could not be found") + var v, ok = ctx.Value(contextKeyVersion).(string) + if !ok || v == "" { + return version.Raw(), slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for Version could not be found in context, falling back to version.Raw()") } - return version, nil + return v, nil } // SetVersion adds the slack-cli version to Golang context for trace logging diff --git a/internal/slackcontext/slackcontext_test.go b/internal/slackcontext/slackcontext_test.go index d09b724d..12bfba99 100644 --- a/internal/slackcontext/slackcontext_test.go +++ b/internal/slackcontext/slackcontext_test.go @@ -22,6 +22,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/tracer" + "github.com/slackapi/slack-cli/internal/version" "github.com/stretchr/testify/require" ) @@ -288,28 +289,20 @@ func Test_SlackContext_SetSystemID(t *testing.T) { } func Test_SlackContext_Version(t *testing.T) { - tests := map[string]struct { - expectedVersion string - expectedError error - }{ - "returns Version when it exists": { - expectedVersion: "v1.2.3", - expectedError: nil, - }, - "returns error when Version not found": { - expectedVersion: "", - expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for Version could not be found"), - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - ctx := t.Context() - ctx = context.WithValue(ctx, contextKeyVersion, tc.expectedVersion) - actualVersion, actualError := Version(ctx) - require.Equal(t, tc.expectedVersion, actualVersion) - require.Equal(t, tc.expectedError, actualError) - }) - } + t.Run("returns Version when it exists", func(t *testing.T) { + ctx := t.Context() + ctx = context.WithValue(ctx, contextKeyVersion, "v1.2.3") + actualVersion, actualError := Version(ctx) + require.Equal(t, "v1.2.3", actualVersion) + require.NoError(t, actualError) + }) + + t.Run("falls back to version.Raw() when Version not found", func(t *testing.T) { + ctx := t.Context() + actualVersion, actualError := Version(ctx) + require.Equal(t, version.Raw(), actualVersion) + require.ErrorContains(t, actualError, "falling back to version.Raw()") + }) } func Test_SlackContext_SetVersion(t *testing.T) { diff --git a/internal/tracking/tracking.go b/internal/tracking/tracking.go index eddebe5d..1dca61e0 100644 --- a/internal/tracking/tracking.go +++ b/internal/tracking/tracking.go @@ -32,7 +32,6 @@ import ( "github.com/slackapi/slack-cli/internal/ioutils" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/style" - "github.com/slackapi/slack-cli/internal/version" ) // TrackingManager is an interface for tracking metrics and events related to CLI activity @@ -169,7 +168,11 @@ func (e *EventTracker) FlushToLogstash(ctx context.Context, cfg *config.Config, // In this case, the root command was not initialized, so none of the bootup routine executed (flags weren't parsed, config not initialized, etc.). return nil } - versionString, _ := strings.CutPrefix(version.Raw(), "v") + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + ioStream.PrintDebug(ctx, "Warning: %s", err.Error()) + } + versionString, _ := strings.CutPrefix(cliVersion, "v") eventData := e.cleanSessionData(e.getSessionData()) sessionID, err := slackcontext.SessionID(ctx) if err != nil { diff --git a/internal/tracking/tracking_test.go b/internal/tracking/tracking_test.go index 7d9b8970..31e5e250 100644 --- a/internal/tracking/tracking_test.go +++ b/internal/tracking/tracking_test.go @@ -15,6 +15,7 @@ package tracking import ( + "context" "fmt" "io" "net/http" @@ -26,7 +27,6 @@ import ( "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" - "github.com/slackapi/slack-cli/internal/version" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -81,6 +81,7 @@ func Test_Tracking_FlushToLogstash(t *testing.T) { assertOnRequest func(t *testing.T, req *http.Request) shouldNotSendRequest bool setup func(cfg *config.Config) + ctxSetup func(ctx context.Context) context.Context }{ "should always send an array to the event logging endpoint": { exitCode: iostreams.ExitOK, @@ -129,8 +130,8 @@ func Test_Tracking_FlushToLogstash(t *testing.T) { shouldNotSendRequest: true, }, "should strip 'v' prefix from version string": { - setup: func(cfg *config.Config) { - version.Version = "v4.2.0" + ctxSetup: func(ctx context.Context) context.Context { + return slackcontext.SetVersion(ctx, "v4.2.0") }, assertOnRequest: func(t *testing.T, req *http.Request) { payload, err := io.ReadAll(req.Body) @@ -151,6 +152,9 @@ func Test_Tracking_FlushToLogstash(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) + if tc.ctxSetup != nil { + ctx = tc.ctxSetup(ctx) + } et := NewEventTracker() var requestSent = false testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { diff --git a/main.go b/main.go index 108d6e75..87ee1211 100644 --- a/main.go +++ b/main.go @@ -84,9 +84,10 @@ func recoveryFunc() { var ctx = context.Background() ctx = slackcontext.SetSessionID(ctx, uuid.New().String()) + ctx = slackcontext.SetVersion(ctx, version.Get()) // set host for logging - clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw()) + clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved) clients.IO.PrintError(ctx, "Recovered from panic: %s\n%s", r, string(debug.Stack())) os.Exit(int(iostreams.ExitError)) } From 354894cbe447c98d9fb2ab80e40b0ce7266e7598 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Mon, 23 Mar 2026 15:09:14 -0700 Subject: [PATCH 4/5] refactor: update slackcontext.Version fallback error message --- internal/slackcontext/slackcontext.go | 2 +- internal/slackcontext/slackcontext_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/slackcontext/slackcontext.go b/internal/slackcontext/slackcontext.go index b92aff47..1e277e0e 100644 --- a/internal/slackcontext/slackcontext.go +++ b/internal/slackcontext/slackcontext.go @@ -178,7 +178,7 @@ func Version(ctx context.Context) (string, error) { var v, ok = ctx.Value(contextKeyVersion).(string) if !ok || v == "" { return version.Raw(), slackerror.New(slackerror.ErrContextValueNotFound). - WithMessage("The value for Version could not be found in context, falling back to version.Raw()") + WithMessage("The value for version could not be found in context, falling back to the build version") } return v, nil } diff --git a/internal/slackcontext/slackcontext_test.go b/internal/slackcontext/slackcontext_test.go index 12bfba99..f011f7c9 100644 --- a/internal/slackcontext/slackcontext_test.go +++ b/internal/slackcontext/slackcontext_test.go @@ -301,7 +301,7 @@ func Test_SlackContext_Version(t *testing.T) { ctx := t.Context() actualVersion, actualError := Version(ctx) require.Equal(t, version.Raw(), actualVersion) - require.ErrorContains(t, actualError, "falling back to version.Raw()") + require.ErrorContains(t, actualError, "falling back to the build version") }) } From 1a0cadd5a8d582dd88570fb1a7213f2e21f7d51b Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Mon, 23 Mar 2026 15:18:30 -0700 Subject: [PATCH 5/5] refactor: use version.Raw() consistently in main.go --- main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index 87ee1211..b79aebf6 100644 --- a/main.go +++ b/main.go @@ -49,7 +49,7 @@ func main() { // Set context values sessionID := uuid.New().String() - cliVersion := version.Get() + cliVersion := version.Raw() ctx = slackcontext.SetSessionID(ctx, sessionID) ctx = slackcontext.SetVersion(ctx, cliVersion) @@ -84,7 +84,7 @@ func recoveryFunc() { var ctx = context.Background() ctx = slackcontext.SetSessionID(ctx, uuid.New().String()) - ctx = slackcontext.SetVersion(ctx, version.Get()) + ctx = slackcontext.SetVersion(ctx, version.Raw()) // set host for logging clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved)