Skip to content

Commit

Permalink
refactor: Display excluded env vars in debug logs
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov committed Dec 10, 2024
1 parent 84265a4 commit f9c34f6
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 41 deletions.
3 changes: 0 additions & 3 deletions internal/commands/launch.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ func (l *LaunchCommand) Execute(cfg *config.Config) error {

runnerEnv := env.PrepareRunnerEnv(cfg)

logs.Debugf("Prepared env vars for runner")

for {
// 3. check until task broker is ready

Expand Down Expand Up @@ -89,7 +87,6 @@ func (l *LaunchCommand) Execute(cfg *config.Config) error {
logs.Debug("Task ready for pickup, launching runner...")
logs.Debugf("Command: %s", cfg.Runner.Command)
logs.Debugf("Args: %v", cfg.Runner.Args)
logs.Debugf("Env vars: %v", env.Keys(runnerEnv))

ctx, cancelHealthMonitor := context.WithCancel(context.Background())
var wg sync.WaitGroup
Expand Down
35 changes: 23 additions & 12 deletions internal/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sort"
"strings"
"task-runner-launcher/internal/config"
"task-runner-launcher/internal/logs"
)

const (
Expand All @@ -28,33 +29,38 @@ const (
RunnerServerURI = "http://127.0.0.1:5681"
)

// allowedOnly filters the current environment down to only those
// environment variables in the allowlist.
func allowedOnly(allowlist []string) []string {
var filtered []string

// partitionByAllowlist divides the current env vars into those included in and
// excluded from the allowlist.
func partitionByAllowlist(allowlist []string) (included, excluded []string) {
for _, env := range os.Environ() {
parts := strings.SplitN(env, "=", 2)
if len(parts) != 2 {
continue
}

key := parts[0]
isAllowed := false
for _, allowedKey := range allowlist {
if key == allowedKey {
filtered = append(filtered, env)
included = append(included, env)
isAllowed = true
break
}
}
if !isAllowed {
excluded = append(excluded, env)
}
}

sort.Strings(filtered) // ensure consistent order
// ensure consistent order
sort.Strings(included)
sort.Strings(excluded)

return filtered
return included, excluded
}

// Keys returns the keys of the environment variables.
func Keys(env []string) []string {
// keys returns the keys of the environment variables.
func keys(env []string) []string {
keys := make([]string, len(env))
for i, env := range env {
keys[i] = strings.SplitN(env, "=", 2)[0]
Expand All @@ -81,10 +87,15 @@ func PrepareRunnerEnv(cfg *config.Config) []string {
defaultEnvs := []string{"LANG", "PATH", "TZ", "TERM"}
allowedEnvs := append(defaultEnvs, cfg.Runner.AllowedEnv...)

runnerEnv := allowedOnly(allowedEnvs)
runnerEnv = append(runnerEnv, "N8N_RUNNERS_HEALTH_CHECK_SERVER_ENABLED=true")
includedEnvs, excludedEnvs := partitionByAllowlist(allowedEnvs)

logs.Debugf("Env vars to exclude from runner: %v", keys(excludedEnvs))

runnerEnv := append(includedEnvs, "N8N_RUNNERS_HEALTH_CHECK_SERVER_ENABLED=true")
runnerEnv = append(runnerEnv, fmt.Sprintf("%s=%s", EnvVarAutoShutdownTimeout, cfg.AutoShutdownTimeout))
runnerEnv = append(runnerEnv, fmt.Sprintf("%s=%s", EnvVarTaskTimeout, cfg.TaskTimeout))

logs.Debugf("Env vars to pass to runner: %v", keys(runnerEnv))

return runnerEnv
}
64 changes: 38 additions & 26 deletions internal/env/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,52 @@ import (
"testing"
)

func TestAllowedOnly(t *testing.T) {
func TestPartitionByAllowlist(t *testing.T) {
tests := []struct {
name string
envVars map[string]string
allowed []string
expected []string
name string
envVars map[string]string
allowList []string
expectedInclude []string
expectedExclude []string
}{
{
name: "returns only allowed env vars",
name: "partitions env vars correctly",
envVars: map[string]string{
"ALLOWED1": "value1",
"ALLOWED2": "value2",
"NOT_ALLOWED1": "value3",
"NOT_ALLOWED2": "value4",
},
allowed: []string{"ALLOWED1", "ALLOWED2"},
expected: []string{
allowList: []string{"ALLOWED1", "ALLOWED2"},
expectedInclude: []string{
"ALLOWED1=value1",
"ALLOWED2=value2",
},
expectedExclude: []string{
"NOT_ALLOWED1=value3",
"NOT_ALLOWED2=value4",
},
},
{
name: "returns empty slice when no env vars match allowlist",
envVars: map[string]string{"FOO": "bar"},
allowed: []string{"BAZ"},
expected: nil,
name: "returns empty slices when no env vars match allowlist",
envVars: map[string]string{"FOO": "bar"},
allowList: []string{"BAZ"},
expectedInclude: nil,
expectedExclude: []string{"FOO=bar"},
},
{
name: "returns empty slice when allowlist is empty",
envVars: map[string]string{"FOO": "bar"},
allowed: []string{},
expected: nil,
name: "returns empty included and all in excluded when allowlist is empty",
envVars: map[string]string{"FOO": "bar"},
allowList: []string{},
expectedInclude: nil,
expectedExclude: []string{"FOO=bar"},
},
{
name: "returns empty slice when env vars is empty",
envVars: map[string]string{},
allowed: []string{"FOO"},
expected: nil,
name: "returns empty slices when env vars is empty",
envVars: map[string]string{},
allowList: []string{"FOO"},
expectedInclude: nil,
expectedExclude: nil,
},
}

Expand All @@ -56,14 +64,18 @@ func TestAllowedOnly(t *testing.T) {
os.Setenv(k, v)
}

got := allowedOnly(tt.allowed)
included, excluded := partitionByAllowlist(tt.allowList)

if tt.expected == nil && len(got) == 0 {
return
if tt.expectedInclude == nil && len(included) == 0 {

Check failure on line 69 in internal/env/env_test.go

View workflow job for this annotation

GitHub Actions / checks

empty-block: this block is empty, you can remove it (revive)
// ok
} else if !reflect.DeepEqual(included, tt.expectedInclude) {
t.Errorf("partitionByAllowlist() included = %v, want %v", included, tt.expectedInclude)
}

if !reflect.DeepEqual(got, tt.expected) {
t.Errorf("AllowedOnly() = %v, want %v", got, tt.expected)
if tt.expectedExclude == nil && len(excluded) == 0 {

Check failure on line 75 in internal/env/env_test.go

View workflow job for this annotation

GitHub Actions / checks

empty-block: this block is empty, you can remove it (revive)
// ok
} else if !reflect.DeepEqual(excluded, tt.expectedExclude) {
t.Errorf("partitionByAllowlist() excluded = %v, want %v", excluded, tt.expectedExclude)
}
})
}
Expand Down Expand Up @@ -94,7 +106,7 @@ func TestKeys(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := Keys(tt.input)
got := keys(tt.input)
if !reflect.DeepEqual(got, tt.expected) {
t.Errorf("Keys() = %v, want %v", got, tt.expected)
}
Expand Down

0 comments on commit f9c34f6

Please sign in to comment.