Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/platform/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ func deployHook(ctx context.Context, clients *shared.ClientFactory) error {
// so we instantiate the default here.
shell := hooks.HookExecutorDefaultProtocol{
IO: clients.IO,
Fs: clients.Fs,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔭 note: Standalone protocol setups must define fs to access .env but we don't error otherwise. AFAICT this is required for just the deploy and run commands.

}
if _, err := shell.Execute(ctx, hookExecOpts); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/platform/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func TestDeployCommand_DeployHook(t *testing.T) {

clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) {
clients.SDKConfig = sdkConfigMock
clients.HookExecutor = hooks.GetHookExecutor(clientsMock.IO, sdkConfigMock)
clients.HookExecutor = hooks.GetHookExecutor(clientsMock.IO, clients.Fs, sdkConfigMock)
})
cmd := NewDeployCommand(clients)
cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return nil }
Expand Down
14 changes: 0 additions & 14 deletions internal/config/dotenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,10 @@ package config
import (
"strings"

"github.com/joho/godotenv"
"github.com/slackapi/slack-cli/internal/version"
"github.com/spf13/afero"
)

// GetDotEnvFileVariables collects only the variables in the .env file
func (c *Config) GetDotEnvFileVariables() (map[string]string, error) {
variables := map[string]string{}
file, err := afero.ReadFile(c.fs, ".env")
if err != nil && !c.os.IsNotExist(err) {
return variables, err
}
return godotenv.UnmarshalBytes(file)
}
Comment on lines -25 to -33
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📦 note: This is moved to our own slackdotenv package to avoid odd dependencies of config and to avoid duplicate logic!


