From 316bbfac5e2efec596dcd86e37d7702fe5ef5f5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 11 Dec 2024 13:54:35 +0100 Subject: [PATCH] refactor: Display excluded env vars in debug logs (#56) --- internal/commands/launch.go | 3 -- internal/env/env.go | 35 +++++++++++++------- internal/env/env_test.go | 66 ++++++++++++++++++++++--------------- 3 files changed, 63 insertions(+), 41 deletions(-) diff --git a/internal/commands/launch.go b/internal/commands/launch.go index 03e94d8..a830a35 100644 --- a/internal/commands/launch.go +++ b/internal/commands/launch.go @@ -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 @@ -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 diff --git a/internal/env/env.go b/internal/env/env.go index f36b1dd..7f340da 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -6,6 +6,7 @@ import ( "sort" "strings" "task-runner-launcher/internal/config" + "task-runner-launcher/internal/logs" ) const ( @@ -28,11 +29,9 @@ 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 { @@ -40,21 +39,28 @@ func allowedOnly(allowlist []string) []string { } 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] @@ -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 } diff --git a/internal/env/env_test.go b/internal/env/env_test.go index 566b2c5..fadfd46 100644 --- a/internal/env/env_test.go +++ b/internal/env/env_test.go @@ -6,46 +6,56 @@ import ( "sort" "task-runner-launcher/internal/config" "testing" + + "github.com/stretchr/testify/assert" ) -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, }, } @@ -56,14 +66,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 { + assert.Empty(t, included) + } else { + assert.Equal(t, tt.expectedInclude, included) } - if !reflect.DeepEqual(got, tt.expected) { - t.Errorf("AllowedOnly() = %v, want %v", got, tt.expected) + if tt.expectedExclude == nil { + assert.Empty(t, excluded) + } else { + assert.Equal(t, tt.expectedExclude, excluded) } }) } @@ -94,7 +108,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) }