diff --git a/build/result.go b/build/result.go index ce83eaffc9f7..0f4b9da5a2e1 100644 --- a/build/result.go +++ b/build/result.go @@ -220,16 +220,24 @@ func containerConfigFromError(solveErr *errdefs.SolveError, cfg *InvokeConfig) ( if err != nil { return nil, err } + // When a solve fails before its inputs/mounts have been resolved (for + // example, when computing the cache key for an exec op fails because an + // input source cannot be found), MountIDs/InputIDs may be empty or shorter + // than the declared mount list. Guard against that so callers (such as the + // debug adapter) get a clean error instead of an index-out-of-range panic. + ids := solveErr.MountIDs + if cfg.Initial { + ids = solveErr.InputIDs + } + if len(ids) < len(exec.Mounts) { + return nil, errors.Errorf("cannot start debug container: failing step has no mount results available (got %d of %d)", len(ids), len(exec.Mounts)) + } var mounts []gateway.Mount for i, mnt := range exec.Mounts { - rid := solveErr.MountIDs[i] - if cfg.Initial { - rid = solveErr.InputIDs[i] - } mounts = append(mounts, gateway.Mount{ Selector: mnt.Selector, Dest: mnt.Dest, - ResultID: rid, + ResultID: ids[i], Readonly: mnt.Readonly, MountType: mnt.MountType, CacheOpt: mnt.CacheOpt, diff --git a/build/result_test.go b/build/result_test.go new file mode 100644 index 000000000000..2fa452ac2e7b --- /dev/null +++ b/build/result_test.go @@ -0,0 +1,132 @@ +package build_test + +import ( + "context" + "testing" + + "github.com/docker/buildx/build" + gateway "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/solver/errdefs" + "github.com/moby/buildkit/solver/pb" + "github.com/stretchr/testify/require" +) + +func Test_Creating_new_container_from_failing_result_handle(t *testing.T) { + t.Parallel() + + mounts := []*pb.Mount{ + {Dest: "/", Input: 0}, + {Dest: "/src", Input: 1}, + } + + run := func(t *testing.T, cfg build.InvokeConfig, inputIDs, mountIDs []string) (*gateway.NewContainerRequest, error) { + t.Helper() + ctx := t.Context() + gw := &stubGwClient{} + solveErr := &errdefs.SolveError{ + Solve: &errdefs.Solve{ + Op: &pb.Op{Op: &pb.Op_Exec{Exec: &pb.ExecOp{Mounts: mounts}}}, + InputIDs: inputIDs, + MountIDs: mountIDs, + }, + } + rh := build.NewResultHandle(ctx, gw, nil, nil, solveErr) + require.NotNil(t, rh, "NewResultHandle must accept a *errdefs.SolveError") + defer rh.Done() + + var err error + require.NotPanics(t, func() { + _, err = rh.NewContainer(ctx, &cfg) + }, "ResultHandle.NewContainer must not panic") + return gw.captured, err + } + + t.Run("returns_an_error_when", func(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + inputIDs []string + mountIDs []string + cfg build.InvokeConfig + }{ + { + name: "mount_ids_are_missing", + mountIDs: nil, + cfg: build.InvokeConfig{}, + }, + { + name: "input_ids_are_missing_and_initial_is_true", + inputIDs: nil, + cfg: build.InvokeConfig{Initial: true}, + }, + { + name: "mount_ids_are_shorter_than_declared_mounts", + inputIDs: []string{"input-0"}, + mountIDs: []string{"mount-0"}, + cfg: build.InvokeConfig{}, + }, + { + name: "input_ids_are_shorter_than_declared_mounts_and_initial_is_true", + inputIDs: []string{"input-0"}, + mountIDs: []string{"mount-0"}, + cfg: build.InvokeConfig{Initial: true}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := run(t, tc.cfg, tc.inputIDs, tc.mountIDs) + require.Error(t, err) + }) + } + }) + + t.Run("forwards_per_mount_result_ids_to_the_gateway", func(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + cfg build.InvokeConfig + wantResultIDs []string + }{ + { + name: "from_mount_ids", + cfg: build.InvokeConfig{}, + wantResultIDs: []string{"mount-0", "mount-1"}, + }, + { + name: "from_input_ids_when_initial_is_true", + cfg: build.InvokeConfig{Initial: true}, + wantResultIDs: []string{"input-0", "input-1"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + captured, err := run(t, tc.cfg, + []string{"input-0", "input-1"}, + []string{"mount-0", "mount-1"}, + ) + require.NoError(t, err) + require.NotNil(t, captured, "gateway.NewContainer should have been called") + require.Len(t, captured.Mounts, len(tc.wantResultIDs)) + for i, want := range tc.wantResultIDs { + require.Equal(t, want, captured.Mounts[i].ResultID, "mount %d ResultID", i) + require.Equal(t, mounts[i].Dest, captured.Mounts[i].Dest, "mount %d Dest", i) + } + }) + } + }) +} + +type stubGwClient struct { + gateway.Client + captured *gateway.NewContainerRequest +} + +func (s *stubGwClient) NewContainer(_ context.Context, req gateway.NewContainerRequest) (gateway.Container, error) { + s.captured = &req + + return nil, nil +} diff --git a/tests/debug.go b/tests/debug.go new file mode 100644 index 000000000000..e2768532898c --- /dev/null +++ b/tests/debug.go @@ -0,0 +1,131 @@ +package tests + +import ( + "bytes" + "io" + "os/exec" + "strings" + "sync" + "testing" + "time" + + "github.com/containerd/continuity/fs/fstest" + "github.com/creack/pty" + "github.com/moby/buildkit/util/testutil/integration" + "github.com/stretchr/testify/require" +) + +var debugTests = []func(t *testing.T, sb integration.Sandbox){ + testDebugBuildMissingBindMountSource, +} + +func testDebugBuildMissingBindMountSource(t *testing.T, sb integration.Sandbox) { + if !isExperimental() { + t.Skip("debug command is experimental") + } + if !isDockerWorker(sb) { + t.Skip("debug monitor test only needs a docker worker") + } + + dockerfile := []byte(` +FROM busybox:latest +RUN --mount=type=bind,source=missing,target=/src true +`) + dir := tmpdir(t, + fstest.CreateFile("Dockerfile", dockerfile, 0o600), + ) + + cmd := buildxCmd(sb, withArgs("debug", "--on=error", "build", "--progress=plain", "--output=type=cacheonly", dir)) + f, err := pty.StartWithSize(cmd, &pty.Winsize{ + Cols: 120, + Rows: 24, + }) + require.NoError(t, err) + defer f.Close() + + var output debugOutput + copyDone := make(chan struct{}) + go func() { + _, _ = io.Copy(&output, f) + close(copyDone) + }() + + waitCh := make(chan error, 1) + go func() { + waitCh <- cmd.Wait() + }() + + exited, waitErr := waitForDebugPromptOrExit(t, &output, waitCh) + if !exited { + _, err = f.Write([]byte("exit\r")) + require.NoError(t, err) + waitErr = waitForDebugCommandExit(t, cmd, waitCh, &output) + } + + _ = f.Close() + select { + case <-copyDone: + case <-time.After(time.Second): + } + + out := output.String() + require.Error(t, waitErr, out) + require.Contains(t, out, "failed to calculate checksum") + require.Contains(t, out, "failed to create container") + require.NotContains(t, out, "panic:") + require.NotContains(t, out, "index out of range") +} + +func waitForDebugPromptOrExit(t *testing.T, output *debugOutput, waitCh <-chan error) (bool, error) { + t.Helper() + + timeout := time.NewTimer(time.Minute) + defer timeout.Stop() + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + + for { + select { + case err := <-waitCh: + return true, err + case <-ticker.C: + if strings.Contains(output.String(), "(buildx) ") { + return false, nil + } + case <-timeout.C: + require.FailNow(t, "timeout waiting for debug monitor", output.String()) + } + } +} + +func waitForDebugCommandExit(t *testing.T, cmd *exec.Cmd, waitCh <-chan error, output *debugOutput) error { + t.Helper() + + select { + case err := <-waitCh: + return err + case <-time.After(30 * time.Second): + if cmd.Process != nil { + _ = cmd.Process.Kill() + } + require.FailNow(t, "timeout waiting for debug command to exit", output.String()) + return nil + } +} + +type debugOutput struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *debugOutput) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *debugOutput) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} diff --git a/tests/integration_test.go b/tests/integration_test.go index 574251dc5075..d2bcfec2c000 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -24,6 +24,7 @@ func TestIntegration(t *testing.T) { var tests []func(t *testing.T, sb integration.Sandbox) tests = append(tests, commonTests...) tests = append(tests, buildTests...) + tests = append(tests, debugTests...) tests = append(tests, policyBuildTests...) tests = append(tests, policyEvalTests...) tests = append(tests, policyBakeTests...)