diff --git a/internal/config/config.go b/internal/config/config.go index fdedcd0d..9d3fc3d5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -24,6 +24,7 @@ import ( // Environment Variable constants const slackAccessibleEnv = "ACCESSIBLE" const slackAutoRequestAAAEnv = "SLACK_AUTO_REQUEST_AAA" +const slackCLIAppIconPathEnv = "SLACK_CLI_APP_ICON_PATH" const slackConfigDirEnv = "SLACK_CONFIG_DIR" const slackDisableTelemetryEnv = "SLACK_DISABLE_TELEMETRY" const slackTestTraceEnv = "SLACK_TEST_TRACE" @@ -43,6 +44,7 @@ type Config struct { APIHostFlag string APIHostResolved string AppFlag string + AppIconPathFlag string AutoRequestAAAFlag bool ConfigDirFlag string DebugEnabled bool diff --git a/internal/config/dotenv.go b/internal/config/dotenv.go index 39302b2b..94946aa1 100644 --- a/internal/config/dotenv.go +++ b/internal/config/dotenv.go @@ -51,6 +51,12 @@ func (c *Config) LoadEnvironmentVariables() error { c.ConfigDirFlag = configDir } + // Load app icon path from environment variables + var appIconPath = strings.TrimSpace(c.os.Getenv(slackCLIAppIconPathEnv)) + if appIconPath != "" { + c.AppIconPathFlag = appIconPath + } + // Disable telemetry if either disable-telemetry or test-version environment variables var disableTelemetry = strings.TrimSpace(c.os.Getenv(slackDisableTelemetryEnv)) var testVersion = strings.TrimSpace(c.os.Getenv(version.EnvTestVersion)) diff --git a/internal/config/dotenv_test.go b/internal/config/dotenv_test.go index 173c0140..3da40ced 100644 --- a/internal/config/dotenv_test.go +++ b/internal/config/dotenv_test.go @@ -153,6 +153,27 @@ func Test_DotEnv_LoadEnvironmentVariables(t *testing.T) { assert.Equal(t, false, cfg.AutoRequestAAAFlag) }, }, + "SLACK_CLI_APP_ICON_PATH=/path/to/icon.png should set AppIconPathFlag": { + envName: "SLACK_CLI_APP_ICON_PATH", + envValue: "/path/to/icon.png", + assertOnConfig: func(t *testing.T, cfg *Config) { + assert.Equal(t, "/path/to/icon.png", cfg.AppIconPathFlag) + }, + }, + "SLACK_CLI_APP_ICON_PATH= should not set AppIconPathFlag": { + envName: "SLACK_CLI_APP_ICON_PATH", + envValue: "", + assertOnConfig: func(t *testing.T, cfg *Config) { + assert.Equal(t, "", cfg.AppIconPathFlag) + }, + }, + "SLACK_CLI_APP_ICON_PATH with whitespace should be trimmed": { + envName: "SLACK_CLI_APP_ICON_PATH", + envValue: " /path/to/icon.png ", + assertOnConfig: func(t *testing.T, cfg *Config) { + assert.Equal(t, "/path/to/icon.png", cfg.AppIconPathFlag) + }, + }, "SLACK_CONFIG_DIR=/path/to/config should set the config dir": { envName: "SLACK_CONFIG_DIR", envValue: "/path/to/config", diff --git a/internal/icon/icon.go b/internal/icon/icon.go index 6ef57f72..b01edafc 100644 --- a/internal/icon/icon.go +++ b/internal/icon/icon.go @@ -20,10 +20,7 @@ import ( "github.com/spf13/afero" ) -func ResolveIconPath(fs afero.Fs, manifestIcon string) string { - if manifestIcon != "" { - return manifestIcon - } +func ResolveIconPath(fs afero.Fs) string { supportedExtensions := []string{".png", ".jpg", ".jpeg", ".gif"} for _, dir := range []string{"assets", "."} { for _, ext := range supportedExtensions { diff --git a/internal/icon/icon_test.go b/internal/icon/icon_test.go index 34651d39..4b370350 100644 --- a/internal/icon/icon_test.go +++ b/internal/icon/icon_test.go @@ -24,19 +24,9 @@ import ( func Test_ResolveIconPath(t *testing.T) { tests := map[string]struct { - manifestIcon string - files []string - expected string + files []string + expected string }{ - "manifest icon set returns it directly": { - manifestIcon: "custom/my-icon.png", - expected: "custom/my-icon.png", - }, - "manifest icon preferred over magic icon files": { - manifestIcon: "custom/my-icon.png", - files: []string{"assets/icon.png", "icon.png"}, - expected: "custom/my-icon.png", - }, "assets/icon.png found": { files: []string{"assets/icon.png"}, expected: "assets/icon.png", @@ -84,7 +74,7 @@ func Test_ResolveIconPath(t *testing.T) { for _, f := range tc.files { require.NoError(t, afero.WriteFile(fs, f, []byte("img"), 0o644)) } - result := ResolveIconPath(fs, tc.manifestIcon) + result := ResolveIconPath(fs) assert.Equal(t, tc.expected, result) }) } diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 9119567f..76c69324 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -218,8 +218,7 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac } } - // upload icon, default to assets/icon.{png,jpg,jpeg,gif} or icon.{png,jpg,jpeg,gif} - iconPath := icon.ResolveIconPath(clients.Fs, slackManifest.Icon) + iconPath := resolveIconPath(ctx, clients, slackManifest.Icon) if iconPath != "" { err = updateIcon(ctx, clients, iconPath, app.AppID, token, manifest.IsFunctionRuntimeSlackHosted()) if err != nil { @@ -519,7 +518,7 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran // upload icon for non-hosted apps (gated behind set-icon experiment) if clients.Config.WithExperimentOn(experiment.SetIcon) { - iconPath := icon.ResolveIconPath(clients.Fs, slackManifest.Icon) + iconPath := resolveIconPath(ctx, clients, slackManifest.Icon) if iconPath != "" { _, iconErr := clients.API().IconSet(ctx, clients.Fs, token, app.AppID, iconPath) if iconErr != nil { @@ -639,6 +638,28 @@ func appendLocalToDisplayName(manifest *types.AppManifest) { } } +// resolveIconPath determines the icon file path using the priority chain: +// SLACK_CLI_APP_ICON_PATH env var > manifest icon field > assets/ and root fallback +func resolveIconPath(ctx context.Context, clients *shared.ClientFactory, manifestIcon string) string { + if envIconPath := clients.Config.AppIconPathFlag; envIconPath != "" { + if _, err := clients.Fs.Stat(envIconPath); err == nil { + return envIconPath + } + clients.IO.PrintDebug(ctx, "SLACK_CLI_APP_ICON_PATH file not found: %s", envIconPath) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from SLACK_CLI_APP_ICON_PATH not found: %s", envIconPath))) + return "" + } + if manifestIcon != "" { + if _, err := clients.Fs.Stat(manifestIcon); err == nil { + return manifestIcon + } + clients.IO.PrintDebug(ctx, "manifest icon file not found: %s", manifestIcon) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from manifest not found: %s", manifestIcon))) + return "" + } + return icon.ResolveIconPath(clients.Fs) +} + // updateIcon will upload the new icon to the Slack API func updateIcon(ctx context.Context, clients *shared.ClientFactory, iconPath, appID string, token string, isHosted bool) error { var span opentracing.Span diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index f9610888..52a04953 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -29,6 +29,7 @@ import ( "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1728,6 +1729,86 @@ func TestContinueDespiteWarning(t *testing.T) { } } +func Test_resolveIconPath(t *testing.T) { + tests := map[string]struct { + envIconPath string + manifestIcon string + setupFiles func(t *testing.T, fs afero.Fs) + expected string + }{ + "env var takes priority over manifest icon": { + envIconPath: "env-icon.png", + manifestIcon: "manifest-icon.png", + setupFiles: func(t *testing.T, fs afero.Fs) { + require.NoError(t, afero.WriteFile(fs, "env-icon.png", []byte("img"), 0o644)) + require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644)) + }, + expected: "env-icon.png", + }, + "env var takes priority over fallback": { + envIconPath: "env-icon.png", + setupFiles: func(t *testing.T, fs afero.Fs) { + require.NoError(t, afero.WriteFile(fs, "env-icon.png", []byte("img"), 0o644)) + require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644)) + }, + expected: "env-icon.png", + }, + "manifest icon used when no env var": { + manifestIcon: "manifest-icon.png", + setupFiles: func(t *testing.T, fs afero.Fs) { + require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644)) + }, + expected: "manifest-icon.png", + }, + "falls back to assets/icon.png when no env var or manifest": { + setupFiles: func(t *testing.T, fs afero.Fs) { + require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644)) + }, + expected: "assets/icon.png", + }, + "falls back to icon.png in root when no assets": { + setupFiles: func(t *testing.T, fs afero.Fs) { + require.NoError(t, afero.WriteFile(fs, "icon.png", []byte("img"), 0o644)) + }, + expected: "icon.png", + }, + "returns empty when no icon found": { + setupFiles: func(t *testing.T, fs afero.Fs) {}, + expected: "", + }, + "env var file not found returns empty": { + envIconPath: "missing-icon.png", + manifestIcon: "manifest-icon.png", + setupFiles: func(t *testing.T, fs afero.Fs) { + require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644)) + }, + expected: "", + }, + "manifest icon file not found returns empty with warning": { + manifestIcon: "missing-manifest-icon.png", + setupFiles: func(t *testing.T, fs afero.Fs) { + require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644)) + }, + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.AddDefaultMocks() + clientsMock.Config.AppIconPathFlag = tc.envIconPath + tc.setupFiles(t, clientsMock.Fs) + output := &bytes.Buffer{} + clientsMock.IO.Stdout.SetOutput(output) + clients := shared.NewClientFactory(clientsMock.MockClientFactory()) + + result := resolveIconPath(ctx, clients, tc.manifestIcon) + assert.Equal(t, tc.expected, result) + }) + } +} + func Test_updateIcon(t *testing.T) { tests := map[string]struct { isHosted bool