// LoadEnvironmentVariables sets flags based on their environment variable value
//
// Note: Values are not loaded from the .env file. Use: `GetDotEnvFileVariables`
func (c *Config) LoadEnvironmentVariables() error {
// Skip when dependencies are not configured
if c.os == nil {
Expand Down
38 changes: 0 additions & 38 deletions internal/config/dotenv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,47 +18,9 @@ import (
"testing"

"github.com/slackapi/slack-cli/internal/slackdeps"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
)

func Test_DotEnv_GetDotEnvFileVariables(t *testing.T) {
tests := map[string]struct {
globalVariableName string
globalVariableValue string
localEnvFile string
expectedValues map[string]string
}{
"environment file variables are read": {
localEnvFile: "SLACK_VARIABLE=12\n",
expectedValues: map[string]string{"SLACK_VARIABLE": "12"},
},
"variable casing is preserved on load": {
localEnvFile: "secret_Token=Key123!\n",
expectedValues: map[string]string{"secret_Token": "Key123!"},
},
"global environment variables are ignored": {
globalVariableName: "SLACK_VARIABLE",
globalVariableValue: "12",
expectedValues: map[string]string{},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
fs := slackdeps.NewFsMock()
os := slackdeps.NewOsMock()
os.AddDefaultMocks()
os.Setenv(tc.globalVariableName, tc.globalVariableValue)
err := afero.WriteFile(fs, ".env", []byte(tc.localEnvFile), 0600)
assert.NoError(t, err)
config := NewConfig(fs, os)
variables, err := config.GetDotEnvFileVariables()
assert.NoError(t, err)
assert.Equal(t, tc.expectedValues, variables)
})
}
}

func Test_DotEnv_LoadEnvironmentVariables(t *testing.T) {
tableTests := map[string]struct {
envName string
Expand Down
4 changes: 3 additions & 1 deletion internal/hooks/hook_executor_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,20 @@ import (

"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/slackapi/slack-cli/internal/slackerror"
"github.com/spf13/afero"
)

// HookExecutorDefaultProtocol uses the original protocol between the CLI and the SDK where diagnostic info
// and hook responses come in via stdout. Data outside the expected JSON payload is ignored, with the
// exception of the 'start' hook, for which it is printed.
type HookExecutorDefaultProtocol struct {
IO iostreams.IOStreamer
Fs afero.Fs
}

// Execute processes the data received by the SDK.
func (e *HookExecutorDefaultProtocol) Execute(ctx context.Context, opts HookExecOpts) (string, error) {
cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(opts)
cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(ctx, opts, e.Fs, e.IO)
if err != nil {
return "", err
}
Expand Down
27 changes: 27 additions & 0 deletions internal/hooks/hook_executor_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/slackapi/slack-cli/internal/slackcontext"
"github.com/slackapi/slack-cli/internal/slackdeps"
"github.com/slackapi/slack-cli/internal/slackerror"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -80,6 +81,31 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) {
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `YIN=yang`)
},
},
"dotenv vars and hook vars are loaded into the environment": {
opts: HookExecOpts{
Hook: HookScript{Name: "happypath", Command: "echo {}"},
Env: map[string]string{
"OPTS_VAR": "from_opts",
},
Exec: &MockExec{
mockCommand: &MockCommand{
MockStdout: []byte("test output"),
Err: nil,
},
},
},
handler: func(t *testing.T, ctx context.Context, executor HookExecutor, opts HookExecOpts) {
// Write a .env file to the mock filesystem
e := executor.(*HookExecutorDefaultProtocol)
_ = afero.WriteFile(e.Fs, ".env", []byte("DOTENV_VAR=from_dotenv\n"), 0600)

response, err := executor.Execute(ctx, opts)
require.Equal(t, "test output", response)
require.NoError(t, err)
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `DOTENV_VAR=from_dotenv`)
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `OPTS_VAR=from_opts`)
},
},
"failed execution": {
opts: HookExecOpts{
Hook: HookScript{Command: "boom", Name: "sadpath"},
Expand Down Expand Up @@ -156,6 +182,7 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) {
ios.AddDefaultMocks()
hookExecutor := &HookExecutorDefaultProtocol{
IO: ios,
Fs: afero.NewMemMapFs(),
}
if tc.handler != nil {
tc.handler(t, ctx, hookExecutor, tc.opts)
Expand Down
4 changes: 3 additions & 1 deletion internal/hooks/hook_executor_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,23 @@ import (

"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/slackapi/slack-cli/internal/slackerror"
"github.com/spf13/afero"
)

// HookExecutorMessageBoundaryProtocol uses a protocol between the CLI and the SDK where diagnostic info
// and hook responses come in via stdout, and hook responses are wrapped in a string denoting the
// message boundary. Only one message payload can be received.
type HookExecutorMessageBoundaryProtocol struct {
IO iostreams.IOStreamer
Fs afero.Fs
}

// generateBoundary is a function for creating boundaries that can be mocked
var generateBoundary = generateMD5FromRandomString

// Execute processes the data received by the SDK.
func (e *HookExecutorMessageBoundaryProtocol) Execute(ctx context.Context, opts HookExecOpts) (string, error) {
cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(opts)
cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(ctx, opts, e.Fs, e.IO)
if err != nil {
return "", err
}
Expand Down
30 changes: 30 additions & 0 deletions internal/hooks/hook_executor_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/slackapi/slack-cli/internal/slackcontext"
"github.com/slackapi/slack-cli/internal/slackdeps"
"github.com/slackapi/slack-cli/internal/slackerror"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand All @@ -41,6 +42,7 @@ func mockBoundaryStringGenerator() string {
func Test_Hook_Execute_V2_Protocol(t *testing.T) {
tests := map[string]struct {
opts HookExecOpts
setup func(afero.Fs)
check func(*testing.T, string, error, ExecInterface)
}{
"error if hook command unavailable": {
Expand Down Expand Up @@ -132,6 +134,29 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
)
},
},
"dotenv vars and hook vars are loaded into the environment": {
opts: HookExecOpts{
Hook: HookScript{Name: "happypath", Command: "echo {}"},
Env: map[string]string{
"OPTS_VAR": "from_opts",
},
Exec: &MockExec{
mockCommand: &MockCommand{
MockStdout: []byte(mockBoundaryString + `{"ok": true}` + mockBoundaryString),
Err: nil,
},
},
},
setup: func(fs afero.Fs) {
_ = afero.WriteFile(fs, ".env", []byte("DOTENV_VAR=from_dotenv\n"), 0600)
},
check: func(t *testing.T, response string, err error, mockExec ExecInterface) {
require.NoError(t, err)
require.Equal(t, `{"ok": true}`, response)
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `DOTENV_VAR=from_dotenv`)
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `OPTS_VAR=from_opts`)
},
},
"fail to parse payload due to improper boundary strings": {
opts: HookExecOpts{
Hook: HookScript{Name: "happypath", Command: "echo {}"},
Expand Down Expand Up @@ -176,8 +201,13 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
config := config.NewConfig(fs, os)
ios := iostreams.NewIOStreamsMock(config, fs, os)
ios.AddDefaultMocks()
memFs := afero.NewMemMapFs()
hookExecutor := &HookExecutorMessageBoundaryProtocol{
IO: ios,
Fs: memFs,
}
if tc.setup != nil {
tc.setup(memFs)
}
response, err := hookExecutor.Execute(ctx, tc.opts)
tc.check(t, response, err, tc.opts.Exec)
Expand Down
16 changes: 6 additions & 10 deletions internal/hooks/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,32 +16,34 @@ package hooks

import (
"context"
"os"
"strings"

"github.com/slackapi/slack-cli/internal/goutils"
"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/spf13/afero"
)

type HookExecutor interface {
Execute(ctx context.Context, opts HookExecOpts) (response string, err error)
}

func GetHookExecutor(ios iostreams.IOStreamer, cfg SDKCLIConfig) HookExecutor {
func GetHookExecutor(ios iostreams.IOStreamer, fs afero.Fs, cfg SDKCLIConfig) HookExecutor {
protocol := cfg.Config.SupportedProtocols.Preferred()
switch protocol {
case HookProtocolV2:
return &HookExecutorMessageBoundaryProtocol{
IO: ios,
Fs: fs,
}
default:
return &HookExecutorDefaultProtocol{
IO: ios,
Fs: fs,
}
}
}

func processExecOpts(opts HookExecOpts) ([]string, []string, []string, error) {
func processExecOpts(ctx context.Context, opts HookExecOpts, fs afero.Fs, io iostreams.IOStreamer) ([]string, []string, []string, error) {
cmdStr, err := opts.Hook.Get()
if err != nil {
return []string{}, []string{}, []string{}, err
Expand All @@ -53,13 +55,7 @@ func processExecOpts(opts HookExecOpts) ([]string, []string, []string, error) {
var cmdArgVars = cmdArgs[1:] // omit the first item because that is the command name
cmdArgVars = append(cmdArgVars, goutils.MapToStringSlice(opts.Args, "--")...)

// Whatever cmd.Env is set to will be the ONLY environment variables that the `cmd` will have access to when it runs.
// To avoid removing any environment variables that are set in the current environment, we first set the cmd.Env to the current environment.
// before adding any new environment variables.
var cmdEnvVars = os.Environ()
for name, value := range opts.Env {
cmdEnvVars = append(cmdEnvVars, name+"="+value)
}
cmdEnvVars := opts.ShellEnv(ctx, fs, io)

return cmdArgs, cmdArgVars, cmdEnvVars, nil
}
2 changes: 1 addition & 1 deletion internal/hooks/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func Test_Hooks_GetHookExecutor(t *testing.T) {
io := iostreams.NewIOStreamsMock(config, fs, os)
sdkConfig := NewSDKConfigMock()
sdkConfig.Config.SupportedProtocols = tc.protocolVersions
hookExecutor := GetHookExecutor(io, sdkConfig)
hookExecutor := GetHookExecutor(io, fs, sdkConfig)
require.IsType(t, tc.expectedType, hookExecutor)
})
}
Expand Down
43 changes: 43 additions & 0 deletions internal/hooks/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@
package hooks

import (
"context"
"fmt"
"io"
"os"
"os/exec"
"runtime"
"strings"

"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/slackapi/slack-cli/internal/slackdotenv"
"github.com/spf13/afero"
)

// ExecInterface is an interface for running shell commands in the OS
Expand Down Expand Up @@ -92,3 +97,41 @@ type HookExecOpts struct {
Stderr io.Writer
Exec ExecInterface
}

// ShellEnv builds the environment variables for a hook command.
func (opts HookExecOpts) ShellEnv(ctx context.Context, fs afero.Fs, io iostreams.IOStreamer) []string {
// Gather environment variables saved to the project ".env" file
dotEnv, err := slackdotenv.Read(fs)
if err != nil {
io.PrintDebug(ctx, "Warning: failed to parse .env file: %s", err)
}
if len(dotEnv) > 0 {
keys := make([]string, 0, len(dotEnv))
for k := range dotEnv {
keys = append(keys, k)
}
io.PrintDebug(ctx, "Loaded variables from .env file: %s", strings.Join(keys, ", "))
}

// Whatever cmd.Env is set to will be the ONLY environment variables that the `cmd` will have access to when it runs.
//
// Order of precedence from lowest to highest:
// 1. Provided "opts.Env" variables
// 2. Saved ".env" file
// 3. Existing shell environment
//
// > Each entry is of the form "key=value".
// > ...
// > If Env contains duplicate environment keys, only the last value in the slice for each duplicate key is used.
//
// https://pkg.go.dev/os/exec#Cmd.Env
var cmdEnvVars []string
for name, value := range opts.Env {
cmdEnvVars = append(cmdEnvVars, name+"="+value)
}
for k, v := range dotEnv {
cmdEnvVars = append(cmdEnvVars, k+"="+v)
}
cmdEnvVars = append(cmdEnvVars, os.Environ()...)
return cmdEnvVars
}
Loading
Loading