diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0da5ed6..3f4ca63 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,7 +8,7 @@ on: branches: [main] permissions: - contents: write + contents: read jobs: test: @@ -62,12 +62,14 @@ jobs: if: startsWith(github.ref, 'refs/tags/v') needs: build runs-on: ubuntu-latest + permissions: + contents: write steps: - uses: actions/download-artifact@v8 with: path: dist merge-multiple: true - - uses: softprops/action-gh-release@v3 + - uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3.0.2 with: files: dist/* generate_release_notes: true diff --git a/Makefile b/Makefile index eb30071..fa46853 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ $(TARGETS): go.mod make $(platform) test: deps - $(GO) test + $(GO) test -race ./... fmt: $(GO) fmt diff --git a/README.md b/README.md index f00b841..cb7e493 100644 --- a/README.md +++ b/README.md @@ -142,9 +142,19 @@ GLOBAL OPTIONS: --version, -v print the version ``` +## Exit codes + +The wrapped command's exit code is passed through unchanged. ``env-aws-params`` +itself uses: + +- ``125`` — invalid usage or a Parameter Store error +- ``126`` — the command was found but could not be started +- ``127`` — the command was not found +- ``128+N`` — the command was killed by signal ``N`` (e.g. ``143`` for ``SIGTERM``) + ## Building from source -This project uses [Go modules](https://go.dev/blog/using-go-modules) and requires Go 1.26+. +This project uses [Go modules](https://go.dev/blog/using-go-modules) and requires Go 1.26.5+. ```bash go mod download diff --git a/data.go b/data.go index 918d680..a42b0d5 100644 --- a/data.go +++ b/data.go @@ -1,29 +1,57 @@ package main import ( - "fmt" + "maps" "regexp" - "sort" + "slices" "strings" + + log "github.com/sirupsen/logrus" ) var InvalidPattern = regexp.MustCompile(`[^a-zA-Z0-9_]`) -func BuildEnvVars(parameters map[string]string, sanitize bool, strip bool, upcase bool) []string { - var vars []string +// MergeEnvVars combines the inherited environment with SSM-derived variables, +// letting Parameter Store win whenever a key is present in both. +// +// Precedence is expressed purely through ordering: os/exec de-duplicates +// cmd.Env before handing it to the child and keeps the last occurrence of each +// key, so ssmVars is appended second. Pass a nil environ to inherit nothing. +// The result is always non-nil, because a nil cmd.Env makes os/exec fall back +// to the parent's environment. +func MergeEnvVars(ssmVars []string, environ []string) []string { + merged := make([]string, 0, len(environ)+len(ssmVars)) + merged = append(merged, environ...) + merged = append(merged, ssmVars...) + return merged +} - for k, v := range parameters { +func BuildEnvVars(parameters map[string]string, sanitize bool, strip bool, upcase bool) []string { + // Transform in sorted parameter order so collisions resolve deterministically. + values := make(map[string]string, len(parameters)) + sources := make(map[string]string, len(parameters)) + for _, name := range slices.Sorted(maps.Keys(parameters)) { + key := name if sanitize { - k = InvalidPattern.ReplaceAllString(k, "_") + key = InvalidPattern.ReplaceAllString(key, "_") } if strip { - k = InvalidPattern.ReplaceAllString(k, "") + key = InvalidPattern.ReplaceAllString(key, "") } if upcase { - k = strings.ToUpper(k) + key = strings.ToUpper(key) } - vars = append(vars, fmt.Sprintf("%s=%s", k, v)) + if prev, ok := sources[key]; ok { + log.Warnf("Parameters %q and %q both map to %s; keeping the value of %q", prev, name, key, name) + } + sources[key] = name + values[key] = parameters[name] + } + + vars := make([]string, 0, len(values)) + for key, value := range values { + vars = append(vars, key+"="+value) } - sort.Strings(vars) + slices.Sort(vars) return vars } diff --git a/data_test.go b/data_test.go index eda7041..346d410 100644 --- a/data_test.go +++ b/data_test.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os/exec" "sort" "testing" ) @@ -19,6 +20,59 @@ func AssertEqual(t *testing.T, value []string, expect []string) { } } +// AssertEqualOrdered is AssertEqual without the sort, for assertions where the +// ordering of the slice is itself the thing under test. +func AssertEqualOrdered(t *testing.T, value []string, expect []string) { + t.Helper() + if len(value) != len(expect) { + t.Fatalf("expected %v entries, got %v: %v", len(expect), len(value), value) + } + for i := range expect { + if value[i] != expect[i] { + t.Errorf("Values at offset %v do not match: expected %q, got %q", i, expect[i], value[i]) + } + } +} + +func TestMergeEnvVarsAppendsSSMLast(t *testing.T) { + ssmVars := []string{"ONLY_SSM=ssm", "SHARED=from-ssm"} + environ := []string{"ONLY_ENV=env", "SHARED=from-environ"} + + expectation := []string{ + "ONLY_ENV=env", "SHARED=from-environ", + "ONLY_SSM=ssm", "SHARED=from-ssm", + } + AssertEqualOrdered(t, MergeEnvVars(ssmVars, environ), expectation) +} + +func TestMergeEnvVarsPristineInheritsNothing(t *testing.T) { + ssmVars := []string{"ONLY_SSM=ssm"} + AssertEqualOrdered(t, MergeEnvVars(ssmVars, nil), ssmVars) +} + +func TestMergeEnvVarsIsNeverNil(t *testing.T) { + // A nil result would make os/exec inherit the parent environment. + if merged := MergeEnvVars(nil, nil); merged == nil { + t.Fatal("expected a non-nil slice, got nil") + } +} + +// The precedence contract holds only because os/exec keeps the last occurrence +// of a duplicate key, so assert it against a real child process rather than +// against the ordering of the slice alone. +func TestMergeEnvVarsSSMWinsInChildProcess(t *testing.T) { + cmd := exec.Command("/bin/sh", "-c", `printf '%s' "$SHARED"`) + cmd.Env = MergeEnvVars([]string{"SHARED=from-ssm"}, []string{"SHARED=from-environ"}) + + out, err := cmd.Output() + if err != nil { + t.Fatalf("running child: %v", err) + } + if got := string(out); got != "from-ssm" { + t.Fatalf("expected child to see %q, got %q", "from-ssm", got) + } +} + func TestBuildEnvVarsUpcaseFalse(t *testing.T) { var params map[string]string @@ -56,6 +110,16 @@ func TestBuildEnvVarsUpperSanitize(t *testing.T) { AssertEqual(t, envVars, expectation) } +func TestBuildEnvVarsCollisionKeepsLastSortedParameter(t *testing.T) { + params := map[string]string{ + "db-host": "from-dash", + "db_host": "from-underscore", + } + + envVars := BuildEnvVars(params, true, false, true) + AssertEqual(t, envVars, []string{"DB_HOST=from-underscore"}) +} + func TestBuildEnvVarsUpperStrip(t *testing.T) { var params map[string]string diff --git a/go.mod b/go.mod index 0628081..3bfd271 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module env-aws-params -go 1.26 +go 1.26.5 require ( github.com/aws/aws-sdk-go-v2 v1.41.7 diff --git a/main.go b/main.go index 0050b98..ee837c1 100644 --- a/main.go +++ b/main.go @@ -8,22 +8,32 @@ import ( "os" "os/exec" "strings" + "syscall" log "github.com/sirupsen/logrus" "github.com/urfave/cli/v3" "golang.org/x/sync/errgroup" ) -var VersionString string +var VersionString = "dev" func main() { + // Stop flag parsing at the wrapped command, so its own flags + // (e.g. sh -c) are passed through instead of rejected. + stopOnFirstArg := 1 cmd := &cli.Command{ - Name: "env-aws-params", - Usage: "Application entry-point that injects SSM Parameter Store values as Environment Variables", - UsageText: "env-aws-params [global options] -p prefix command [command arguments]", - Version: VersionString, - Flags: cliFlags(), - Action: action, + Name: "env-aws-params", + Usage: "Application entry-point that injects SSM Parameter Store values as Environment Variables", + UsageText: "env-aws-params [global options] -p prefix command [command arguments]", + Version: VersionString, + Flags: cliFlags(), + Action: action, + StopOnNthArg: &stopOnFirstArg, + // Flag parsing errors never reach action, so they are mapped to + // the documented usage-error exit code here. + OnUsageError: func(_ context.Context, _ *cli.Command, err error, _ bool) error { + return cli.Exit(errorPrefix(err), 125) + }, } if err := cmd.Run(context.Background(), os.Args); err != nil { log.Fatal(err) @@ -38,16 +48,15 @@ func action(ctx context.Context, cmd *cli.Command) error { log.SetOutput(io.Discard) } - code, err := validateArgs(cmd.NArg(), cmd.Bool("sanitize"), cmd.Bool("strip")) - if code > 0 { - return cli.Exit(errorPrefix(err), code) + if err := validateArgs(cmd.NArg(), cmd.Bool("sanitize"), cmd.Bool("strip")); err != nil { + return cli.Exit(errorPrefix(err), 125) } var envVars []string if len(cmd.StringSlice("prefix")) > 0 { params, err := getParameters(ctx, cmd) if err != nil { - return cli.Exit(errorPrefix(err), -1) + return cli.Exit(errorPrefix(err), 125) } envVars = BuildEnvVars( @@ -64,19 +73,28 @@ func action(ctx context.Context, cmd *cli.Command) error { log.Warn("No prefix set; executing command without retrieving SSM parameters") } - // SSM-derived values come first so they win over inherited environ: - // glibc/musl/Apple libc all return the first match from getenv(). - if !cmd.Bool("pristine") { - envVars = append(envVars, os.Environ()...) + // SSM-derived values are merged last so they win over the inherited + // environment; --pristine inherits nothing at all. See MergeEnvVars. + environ := os.Environ() + if cmd.Bool("pristine") { + environ = nil } + envVars = MergeEnvVars(envVars, environ) args := cmd.Args() if err := RunCommand(args.First(), args.Tail(), envVars); err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) { - return cli.Exit(errorPrefix(err), exitErr.ExitCode()) + code := exitErr.ExitCode() + if status, ok := exitErr.Sys().(syscall.WaitStatus); ok && status.Signaled() { + code = 128 + int(status.Signal()) + } + return cli.Exit(errorPrefix(err), code) } - return cli.Exit(errorPrefix(err), 128) + if errors.Is(err, exec.ErrNotFound) || errors.Is(err, os.ErrNotExist) { + return cli.Exit(errorPrefix(err), 127) + } + return cli.Exit(errorPrefix(err), 126) } return nil } @@ -168,14 +186,14 @@ func getParameters(ctx context.Context, cmd *cli.Command) (map[string]string, er return values, nil } -func validateArgs(nargs int, sanitize, strip bool) (int, error) { +func validateArgs(nargs int, sanitize, strip bool) error { if nargs == 0 { - return 1, errors.New("command not specified") + return errors.New("command not specified") } if sanitize && strip { - return 2, errors.New("--sanitize and --strip are mutually exclusive behaviors") + return errors.New("--sanitize and --strip are mutually exclusive behaviors") } - return 0, nil + return nil } diff --git a/main_test.go b/main_test.go index 6a1ffe4..2efd3a1 100644 --- a/main_test.go +++ b/main_test.go @@ -6,41 +6,25 @@ import ( ) func TestValidateArgsNoPrefixIsValid(t *testing.T) { - code, err := validateArgs(1, false, false) - if code != 0 { - t.Fatalf("expected code to be 0, got %v", code) - } - if err != nil { + if err := validateArgs(1, false, false); err != nil { t.Fatalf("expected err to be nil, got %v", err) } } func TestValidateArgsMissingCommand(t *testing.T) { - code, err := validateArgs(0, false, false) - if code != 1 { - t.Fatalf("expected code to be 1, got %v", code) - } - if err == nil { + if err := validateArgs(0, false, false); err == nil { t.Fatalf("expected err to be set, got nil") } } func TestValidateArgsStripAndSanitize(t *testing.T) { - code, err := validateArgs(1, true, true) - if code != 2 { - t.Fatalf("expected code to be 2, got %v", code) - } - if err == nil { + if err := validateArgs(1, true, true); err == nil { t.Fatalf("expected err to be set, got nil") } } func TestValidateArgsValid(t *testing.T) { - code, err := validateArgs(1, false, true) - if code != 0 { - t.Fatalf("expected code to be 0, got %v", code) - } - if err != nil { + if err := validateArgs(1, false, true); err != nil { t.Fatalf("expected err to be nil, got %v", err) } } diff --git a/runner.go b/runner.go index ee98f8d..d66dfc7 100644 --- a/runner.go +++ b/runner.go @@ -12,31 +12,41 @@ import ( func RunCommand(command string, args []string, envVars []string) error { cmd := exec.Command(command, args...) + + // A nil Env makes os/exec hand the child the parent's environment, which + // would silently defeat --pristine when there is nothing to inject. + if envVars == nil { + envVars = []string{} + } cmd.Env = envVars cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr + // Signals arriving before Start queue up in the channel and are forwarded + // once the child is running. + sigc := make(chan os.Signal, 32) + signal.Notify(sigc) + if err := cmd.Start(); err != nil { return err } log.Infof("PID %v running %s %s", cmd.Process.Pid, cmd.Path, strings.Join(args, " ")) - sigc := make(chan os.Signal, 1) - signal.Notify(sigc, - syscall.SIGHUP, - syscall.SIGINT, - syscall.SIGTERM, - syscall.SIGQUIT) go func() { - sigv := <-sigc - killErr := syscall.Kill(-os.Getpid(), sigv.(syscall.Signal)) - log.WithFields(log.Fields{ - "err": killErr, - "pid": -cmd.Process.Pid, - "signal": sigv, - }).Info("Caught signal, sent to child") + for sigv := range sigc { + // SIGCHLD is for the wrapper itself; SIGURG is used by the Go runtime. + if sigv == syscall.SIGCHLD || sigv == syscall.SIGURG { + continue + } + sigErr := cmd.Process.Signal(sigv) + log.WithFields(log.Fields{ + "err": sigErr, + "pid": cmd.Process.Pid, + "signal": sigv, + }).Debug("Forwarded signal to child") + } }() return cmd.Wait()