diff --git a/internal/cmd/go.go b/internal/cmd/go.go index fc3dd9b7..1570a70b 100644 --- a/internal/cmd/go.go +++ b/internal/cmd/go.go @@ -23,7 +23,9 @@ var ( Args: true, SkipFlagParsing: true, Action: func(ctx *cli.Context) error { - pin.AutoPinOrchestrion(ctx.Context) + if err := pin.AutoPinOrchestrion(ctx.Context, ctx.App.Writer, ctx.App.ErrWriter); err != nil { + return cli.Exit(err, -1) + } if err := goproxy.Run(ctx.Context, ctx.Args().Slice(), goproxy.WithToolexec(binpath.Orchestrion, "toolexec")); err != nil { var exitErr *exec.ExitError diff --git a/internal/cmd/toolexec.go b/internal/cmd/toolexec.go index 2bb8a3c1..8ba41988 100644 --- a/internal/cmd/toolexec.go +++ b/internal/cmd/toolexec.go @@ -50,7 +50,9 @@ var Toolexec = &cli.Command{ } // Ensure Orchestrion is properly pinned - pin.AutoPinOrchestrion(ctx.Context) + if err := pin.AutoPinOrchestrion(ctx.Context, ctx.App.Writer, ctx.App.ErrWriter); err != nil { + return cli.Exit(err, -1) + } if proxyCmd.ShowVersion() { log.Trace().Strs("command", proxyCmd.Args()).Msg("Toolexec version command") diff --git a/internal/cmd/version.go b/internal/cmd/version.go index c73ced88..9bf6e346 100644 --- a/internal/cmd/version.go +++ b/internal/cmd/version.go @@ -9,7 +9,6 @@ import ( "fmt" "runtime" - "github.com/DataDog/orchestrion/internal/ensure" "github.com/DataDog/orchestrion/internal/version" "github.com/urfave/cli/v2" ) @@ -40,11 +39,6 @@ var Version = &cli.Command{ } if c.Bool("verbose") { - if startupVersion := ensure.StartupVersion(); startupVersion != version.Tag() { - if _, err := fmt.Fprintf(c.App.Writer, " (started as %s)", startupVersion); err != nil { - return err - } - } if _, err := fmt.Fprintf(c.App.Writer, " built with %s (%s/%s)", runtime.Version(), runtime.GOOS, runtime.GOARCH); err != nil { return err } diff --git a/internal/cmd/version_test.go b/internal/cmd/version_test.go index b1a51ed6..ea070a60 100644 --- a/internal/cmd/version_test.go +++ b/internal/cmd/version_test.go @@ -55,12 +55,11 @@ func TestVersion(t *testing.T) { t.Run("verbose with respawn", func(t *testing.T) { var output bytes.Buffer - t.Setenv("DD_ORCHESTRION_STARTUP_VERSION", "v0.0.0") set := *set set.Parse([]string{"-verbose"}) ctx := cli.NewContext(&cli.App{Writer: &output}, &set, nil) require.NoError(t, cmd.Version.Action(ctx)) - require.Equal(t, fmt.Sprintf("orchestrion %s (started as v0.0.0) built with %s (%s/%s)\n", version.Tag(), runtime.Version(), runtime.GOOS, runtime.GOARCH), output.String()) + require.Equal(t, fmt.Sprintf("orchestrion %s built with %s (%s/%s)\n", version.Tag(), runtime.Version(), runtime.GOOS, runtime.GOARCH), output.String()) }) } diff --git a/internal/ensure/integration/main.go b/internal/ensure/integration/main.go deleted file mode 100644 index eecbe1fc..00000000 --- a/internal/ensure/integration/main.go +++ /dev/null @@ -1,25 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2023-present Datadog, Inc. - -package main - -import ( - "context" - "fmt" - "log" - - "github.com/DataDog/orchestrion/internal/ensure" -) - -// main is the entry point of a command that is used by the [ensure] integration -// test to verify the [ensure.RequiredVersion] function behaves correctly in -// real conditions. -func main() { - if err := ensure.RequiredVersion(context.Background()); err != nil { - log.Fatalln(err) - } - - _, _ = fmt.Println("This command has not respawned!") -} diff --git a/internal/ensure/integration_test.go b/internal/ensure/integration_test.go deleted file mode 100644 index 819fe64a..00000000 --- a/internal/ensure/integration_test.go +++ /dev/null @@ -1,125 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2023-present Datadog, Inc. - -package ensure_test - -import ( - "bytes" - "fmt" - "os" - "os/exec" - "path/filepath" - "runtime" - "strings" - "testing" - - "github.com/DataDog/orchestrion/internal/version" - "github.com/stretchr/testify/require" - "golang.org/x/mod/semver" -) - -var ensureDir string - -func Test(t *testing.T) { - tmp, err := os.MkdirTemp("", "ensure_test-*") - require.NoError(t, err, "failed to create temporary working directory") - defer os.RemoveAll(tmp) - - testMain := filepath.Join(tmp, "bin", "test_main") - - _, err = shell(ensureDir, "go", "build", "-o", testMain, "./integration") - require.NoError(t, err, "failed to build test_main helper") - - type test struct { - // version is the orchestrion version to mention in the `go.mod` files' require directive. The - // test will run go mod tidy, so this version must be an existing release tag. We typically use - // `v0.6.0` for testing purposes, but you can specify another existing version if there is a - // reason to. If blank, no `require` directive will be added in `go.mod` to require orchestrion. - version string - // replaces causes the `go.mod` file to have a `replace` directive that redirects the - // Orchestrion package to the version that is currently being tested. - replaces bool - // output is the expected output from running the `test_main` command, which is created from - // compiling the `./integration` package. - output string - // fails is true when the `test_main` helper is expected to exit with a non-0 status code. When - // true, the value of `output` is not asserted against. - fails bool - } - for name, test := range map[string]test{ - "v0.9.0": {version: "v0.9.0", output: "orchestrion v0.9.0"}, - "replaced": {version: "v0.9.0", replaces: true, output: "This command has not respawned!"}, - "none": {fails: true}, - } { - t.Run(name, func(t *testing.T) { - if !test.replaces && semver.Compare(test.version, version.Tag()) >= 0 { - // Tests w/o replace can't run if the "happy" version has not been released yet. v0.9.0 includes a module path - // re-capitalization which forces us to skip temporarily at least until that is released. - t.Skipf("Skipping test because version %s is newer than the current version (%s)", test.version, version.Tag()) - } - - wd := filepath.Join(tmp, name) - require.NoError(t, os.Mkdir(wd, 0750), "failed to create test working directory") - - goMod := []string{ - "module integration_test_case", - "", - fmt.Sprintf("go %s", runtime.Version()[2:]), - "", - } - - if test.version != "" { - goMod = append(goMod, fmt.Sprintf("require github.com/DataDog/orchestrion %s", test.version), "") - - // So that "go mod tidy" does not remove the requirement... - require.NoError(t, - os.WriteFile(filepath.Join(wd, "tools.go"), []byte(strings.Join([]string{ - "//go:build tools", - "package tools", - "", - "import _ \"github.com/DataDog/orchestrion\"", - }, "\n")), 0o644), - "failed to write tools.go", - ) - } - if test.replaces { - goMod = append(goMod, fmt.Sprintf("replace github.com/DataDog/orchestrion => %s", filepath.Dir(filepath.Dir(ensureDir))), "") - } - - require.NoError(t, - os.WriteFile(filepath.Join(wd, "go.mod"), []byte(strings.Join(goMod, "\n")), 0o644), - "failed to create go.mod file", - ) - - _, err := shell(wd, "go", "mod", "tidy") - require.NoError(t, err, "failed to 'go mod tidy'") - - out, err := shell(wd, testMain, "version") - if test.fails { - _, ok := err.(*exec.ExitError) - require.True(t, ok, "unexpected error while running test_main: %v", err) - } else { - require.NoError(t, err, "failed to run test_main helper") - require.Equal(t, test.output, out, "unexpected output from test_main helper") - } - }) - } -} - -func shell(dir string, cmd string, args ...string) (string, error) { - var stdout bytes.Buffer - - child := exec.Command(cmd, args...) - child.Dir = dir - child.Stdout = &stdout - - err := child.Run() - return strings.TrimSpace(stdout.String()), err -} - -func init() { - _, file, _, _ := runtime.Caller(0) - ensureDir = filepath.Dir(file) -} diff --git a/internal/ensure/requiredversion.go b/internal/ensure/requiredversion.go index 51d27ece..a8f60182 100644 --- a/internal/ensure/requiredversion.go +++ b/internal/ensure/requiredversion.go @@ -9,11 +9,8 @@ import ( "context" "errors" "fmt" - "os" - "os/exec" "path/filepath" "runtime" - "syscall" "github.com/DataDog/orchestrion/internal/goenv" "github.com/DataDog/orchestrion/internal/version" @@ -21,40 +18,38 @@ import ( "golang.org/x/tools/go/packages" ) -const ( - orchestrionPkgPath = "github.com/DataDog/orchestrion" - envVarRespawnedFor = "DD_ORCHESTRION_RESPAWNED_FOR" - envVarStartupVersion = "DD_ORCHESTRION_STARTUP_VERSION" - envValRespawnReplaced = "" -) +const orchestrionPkgPath = "github.com/DataDog/orchestrion" -var ( - errRespawnLoop = errors.New("re-spawn loop detected") - orchestrionSrcDir string -) +var orchestrionSrcDir string + +// IncorrectVersionError is returned by [RequiredVersion] when the version of orchestrion running +// does not match the one required by `go.mod`. +type IncorrectVersionError struct { + // RequiredVersion is the version declared in `go.mod`, or a blank string if a `replace` directive + // for "github.com/DataDog/orchestrion" is present in `go.mod`. + RequiredVersion string +} // RequiredVersion makes sure the version of the tool currently running is the same as the one -// required in the current working directory's "go.mod" file by calling `syscall.Exec` with the -// relevant `go run` command if necessary to replace the current process with one using the required -// version. +// required in the current working directory's "go.mod" file. // // If this returns `nil`, the current process is running the correct version of the tool and can -// proceed with it's intended purpose. If it returns an error, that should be presented to the user -// before exiting with a non-0 status code. If the process was correctly substituted, this function -// never returns control to its caller (as the process has been replaced). +// proceed with it's intended purpose. If it returns an [IncorrectVersionError], the caller should +// determine whether to print a warning or exit in error, presenting the returned error to the user. func RequiredVersion(ctx context.Context) error { - return requiredVersion(ctx, goModVersion, os.Getenv, syscall.Exec, os.Args) + return requiredVersion(ctx, goModVersion) } -// StartupVersion returns the version of Orchestrion that has started this process. If this is the -// same as version.Tag, this process hasn't needed to be re-started. This is useful to provide -// complete information about proxied executions (e.g: in the output of `orchestrion version`), -// in cases where a "globally" installed binary substituted itself for a version from `go.mod`. -func StartupVersion() string { - if env := os.Getenv(envVarStartupVersion); env != "" { - return env +func (e IncorrectVersionError) Error() string { + const useMessage = "use `go run github.com/DataDog/orchestrion` instead of just `orchestrion`" + if e.RequiredVersion == "" { + return "orchestrion is diverted by a replace directive; please " + useMessage } - return version.Tag() + return fmt.Sprintf( + "orchestrion@%s is required by `go.mod`, but this is orchestrion@%s - please run `go install github.com/DataDog/orchestrion@%[1]s` or "+useMessage, + e.RequiredVersion, + version.Tag(), + ) } // requiredVersion is the internal implementation of RequiredVersion, and takes the goModVersion and @@ -62,12 +57,7 @@ func StartupVersion() string { func requiredVersion( ctx context.Context, goModVersion func(context.Context, string) (string, string, error), - osGetenv func(string) string, - syscallExec func(argv0 string, argv []string, env []string) error, - osArgs []string, ) error { - log := zerolog.Ctx(ctx) - rVersion, path, err := goModVersion(ctx, "" /* Current working directory */) if err != nil { return fmt.Errorf("failed to determine go.mod requirement for %q: %w", orchestrionPkgPath, err) @@ -79,54 +69,7 @@ func requiredVersion( return nil } - if respawn := osGetenv(envVarRespawnedFor); respawn != "" && respawn != envValRespawnReplaced { - // We're already re-spawning for a non-local version, so we should not be re-spawning again... - // If that were the case, we'd likely end up in an infinite loop of re-spawning, which is very - // much undesirable. - return fmt.Errorf( - "%w (wanted %s, got %s, already respawning for %s)", - errRespawnLoop, - rVersion, - version.Tag(), - respawn, - ) - } - - if rVersion == "" { - // If there is no required version, it means a replace directive is in use, and it does not - // macth the running process' original source tree, so we will unconditionally re-spawn. - rVersion = envValRespawnReplaced - } - - log.Info().Msgf("Re-starting with '%s@%s' (this is %s)", orchestrionPkgPath, rVersion, version.Tag()) - - goBin, err := exec.LookPath("go") - if err != nil { - return fmt.Errorf("failed to resolve go from PATH: %w", err) - } - - if len(osArgs) == 0 { - panic("received 0-length osArgs, which is not supposed to happen") - } - - args := make([]string, len(osArgs)+2) - args[0] = goBin - args[1] = "run" - args[2] = orchestrionPkgPath - copy(args[3:], osArgs[1:]) - - env := os.Environ() - env = append( - env, - fmt.Sprintf("%s=%s", envVarRespawnedFor, rVersion), - fmt.Sprintf("%s=%s", envVarStartupVersion, version.Tag()), - ) - - runtime.LockOSThread() - defer runtime.UnlockOSThread() - - // Won't return control if successful, never returns a `nil` error value. - return syscallExec(goBin, args, env) + return IncorrectVersionError{RequiredVersion: rVersion} } // goModVersion returns the version and path of the "github.com/DataDog/orchestrion" module that is @@ -175,6 +118,6 @@ func goModVersion(ctx context.Context, dir string) (moduleVersion string, module } func init() { - _, file, _, _ := runtime.Caller(0) - orchestrionSrcDir = filepath.Dir(filepath.Dir(filepath.Dir(file))) + _, thisFile, _, _ := runtime.Caller(0) + orchestrionSrcDir = filepath.Join(thisFile, "..", "..", "..") } diff --git a/internal/ensure/requiredversion_test.go b/internal/ensure/requiredversion_test.go index f4ccce69..a3a8ffa3 100644 --- a/internal/ensure/requiredversion_test.go +++ b/internal/ensure/requiredversion_test.go @@ -14,7 +14,6 @@ import ( "path/filepath" "runtime" "strings" - "sync/atomic" "testing" "github.com/DataDog/orchestrion/internal/goenv" @@ -105,57 +104,44 @@ func TestGoModVersion(t *testing.T) { } func TestRequiredVersion(t *testing.T) { - goBin, err := goenv.GoBinPath() - require.NoError(t, err, "could not resolve go command path") - testError := errors.New("simulated failure") - osArgs := []string{"/path/to/go/compile", "-a", "./..."} type goModVersionResult struct { version string path string err error } - type expectedOutcome struct { - err error - respawns bool - } + type expectedOutcome = error type testCase struct { - goModVersion goModVersionResult - envVarRespawned string - expected expectedOutcome + goModVersion goModVersionResult + expected expectedOutcome } rawTag, _ := version.TagInfo() for name, tc := range map[string]testCase{ "happy path": { goModVersion: goModVersionResult{version: rawTag}, - expected: expectedOutcome{err: nil, respawns: false}, + expected: nil, }, "happy path, replaced to this": { goModVersion: goModVersionResult{path: orchestrionSrcDir}, - expected: expectedOutcome{err: nil, respawns: false}, + expected: nil, }, "go.mod failure": { goModVersion: goModVersionResult{err: testError}, - expected: expectedOutcome{err: testError}, + expected: testError, }, - "respawn needed (requires different version)": { + "different version required": { goModVersion: goModVersionResult{version: "v1337.42.0-phony.0"}, - expected: expectedOutcome{respawns: true}, + expected: IncorrectVersionError{RequiredVersion: "v1337.42.0-phony.0"}, }, - "respawn needed (blank required version, blank path)": { + "blank version and directory": { // This might never happen in the wild goModVersion: goModVersionResult{}, - expected: expectedOutcome{respawns: true}, + expected: IncorrectVersionError{RequiredVersion: ""}, }, - "respawn needed (blank required version, mismatched path)": { + "replaced to a different path": { goModVersion: goModVersionResult{path: "/phony/orchestrion/path"}, - expected: expectedOutcome{respawns: true}, - }, - "respawn loop": { - goModVersion: goModVersionResult{version: "v1337.42.0-phony.0"}, - envVarRespawned: "v1.2.3-example.1", - expected: expectedOutcome{err: errRespawnLoop}, + expected: IncorrectVersionError{RequiredVersion: ""}, }, } { t.Run(name, func(t *testing.T) { @@ -163,31 +149,10 @@ func TestRequiredVersion(t *testing.T) { require.Equal(t, "", dir) return tc.goModVersion.version, tc.goModVersion.path, tc.goModVersion.err } - mockGetenv := func(name string) string { - require.Equal(t, envVarRespawnedFor, name) - return tc.envVarRespawned - } - var syscallExecCalled atomic.Bool - mockSyscallExec := func(arg0 string, args []string, _ []string) error { - t.Helper() - syscallExecCalled.Store(true) - require.Equal(t, goBin, arg0) - require.GreaterOrEqual(t, len(args), 3) - require.Equal(t, []string{goBin, "run", orchestrionPkgPath}, args[:3]) - require.Equal(t, osArgs[1:], args[3:]) + err := requiredVersion(context.Background(), mockGoVersion) - return nil - } - - err := requiredVersion(context.Background(), mockGoVersion, mockGetenv, mockSyscallExec, osArgs) - - if tc.expected.err != nil { - require.ErrorIs(t, err, tc.expected.err) - } else { - require.NoError(t, err) - } - require.Equal(t, tc.expected.respawns, syscallExecCalled.Load()) + require.ErrorIs(t, err, tc.expected) }) } } diff --git a/internal/pin/auto.go b/internal/pin/auto.go index e39fcb44..c98ea138 100644 --- a/internal/pin/auto.go +++ b/internal/pin/auto.go @@ -7,7 +7,9 @@ package pin import ( "context" + "errors" "fmt" + "io" "os" "strings" @@ -21,60 +23,66 @@ import ( const envVarCheckedGoMod = "DD_ORCHESTRION_IS_GOMOD_VERSION" -// AutoPinOrchestrion automatically runs `pinOrchestrion` if the necessary -// requirements are not already met. It prints messages to `os.Stderr` to inform +// AutoPinOrchestrion automatically runs [PinOrchestrion] if the necessary +// requirements are not already met. It prints messages to `stderr` to inform // the user about what is going on. -func AutoPinOrchestrion(ctx context.Context) { +func AutoPinOrchestrion(ctx context.Context, stdout io.Writer, stderr io.Writer) error { log := zerolog.Ctx(ctx) if os.Getenv(envVarCheckedGoMod) == "true" { // A parent process (or ourselves earlier) has already done the check - return + return nil } // Make sure we don't do this again - defer func() { - _ = os.Setenv(envVarCheckedGoMod, "true") - }() + if err := os.Setenv(envVarCheckedGoMod, "true"); err != nil { + return fmt.Errorf("os.Setenv("+envVarCheckedGoMod+", true): %w", err) + } - var requiredVersionError error if _, isDev := version.TagInfo(); !isDev { - requiredVersionError = ensure.RequiredVersion(ctx) - if requiredVersionError == nil { + err := ensure.RequiredVersion(ctx) + if errors.As(err, &ensure.IncorrectVersionError{}) { + // There is already a required version, but we're not running that one! + log.Trace().Err(err).Msg("Orchestrion is already in go.mod, but we are not running the correct one; returning an error") + return err + } + if err == nil { // We're good to go - return + log.Trace().Msg("Orchestrion is already in go.mod, and we are running the correct version, no automatic pinning required") + return nil } + log.Trace().Err(err).Msg("Failed to detect required version of orchestrion from go.mod") } else { log.Trace().Msg("Skipping ensure.RequiredVersion because this is a development build") _, err := os.Stat(config.FilenameOrchestrionToolGo) if err == nil { log.Trace().Msg("Found " + config.FilenameOrchestrionToolGo + " file, no automatic pinning required") - return + return nil + } + if !errors.Is(err, os.ErrNotExist) { + log.Trace().Err(err).Msg("Failed to stat " + config.FilenameOrchestrionToolGo + ", returning error") + return err } - requiredVersionError = fmt.Errorf("stat %s: %w", config.FilenameOrchestrionToolGo, err) + log.Trace().Msg("No " + config.FilenameOrchestrionToolGo + " file found, will attempt automatic pinning") } - log.Trace().Err(requiredVersionError).Msg("Failed to detect required version of orchestrion from go.mod") - var ( box = lipgloss.NewStyle() stylePath = lipgloss.NewStyle() styleFile = lipgloss.NewStyle() styleCmd = lipgloss.NewStyle() ) - if term.IsTerminal(int(os.Stderr.Fd())) { + if stderr, isFile := stderr.(*os.File); isFile && term.IsTerminal(int(stderr.Fd())) { box = box.Border(lipgloss.RoundedBorder()). BorderForeground(lipgloss.ANSIColor(1)). Padding(1, 2) - if w, _, err := term.GetSize(int(os.Stderr.Fd())); err == nil { + if w, _, err := term.GetSize(int(stderr.Fd())); err == nil { box.Width(w - box.GetHorizontalMargins() - box.GetHorizontalBorderSize()) } stylePath = stylePath.Foreground(lipgloss.ANSIColor(4)).Underline(true) styleFile = styleFile.Foreground(lipgloss.ANSIColor(2)).Underline(true) styleCmd = styleCmd.Foreground(lipgloss.ANSIColor(5)).Bold(true).Underline(true) - } else { - _, _ = fmt.Fprintf(os.Stderr, "Version check error: %v\n", requiredVersionError) } var builder strings.Builder @@ -96,10 +104,11 @@ func AutoPinOrchestrion(ctx context.Context) { _, _ = builder.WriteString("\n\nYou should commit the resulting changes into your source control system.") message := builder.String() - _, _ = fmt.Fprintln(os.Stderr, box.Render(message)) + _, _ = fmt.Fprintln(stderr, box.Render(message)) - if err := PinOrchestrion(ctx, Options{}); err != nil { - _, _ = fmt.Fprintf(os.Stderr, "Failed to pin orchestrion in go.mod: %v\n", err) - os.Exit(1) + if err := PinOrchestrion(ctx, Options{Writer: stdout, ErrWriter: stderr}); err != nil { + return fmt.Errorf("failed to pin orchestrion in go.mod: %w", err) } + + return nil } diff --git a/internal/pin/auto_test.go b/internal/pin/auto_test.go index ab266f65..15714b2e 100644 --- a/internal/pin/auto_test.go +++ b/internal/pin/auto_test.go @@ -7,6 +7,7 @@ package pin import ( "context" + "io" "os" "path/filepath" "testing" @@ -25,7 +26,7 @@ func TestAutoPin(t *testing.T) { tmp := scaffold(t, make(map[string]string)) require.NoError(t, os.Chdir(tmp)) - AutoPinOrchestrion(context.Background()) + AutoPinOrchestrion(context.Background(), io.Discard, io.Discard) assert.NotEmpty(t, os.Getenv(envVarCheckedGoMod)) @@ -47,7 +48,7 @@ func TestAutoPin(t *testing.T) { t.Setenv(envVarCheckedGoMod, "true") - AutoPinOrchestrion(context.Background()) + AutoPinOrchestrion(context.Background(), io.Discard, io.Discard) assert.NoFileExists(t, filepath.Join(tmp, config.FilenameOrchestrionToolGo)) assert.NoFileExists(t, filepath.Join(tmp, "go.sum"))