diff --git a/CHANGELOG.md b/CHANGELOG.md index 0669508cfa..348162d8e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ All notable changes to `src-cli` are documented in this file. ### Added +- Mounting files now works when running batch changes server side. [sourcegraph/src-cli#816](https://github.com/sourcegraph/src-cli/pull/816) + ### Changed ### Fixed @@ -857,4 +859,4 @@ Re-release of 3.29.3 for Sourcegraph 3.30. ### Changed - The terminal UI has been replaced by the logger-based UI that was previously only visible in verbose-mode (`-v`). [#228](https://github.com/sourcegraph/src-cli/pull/228) -- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. [#235](https://github.com/sourcegraph/src-cli/pull/235) \ No newline at end of file +- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. [#235](https://github.com/sourcegraph/src-cli/pull/235) diff --git a/cmd/src/batch_common.go b/cmd/src/batch_common.go index a9706d1fba..8bfee5a05e 100644 --- a/cmd/src/batch_common.go +++ b/cmd/src/batch_common.go @@ -304,7 +304,7 @@ func executeBatchSpec(ctx context.Context, ui ui.ExecUI, opts executeBatchSpecOp // Parse flags and build up our service and executor options. ui.ParsingBatchSpec() - batchSpec, rawSpec, err := parseBatchSpec(ctx, opts.file, svc, false) + batchSpec, batchSpecDir, rawSpec, err := parseBatchSpec(ctx, opts.file, svc) if err != nil { var multiErr errors.MultiError if errors.As(err, &multiErr) { @@ -378,15 +378,14 @@ func executeBatchSpec(ctx context.Context, ui ui.ExecUI, opts executeBatchSpecOp Creator: workspaceCreator, EnsureImage: imageCache.Ensure, Parallelism: parallelism, + WorkingDirectory: batchSpecDir, Timeout: opts.flags.timeout, TempDir: opts.flags.tempDir, GlobalEnv: os.Environ(), - IsRemote: false, }, Logger: logManager, Cache: executor.NewDiskCache(opts.flags.cacheDir), GlobalEnv: os.Environ(), - IsRemote: false, }, ) @@ -508,13 +507,10 @@ func setReadDeadlineOnCancel(ctx context.Context, f *os.File) { // parseBatchSpec parses and validates the given batch spec. If the spec has // validation errors, they are returned. -// -// isRemote argument is a temporary argument used to determine if the batch spec is being parsed for remote -// (server-side) processing. Remote processing does not support mounts yet. -func parseBatchSpec(ctx context.Context, file string, svc *service.Service, isRemote bool) (*batcheslib.BatchSpec, string, error) { +func parseBatchSpec(ctx context.Context, file string, svc *service.Service) (*batcheslib.BatchSpec, string, string, error) { f, err := batchOpenFileFlag(file) if err != nil { - return nil, "", err + return nil, "", "", err } defer f.Close() @@ -526,26 +522,35 @@ func parseBatchSpec(ctx context.Context, file string, svc *service.Service, isRe data, err := io.ReadAll(f) if err != nil { - return nil, "", errors.Wrap(err, "reading batch spec") + return nil, "", "", errors.Wrap(err, "reading batch spec") } + dir, err := getBatchSpecDirectory(file) + if err != nil { + return nil, "", "", errors.Wrap(err, "batch spec path") + } + + spec, err := svc.ParseBatchSpec(dir, data) + return spec, dir, string(data), err +} + +func getBatchSpecDirectory(file string) (string, error) { var workingDirectory string + var err error // if the batch spec is being provided via standard input, set the working directory to the current directory if file == "" || file == "-" { workingDirectory, err = os.Getwd() if err != nil { - return nil, "", errors.Wrap(err, "batch spec path") + return "", errors.Wrap(err, "batch spec path") } } else { p, err := filepath.Abs(file) if err != nil { - return nil, "", errors.Wrap(err, "batch spec path") + return "", errors.Wrap(err, "batch spec path") } workingDirectory = filepath.Dir(p) } - - spec, err := svc.ParseBatchSpec(workingDirectory, data, isRemote) - return spec, string(data), err + return workingDirectory, nil } func checkExecutable(cmd string, args ...string) error { diff --git a/cmd/src/batch_exec.go b/cmd/src/batch_exec.go index da58dc3ed8..76cb2bbbe2 100644 --- a/cmd/src/batch_exec.go +++ b/cmd/src/batch_exec.go @@ -10,16 +10,17 @@ import ( "path/filepath" "time" + "github.com/sourcegraph/src-cli/internal/batches/docker" + "github.com/sourcegraph/src-cli/internal/batches/log" + "github.com/sourcegraph/src-cli/internal/batches/repozip" + "github.com/sourcegraph/src-cli/internal/batches/workspace" + "github.com/sourcegraph/sourcegraph/lib/errors" - "github.com/sourcegraph/src-cli/internal/batches/docker" "github.com/sourcegraph/src-cli/internal/batches/executor" "github.com/sourcegraph/src-cli/internal/batches/graphql" - "github.com/sourcegraph/src-cli/internal/batches/log" - "github.com/sourcegraph/src-cli/internal/batches/repozip" "github.com/sourcegraph/src-cli/internal/batches/service" "github.com/sourcegraph/src-cli/internal/batches/ui" - "github.com/sourcegraph/src-cli/internal/batches/workspace" "github.com/sourcegraph/src-cli/internal/cmderrors" batcheslib "github.com/sourcegraph/sourcegraph/lib/batches" @@ -30,10 +31,11 @@ const ( ) type executorModeFlags struct { - timeout time.Duration - file string - tempDir string - repoDir string + timeout time.Duration + file string + tempDir string + repoDir string + workspaceFilesDir string } func newExecutorModeFlags(flagSet *flag.FlagSet) (f *executorModeFlags) { @@ -42,6 +44,7 @@ func newExecutorModeFlags(flagSet *flag.FlagSet) (f *executorModeFlags) { flagSet.StringVar(&f.file, "f", "", "The workspace execution input file to read.") flagSet.StringVar(&f.tempDir, "tmp", "", "Directory for storing temporary data.") flagSet.StringVar(&f.repoDir, "repo", "", "Path of the checked out repo on disk.") + flagSet.StringVar(&f.workspaceFilesDir, "workspaceFiles", "", "Path of workspace files on disk.") return f } @@ -69,7 +72,7 @@ github.com/sourcegraph/sourcegraph/lib/batches. Usage: - src batch exec -f FILE -repo DIR [command options] + src batch exec -f FILE -repo DIR -workspaceFiles DIR [command options] Examples: @@ -140,6 +143,12 @@ func executeBatchSpecInWorkspaces(ctx context.Context, flags *executorModeFlags) } } + // Grab the absolute path to the workspace files contents. + workspaceFilesDir, err := filepath.Abs(flags.workspaceFilesDir) + if err != nil { + return errors.Wrap(err, "getting absolute path for workspace files dir") + } + // Test if git is available. if err := checkExecutable("git", "version"); err != nil { return err @@ -177,8 +186,7 @@ func executeBatchSpecInWorkspaces(ctx context.Context, flags *executorModeFlags) ui.PreparingContainerImagesSuccess() // Empty for now until we support secrets or env var settings in SSBC. - globalEnv := []string{} - isRemote := true + var globalEnv []string // Set up the execution UI. taskExecUI := ui.ExecutingTasks(false, 1) @@ -191,19 +199,18 @@ func executeBatchSpecInWorkspaces(ctx context.Context, flags *executorModeFlags) EnsureImage: imageCache.Ensure, Task: task, // TODO: Should be slightly less than the executor timeout. Can we somehow read that? - Timeout: flags.timeout, - TempDir: tempDir, - GlobalEnv: globalEnv, - // Temporarily prevent the ability to sending a batch spec with a mount for server-side processing. - IsRemote: isRemote, - RepoArchive: &repozip.NoopArchive{}, - UI: taskExecUI.StepsExecutionUI(task), + Timeout: flags.timeout, + TempDir: tempDir, + WorkingDirectory: workspaceFilesDir, + GlobalEnv: globalEnv, + RepoArchive: &repozip.NoopArchive{}, + UI: taskExecUI.StepsExecutionUI(task), } results, err := executor.RunSteps(ctx, opts) // Write all step cache results for all results. for _, stepRes := range results { - cacheKey := task.CacheKey(globalEnv, isRemote, stepRes.StepIndex) + cacheKey := task.CacheKey(globalEnv, workspaceFilesDir, stepRes.StepIndex) k, err := cacheKey.Key() if err != nil { return errors.Wrap(err, "calculating step cache key") diff --git a/cmd/src/batch_remote.go b/cmd/src/batch_remote.go index 25768f8052..f83df8f6bb 100644 --- a/cmd/src/batch_remote.go +++ b/cmd/src/batch_remote.go @@ -63,7 +63,7 @@ Examples: // may as well validate it at the same time so we don't even have to go to // the backend if it's invalid. ui.ParsingBatchSpec() - spec, raw, err := parseBatchSpec(ctx, file, svc, true) + spec, batchSpecDir, raw, err := parseBatchSpec(ctx, file, svc) if err != nil { ui.ParsingBatchSpecFailure(err) return err @@ -100,6 +100,21 @@ Examples: } ui.SendingBatchSpecSuccess() + hasWorkspaceFiles := false + for _, step := range spec.Steps { + if len(step.Mount) > 0 { + hasWorkspaceFiles = true + break + } + } + if hasWorkspaceFiles { + ui.UploadingWorkspaceFiles() + if err = svc.UploadBatchSpecWorkspaceFiles(ctx, batchSpecDir, batchSpecID, spec.Steps); err != nil { + return err + } + ui.UploadingWorkspaceFilesSuccess() + } + // Wait for the workspaces to be resolved. ui.ResolvingWorkspaces() ticker := time.NewTicker(1 * time.Second) diff --git a/cmd/src/batch_repositories.go b/cmd/src/batch_repositories.go index 31cbbf6d8c..b3b96d5891 100644 --- a/cmd/src/batch_repositories.go +++ b/cmd/src/batch_repositories.go @@ -74,7 +74,7 @@ Examples: } out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) - spec, _, err := parseBatchSpec(ctx, file, svc, false) + spec, _, _, err := parseBatchSpec(ctx, file, svc) if err != nil { ui := &ui.TUI{Out: out} ui.ParsingBatchSpecFailure(err) diff --git a/cmd/src/batch_validate.go b/cmd/src/batch_validate.go index a9571348e9..e83ac3f553 100644 --- a/cmd/src/batch_validate.go +++ b/cmd/src/batch_validate.go @@ -73,7 +73,7 @@ Examples: return err } - if _, _, err := parseBatchSpec(ctx, file, svc, false); err != nil { + if _, _, _, err := parseBatchSpec(ctx, file, svc); err != nil { ui.ParsingBatchSpecFailure(err) return err } diff --git a/go.mod b/go.mod index 5679c7ee98..9dd450cae6 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/sourcegraph/go-diff v0.6.1 github.com/sourcegraph/jsonx v0.0.0-20200629203448-1a936bd500cf github.com/sourcegraph/scip v0.2.1 - github.com/sourcegraph/sourcegraph/lib v0.0.0-20220825181731-397a768a5290 + github.com/sourcegraph/sourcegraph/lib v0.0.0-20221004162410-237d5855fa13 github.com/stretchr/testify v1.8.0 golang.org/x/net v0.0.0-20220722155237-a158d28d115b golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 @@ -105,7 +105,7 @@ require ( go.uber.org/zap v1.23.0 // indirect golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 // indirect golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect - golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 // indirect + golang.org/x/sys v0.0.0-20220928140112-f11e5e49a4ec // indirect golang.org/x/term v0.0.0-20220411215600-e5f449aeb171 // indirect golang.org/x/text v0.3.7 // indirect golang.org/x/tools v0.1.12 // indirect diff --git a/go.sum b/go.sum index 9a7f9524d9..fbc09ca2a3 100644 --- a/go.sum +++ b/go.sum @@ -373,8 +373,8 @@ github.com/sourcegraph/log v0.0.0-20220901143117-fc0516a694c9 h1:JjFyvx9hCD5+Jpu github.com/sourcegraph/log v0.0.0-20220901143117-fc0516a694c9/go.mod h1:UxiwB6C3xk3xOySJpW1R0MDUyfGuJRFS5Z8C+SA5p2I= github.com/sourcegraph/scip v0.2.1 h1:t4cTR8TU81Psfmcj3ZYer67JFzsR/Sc7w5rsuckRo1Y= github.com/sourcegraph/scip v0.2.1/go.mod h1:EYyT39nXdZDNVmgbJAlyIVWbEb1txnAOKpJPSYpvgXk= -github.com/sourcegraph/sourcegraph/lib v0.0.0-20220825181731-397a768a5290 h1:SLCu3Rf1eLZ4sNKl0Bg1oURTgDxEutRCaTQt5dpVqH4= -github.com/sourcegraph/sourcegraph/lib v0.0.0-20220825181731-397a768a5290/go.mod h1:9wnFUNfpORLAOJn4XAO7ZeWnYkf6/CxlWaTU1vlpuKc= +github.com/sourcegraph/sourcegraph/lib v0.0.0-20221004162410-237d5855fa13 h1:HEj9QVz35nIU4plPb7iJSFjcTA14zVnfAdfYZF5jsKg= +github.com/sourcegraph/sourcegraph/lib v0.0.0-20221004162410-237d5855fa13/go.mod h1:e3v3msUOgthv/QCWUzhFdfR3+hwUSn37rtuw49IVJg4= github.com/sourcegraph/yaml v1.0.1-0.20200714132230-56936252f152 h1:z/MpntplPaW6QW95pzcAR/72Z5TWDyDnSo0EOcyij9o= github.com/sourcegraph/yaml v1.0.1-0.20200714132230-56936252f152/go.mod h1:GIjDIg/heH5DOkXY3YJ/wNhfHsQHoXGjl8G8amsYQ1I= github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ= @@ -540,8 +540,8 @@ golang.org/x/sys v0.0.0-20211117180635-dee7805ff2e1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211213223007-03aa0b5f6827/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 h1:v6hYoSR9T5oet+pMXwUWkbiVqx/63mlHjefrHmxwfeY= -golang.org/x/sys v0.0.0-20220829200755-d48e67d00261/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220928140112-f11e5e49a4ec h1:BkDtF2Ih9xZ7le9ndzTA7KJow28VbQW3odyk/8drmuI= +golang.org/x/sys v0.0.0-20220928140112-f11e5e49a4ec/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20220411215600-e5f449aeb171 h1:EH1Deb8WZJ0xc0WK//leUHXcX9aLE5SymusoTmMZye8= golang.org/x/term v0.0.0-20220411215600-e5f449aeb171/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= diff --git a/internal/batches/executor/coordinator.go b/internal/batches/executor/coordinator.go index 77441f3717..d1c16e64b6 100644 --- a/internal/batches/executor/coordinator.go +++ b/internal/batches/executor/coordinator.go @@ -67,7 +67,7 @@ func (c *Coordinator) CheckCache(ctx context.Context, batchSpec *batcheslib.Batc func (c *Coordinator) ClearCache(ctx context.Context, tasks []*Task) error { for _, task := range tasks { for i := len(task.Steps) - 1; i > -1; i-- { - key := task.CacheKey(c.opts.GlobalEnv, c.opts.IsRemote, i) + key := task.CacheKey(c.opts.GlobalEnv, c.opts.ExecOpts.WorkingDirectory, i) if err := c.opts.Cache.Clear(ctx, key); err != nil { return errors.Wrapf(err, "clearing cache for step %d in %q", i, task.Repository.Name) } @@ -100,7 +100,7 @@ func (c *Coordinator) checkCacheForTask(ctx context.Context, batchSpec *batchesl return specs, false, nil } -func (c Coordinator) buildChangesetSpecs(task *Task, batchSpec *batcheslib.BatchSpec, result execution.AfterStepResult) ([]*batcheslib.ChangesetSpec, error) { +func (c *Coordinator) buildChangesetSpecs(task *Task, batchSpec *batcheslib.BatchSpec, result execution.AfterStepResult) ([]*batcheslib.ChangesetSpec, error) { input := &batcheslib.ChangesetSpecInput{ Repository: batcheslib.Repository{ ID: task.Repository.ID, @@ -128,7 +128,7 @@ func (c *Coordinator) loadCachedStepResults(ctx context.Context, task *Task, glo // We start at the back so that we can find the _last_ cached step, // then restart execution on the following step. for i := len(task.Steps) - 1; i > -1; i-- { - key := task.CacheKey(globalEnv, c.opts.IsRemote, i) + key := task.CacheKey(globalEnv, c.opts.ExecOpts.WorkingDirectory, i) result, found, err := c.opts.Cache.Get(ctx, key) if err != nil { @@ -181,7 +181,7 @@ func (c *Coordinator) ExecuteAndBuildSpecs(ctx context.Context, batchSpec *batch // Write all step cache results to the cache. for _, res := range results { for _, stepRes := range res.stepResults { - cacheKey := res.task.CacheKey(c.opts.GlobalEnv, c.opts.IsRemote, stepRes.StepIndex) + cacheKey := res.task.CacheKey(c.opts.GlobalEnv, c.opts.ExecOpts.WorkingDirectory, stepRes.StepIndex) if err := c.opts.Cache.Set(ctx, cacheKey, stepRes); err != nil { return nil, nil, errors.Wrapf(err, "caching result for step %d", stepRes.StepIndex) } diff --git a/internal/batches/executor/executor.go b/internal/batches/executor/executor.go index 069727e82d..463f46f8c0 100644 --- a/internal/batches/executor/executor.go +++ b/internal/batches/executor/executor.go @@ -62,11 +62,12 @@ type NewExecutorOpts struct { Logger log.LogManager // Config - Parallelism int - Timeout time.Duration - TempDir string - IsRemote bool - GlobalEnv []string + Parallelism int + Timeout time.Duration + WorkingDirectory string + TempDir string + IsRemote bool + GlobalEnv []string } type executor struct { @@ -168,15 +169,15 @@ func (x *executor) do(ctx context.Context, task *Task, ui TaskExecutionUI) (err // Actually execute the steps. opts := &RunStepsOpts{ - Task: task, - Logger: l, - WC: x.opts.Creator, - EnsureImage: x.opts.EnsureImage, - TempDir: x.opts.TempDir, - IsRemote: x.opts.IsRemote, - GlobalEnv: x.opts.GlobalEnv, - Timeout: x.opts.Timeout, - RepoArchive: repoArchive, + Task: task, + Logger: l, + WC: x.opts.Creator, + EnsureImage: x.opts.EnsureImage, + TempDir: x.opts.TempDir, + GlobalEnv: x.opts.GlobalEnv, + Timeout: x.opts.Timeout, + RepoArchive: repoArchive, + WorkingDirectory: x.opts.WorkingDirectory, UI: ui.StepsExecutionUI(task), } diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go index 40f704a13b..7804793e32 100644 --- a/internal/batches/executor/run_steps.go +++ b/internal/batches/executor/run_steps.go @@ -8,6 +8,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "strings" "time" @@ -38,6 +39,8 @@ type RunStepsOpts struct { Task *Task // TempDir points to where temporary files of the execution should live at. TempDir string + // WorkingDirectory points to where the workspace files should live at. + WorkingDirectory string // Timeout sets the deadline for the execution context. When exceeded, // execution will stop and an error is returned. Timeout time.Duration @@ -45,9 +48,6 @@ type RunStepsOpts struct { RepoArchive repozip.Archive Logger log.TaskLogger UI StepsExecutionUI - // IsRemote toggles server-side execution mode. This disables file mounts using - // step.mounts. - IsRemote bool // GlobalEnv is the os.Environ() for the execution. We don't read from os.Environ() // directly to allow injecting variables and hiding others. GlobalEnv []string @@ -321,12 +321,13 @@ func executeSingleStep( args = append(args, "--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", source.Name(), target)) } - // Temporarily add a guard to prevent a path to mount path for server-side processing. - if !opts.IsRemote { - // Mount any paths on the local system to the docker container. The paths have already been validated during parsing. - for _, mount := range step.Mount { - args = append(args, "--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", mount.Path, mount.Mountpoint)) + // Mount any paths on the local system to the docker container. The paths have already been validated during parsing. + for _, mount := range step.Mount { + workspaceFilePath, err := getAbsoluteMountPath(opts.WorkingDirectory, mount.Path) + if err != nil { + return bytes.Buffer{}, bytes.Buffer{}, err } + args = append(args, "--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", workspaceFilePath, mount.Mountpoint)) } for k, v := range env { @@ -591,6 +592,29 @@ func createCidFile(ctx context.Context, tempDir string, repoSlug string) (string return cidFile.Name(), cleanup, nil } +func getAbsoluteMountPath(batchSpecDir string, mountPath string) (string, error) { + p := mountPath + if !filepath.IsAbs(p) { + // Try to build the absolute path since Docker will only mount absolute paths + p = filepath.Join(batchSpecDir, p) + } + pathInfo, err := os.Stat(p) + if os.IsNotExist(err) { + return "", errors.Newf("mount path %s does not exist", p) + } else if err != nil { + return "", errors.Wrap(err, "mount path validation") + } + if !strings.HasPrefix(p, batchSpecDir) { + return "", errors.Newf("mount path %s is not in the same directory or subdirectory as the batch spec", mountPath) + } + // Mounting a directory on Docker must end with the separator. So, append the file separator to make + // users' lives easier. + if pathInfo.IsDir() && !strings.HasSuffix(p, string(filepath.Separator)) { + p += string(filepath.Separator) + } + return p, nil +} + type stepFailedErr struct { Run string Container string diff --git a/internal/batches/executor/task.go b/internal/batches/executor/task.go index aba8779004..ee7f8ad55e 100644 --- a/internal/batches/executor/task.go +++ b/internal/batches/executor/task.go @@ -38,12 +38,7 @@ func (t *Task) ArchivePathToFetch() string { return "" } -func (t *Task) CacheKey(globalEnv []string, isRemote bool, stepIndex int) cache.Keyer { - var metadataRetriever cache.MetadataRetriever - // If the task is being run locally, set the metadata retrieve to use the filesystem based implementation. - if !isRemote { - metadataRetriever = fileMetadataRetriever{} - } +func (t *Task) CacheKey(globalEnv []string, workingDir string, stepIndex int) cache.Keyer { return &cache.CacheKey{ Repository: batcheslib.Repository{ ID: t.Repository.ID, @@ -56,8 +51,7 @@ func (t *Task) CacheKey(globalEnv []string, isRemote bool, stepIndex int) cache. OnlyFetchWorkspace: t.OnlyFetchWorkspace, Steps: t.Steps, BatchChangeAttributes: t.BatchChangeAttributes, - // TODO: This should be cached. - MetadataRetriever: metadataRetriever, + MetadataRetriever: fileMetadataRetriever{workingDirectory: workingDir}, GlobalEnv: globalEnv, @@ -65,14 +59,16 @@ func (t *Task) CacheKey(globalEnv []string, isRemote bool, stepIndex int) cache. } } -type fileMetadataRetriever struct{} +type fileMetadataRetriever struct { + workingDirectory string +} func (f fileMetadataRetriever) Get(steps []batcheslib.Step) ([]cache.MountMetadata, error) { var mountsMetadata []cache.MountMetadata for _, step := range steps { // Build up the metadata for each mount for each step for _, mount := range step.Mount { - metadata, err := getMountMetadata(mount.Path) + metadata, err := f.getMountMetadata(f.workingDirectory, mount.Path) if err != nil { return nil, err } @@ -83,8 +79,12 @@ func (f fileMetadataRetriever) Get(steps []batcheslib.Step) ([]cache.MountMetada return mountsMetadata, nil } -func getMountMetadata(path string) ([]cache.MountMetadata, error) { - info, err := os.Stat(path) +func (f fileMetadataRetriever) getMountMetadata(baseDir string, path string) ([]cache.MountMetadata, error) { + fullPath := path + if !filepath.IsAbs(path) { + fullPath = filepath.Join(baseDir, path) + } + info, err := os.Stat(fullPath) if errors.Is(err, os.ErrNotExist) { return nil, errors.Newf("path %s does not exist", path) } else if err != nil { @@ -92,30 +92,33 @@ func getMountMetadata(path string) ([]cache.MountMetadata, error) { } var metadata []cache.MountMetadata if info.IsDir() { - dirMetadata, err := getDirectoryMountMetadata(path) + dirMetadata, err := f.getDirectoryMountMetadata(fullPath) if err != nil { return nil, err } metadata = append(metadata, dirMetadata...) } else { - metadata = append(metadata, cache.MountMetadata{Path: path, Size: info.Size(), Modified: info.ModTime().UTC()}) + relativePath, err := filepath.Rel(f.workingDirectory, fullPath) + if err != nil { + return nil, err + } + metadata = append(metadata, cache.MountMetadata{Path: relativePath, Size: info.Size(), Modified: info.ModTime().UTC()}) } return metadata, nil } // getDirectoryMountMetadata reads all the files in the directory with the given // path and returns the cache.MountMetadata for all of them. -func getDirectoryMountMetadata(path string) ([]cache.MountMetadata, error) { +func (f fileMetadataRetriever) getDirectoryMountMetadata(path string) ([]cache.MountMetadata, error) { dir, err := os.ReadDir(path) if err != nil { return nil, err } var metadata []cache.MountMetadata for _, dirEntry := range dir { - newPath := filepath.Join(path, dirEntry.Name()) // Go back to the very start. Need to get the FileInfo again for the new path and figure out if it is a // directory or a file. - fileMetadata, err := getMountMetadata(newPath) + fileMetadata, err := f.getMountMetadata(path, dirEntry.Name()) if err != nil { return nil, err } diff --git a/internal/batches/executor/task_test.go b/internal/batches/executor/task_test.go index 423eff5fc7..de7b9a6e3e 100644 --- a/internal/batches/executor/task_test.go +++ b/internal/batches/executor/task_test.go @@ -39,7 +39,9 @@ func TestFileMetadataRetriever_Get(t *testing.T) { err = os.Chtimes(anotherScriptPath, modDate, modDate) require.NoError(t, err) - retriever := fileMetadataRetriever{} + retriever := fileMetadataRetriever{ + workingDirectory: tempDir, + } tests := []struct { name string @@ -53,13 +55,13 @@ func TestFileMetadataRetriever_Get(t *testing.T) { { Run: "foo", Mount: []batches.Mount{{ - Path: sampleScriptPath, + Path: "./sample.sh", Mountpoint: "/tmp/foo.sh", }}, }, }, expectedMetadata: []cache.MountMetadata{ - {Path: sampleScriptPath, Size: 0, Modified: modDate}, + {Path: "sample.sh", Size: 0, Modified: modDate}, }, }, { @@ -69,19 +71,19 @@ func TestFileMetadataRetriever_Get(t *testing.T) { Run: "foo", Mount: []batches.Mount{ { - Path: sampleScriptPath, + Path: "./sample.sh", Mountpoint: "/tmp/foo.sh", }, { - Path: anotherScriptPath, + Path: "./another.sh", Mountpoint: "/tmp/bar.sh", }, }, }, }, expectedMetadata: []cache.MountMetadata{ - {Path: sampleScriptPath, Size: 0, Modified: modDate}, - {Path: anotherScriptPath, Size: 0, Modified: modDate}, + {Path: "sample.sh", Size: 0, Modified: modDate}, + {Path: "another.sh", Size: 0, Modified: modDate}, }, }, { @@ -90,14 +92,14 @@ func TestFileMetadataRetriever_Get(t *testing.T) { { Run: "foo", Mount: []batches.Mount{{ - Path: tempDir, + Path: "./", Mountpoint: "/tmp/scripts", }}, }, }, expectedMetadata: []cache.MountMetadata{ - {Path: anotherScriptPath, Size: 0, Modified: modDate}, - {Path: sampleScriptPath, Size: 0, Modified: modDate}, + {Path: "another.sh", Size: 0, Modified: modDate}, + {Path: "sample.sh", Size: 0, Modified: modDate}, }, }, { @@ -119,21 +121,21 @@ func TestFileMetadataRetriever_Get(t *testing.T) { { Run: "foo", Mount: []batches.Mount{{ - Path: sampleScriptPath, + Path: "./sample.sh", Mountpoint: "/tmp/foo.sh", }}, }, { Run: "foo", Mount: []batches.Mount{{ - Path: sampleScriptPath, + Path: "./sample.sh", Mountpoint: "/tmp/foo.sh", }}, }, }, expectedMetadata: []cache.MountMetadata{ - {Path: sampleScriptPath, Size: 0, Modified: modDate}, - {Path: sampleScriptPath, Size: 0, Modified: modDate}, + {Path: "sample.sh", Size: 0, Modified: modDate}, + {Path: "sample.sh", Size: 0, Modified: modDate}, }, }, } diff --git a/internal/batches/service/remote.go b/internal/batches/service/remote.go index 6d4577812b..b0492560e4 100644 --- a/internal/batches/service/remote.go +++ b/internal/batches/service/remote.go @@ -2,12 +2,18 @@ package service import ( "context" + "fmt" + "io" + "mime/multipart" + "net/http" + "os" + "path/filepath" + "strings" + "github.com/sourcegraph/sourcegraph/lib/batches" "github.com/sourcegraph/sourcegraph/lib/errors" ) -var ErrServerSideBatchChangesUnsupported = errors.New("server side batch changes are not available on this Sourcegraph instance") - const upsertEmptyBatchChangeQuery = ` mutation UpsertEmptyBatchChange( $name: String! @@ -96,6 +102,143 @@ func (svc *Service) CreateBatchSpecFromRaw( return resp.CreateBatchSpecFromRaw.ID, nil } +// UploadBatchSpecWorkspaceFiles uploads workspace files to the server. +func (svc *Service) UploadBatchSpecWorkspaceFiles(ctx context.Context, workingDir string, batchSpecID string, steps []batches.Step) error { + filePaths := make(map[string]bool) + for _, step := range steps { + for _, mount := range step.Mount { + paths, err := getFilePaths(workingDir, mount.Path) + if err != nil { + return err + } + // Dedupe any files. + for _, path := range paths { + if !filePaths[path] { + filePaths[path] = true + } + } + } + } + + for filePath := range filePaths { + if err := svc.uploadFile(ctx, workingDir, filePath, batchSpecID); err != nil { + return err + } + } + return nil +} + +func getFilePaths(workingDir, filePath string) ([]string, error) { + var filePaths []string + actualFilePath := filepath.Join(workingDir, filePath) + info, err := os.Stat(actualFilePath) + if err != nil { + return nil, err + } + + if info.IsDir() { + dir, err := os.ReadDir(actualFilePath) + if err != nil { + return nil, err + } + for _, dirEntry := range dir { + paths, err := getFilePaths(workingDir, filepath.Join(filePath, dirEntry.Name())) + if err != nil { + return nil, err + } + filePaths = append(filePaths, paths...) + } + } else { + relPath, err := filepath.Rel(workingDir, actualFilePath) + if err != nil { + return nil, err + } + filePaths = append(filePaths, relPath) + } + return filePaths, nil +} + +func (svc *Service) uploadFile(ctx context.Context, workingDir, filePath, batchSpecID string) error { + // Create a pipe so the requests can be chunked to the server + pipeReader, pipeWriter := io.Pipe() + multipartWriter := multipart.NewWriter(pipeWriter) + + // Write in a separate goroutine to properly chunk the file content. Writing to the pipe lets us not have + // to put the whole file in memory. + go func() { + defer pipeWriter.Close() + defer multipartWriter.Close() + + if err := createFormFile(multipartWriter, workingDir, filePath); err != nil { + pipeWriter.CloseWithError(err) + } + }() + + request, err := svc.client.NewHTTPRequest(ctx, http.MethodPost, fmt.Sprintf(".api/files/batch-changes/%s", batchSpecID), pipeReader) + if err != nil { + return err + } + request.Header.Set("Content-Type", multipartWriter.FormDataContentType()) + + resp, err := svc.client.Do(request) + if err != nil { + // Errors passed to pipeWriter.CloseWithError come through here. + return err + } + // 2xx and 3xx are ok + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusBadRequest { + p, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + return errors.New(string(p)) + } + return nil +} + +const maxFileSize = 10 << 20 // 10MB + +func createFormFile(w *multipart.Writer, workingDir string, mountPath string) error { + f, err := os.Open(filepath.Join(workingDir, mountPath)) + if err != nil { + return err + } + defer f.Close() + + // Limit the size of file to 10MB + fileStat, err := f.Stat() + if err != nil { + return err + } + if fileStat.Size() > maxFileSize { + return errors.New("file exceeds limit of 10MB") + } + + filePath, fileName := filepath.Split(mountPath) + filePath = strings.Trim(strings.TrimSuffix(filePath, string(filepath.Separator)), ".") + // Ensure Windows separators are changed to Unix. + filePath = strings.ReplaceAll(filePath, "\\", "/") + if err = w.WriteField("filepath", filePath); err != nil { + return err + } + fileInfo, err := f.Stat() + if err != nil { + return err + } + if err = w.WriteField("filemod", fileInfo.ModTime().UTC().String()); err != nil { + return err + } + + part, err := w.CreateFormFile("file", fileName) + if err != nil { + return err + } + if _, err = io.Copy(part, f); err != nil { + return err + } + return nil +} + const executeBatchSpecQuery = ` mutation ExecuteBatchSpec($batchSpec: ID!, $noCache: Boolean!) { executeBatchSpec(batchSpec: $batchSpec, noCache: $noCache) { diff --git a/internal/batches/service/remote_test.go b/internal/batches/service/remote_test.go index 6cbfecaa2b..563a862383 100644 --- a/internal/batches/service/remote_test.go +++ b/internal/batches/service/remote_test.go @@ -1,15 +1,26 @@ package service_test import ( + "bytes" "context" "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "strings" "testing" mockclient "github.com/sourcegraph/src-cli/internal/api/mock" "github.com/sourcegraph/src-cli/internal/batches/service" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/sourcegraph/sourcegraph/lib/batches" "github.com/sourcegraph/sourcegraph/lib/errors" ) @@ -191,3 +202,518 @@ func TestService_CreateBatchSpecFromRaw(t *testing.T) { }) } } + +func TestService_UploadBatchSpecWorkspaceFiles(t *testing.T) { + tests := []struct { + name string + steps []batches.Step + + setup func(workingDir string) error + mockInvokes func(client *mockclient.Client) + + expectedError error + }{ + { + name: "Upload single file", + steps: []batches.Step{{ + Mount: []batches.Mount{{ + Path: "./hello.txt", + }}, + }}, + setup: func(workingDir string) error { + return writeTempFile(workingDir, "hello.txt", "hello world!") + }, + mockInvokes: func(client *mockclient.Client) { + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Once() + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + entry := &multipartFormEntry{ + fileName: "hello.txt", + content: "hello world!", + } + requestMatcher := multipartFormRequestMatcher(entry) + client.On("Do", mock.MatchedBy(requestMatcher)). + Return(resp, nil). + Once() + }, + }, + { + name: "Deduplicate files", + steps: []batches.Step{{ + Mount: []batches.Mount{ + { + Path: "./hello.txt", + }, + { + Path: "./hello.txt", + }, + { + Path: "./hello.txt", + }, + }, + }}, + setup: func(workingDir string) error { + return writeTempFile(workingDir, "hello.txt", "hello world!") + }, + mockInvokes: func(client *mockclient.Client) { + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Once() + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + entry := &multipartFormEntry{ + fileName: "hello.txt", + content: "hello world!", + } + requestMatcher := multipartFormRequestMatcher(entry) + client.On("Do", mock.MatchedBy(requestMatcher)). + Return(resp, nil). + Once() + }, + }, + { + name: "Upload multiple files", + steps: []batches.Step{{ + Mount: []batches.Mount{ + { + Path: "./hello.txt", + }, + { + Path: "./world.txt", + }, + }, + }}, + setup: func(workingDir string) error { + if err := writeTempFile(workingDir, "hello.txt", "hello"); err != nil { + return err + } + return writeTempFile(workingDir, "world.txt", "world!") + }, + mockInvokes: func(client *mockclient.Client) { + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Twice() + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + helloEntry := &multipartFormEntry{ + fileName: "hello.txt", + content: "hello", + } + client. + On("Do", mock.MatchedBy(multipartFormRequestMatcher(helloEntry))). + Return(resp, nil). + Once() + + worldEntry := &multipartFormEntry{ + fileName: "world.txt", + content: "world!", + } + client. + On("Do", mock.MatchedBy(multipartFormRequestMatcher(worldEntry))). + Return(resp, nil). + Once() + }, + }, + { + name: "Upload directory", + steps: []batches.Step{{ + Mount: []batches.Mount{ + { + Path: "./", + }, + }, + }}, + setup: func(workingDir string) error { + if err := writeTempFile(workingDir, "hello.txt", "hello"); err != nil { + return err + } + return writeTempFile(workingDir, "world.txt", "world!") + }, + mockInvokes: func(client *mockclient.Client) { + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Twice() + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + helloEntry := &multipartFormEntry{ + fileName: "hello.txt", + content: "hello", + } + client. + On("Do", mock.MatchedBy(multipartFormRequestMatcher(helloEntry))). + Return(resp, nil). + Once() + + worldEntry := &multipartFormEntry{ + fileName: "world.txt", + content: "world!", + } + client. + On("Do", mock.MatchedBy(multipartFormRequestMatcher(worldEntry))). + Return(resp, nil). + Once() + }, + }, + { + name: "Upload subdirectory", + steps: []batches.Step{{ + Mount: []batches.Mount{ + { + Path: "./scripts", + }, + }, + }}, + setup: func(workingDir string) error { + dir := filepath.Join(workingDir, "scripts") + if err := os.Mkdir(dir, os.ModePerm); err != nil { + return err + } + return writeTempFile(dir, "hello.txt", "hello world!") + }, + mockInvokes: func(client *mockclient.Client) { + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Once() + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + entry := &multipartFormEntry{ + path: "scripts", + fileName: "hello.txt", + content: "hello world!", + } + client.On("Do", mock.MatchedBy(multipartFormRequestMatcher(entry))). + Return(resp, nil). + Once() + }, + }, + { + name: "Upload files and directory", + steps: []batches.Step{{ + Mount: []batches.Mount{ + { + Path: "./hello.txt", + }, + { + Path: "./world.txt", + }, + { + Path: "./scripts", + }, + }, + }}, + setup: func(workingDir string) error { + if err := writeTempFile(workingDir, "hello.txt", "hello"); err != nil { + return err + } + if err := writeTempFile(workingDir, "world.txt", "world!"); err != nil { + return err + } + dir := filepath.Join(workingDir, "scripts") + if err := os.Mkdir(dir, os.ModePerm); err != nil { + return err + } + return writeTempFile(dir, "something-else.txt", "this is neat") + }, + mockInvokes: func(client *mockclient.Client) { + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Times(3) + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + helloEntry := &multipartFormEntry{ + fileName: "hello.txt", + content: "hello", + } + client.On("Do", mock.MatchedBy(multipartFormRequestMatcher(helloEntry))). + Return(resp, nil). + Once() + worldEntry := &multipartFormEntry{ + fileName: "world.txt", + content: "world!", + } + client.On("Do", mock.MatchedBy(multipartFormRequestMatcher(worldEntry))). + Return(resp, nil). + Once() + somethingElseEntry := &multipartFormEntry{ + path: "scripts", + fileName: "something-else.txt", + content: "this is neat", + } + client.On("Do", mock.MatchedBy(multipartFormRequestMatcher(somethingElseEntry))). + Return(resp, nil). + Once() + }, + }, + { + name: "Bad status code", + steps: []batches.Step{{ + Mount: []batches.Mount{{ + Path: "./hello.txt", + }}, + }}, + setup: func(workingDir string) error { + return writeTempFile(workingDir, "hello.txt", "hello world!") + }, + mockInvokes: func(client *mockclient.Client) { + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Return(req, nil). + Once() + + resp := &http.Response{ + StatusCode: http.StatusInternalServerError, + Body: io.NopCloser(bytes.NewReader([]byte("failed to upload file"))), + } + client.On("Do", mock.Anything). + Return(resp, nil). + Once() + }, + expectedError: errors.New("failed to upload file"), + }, + { + name: "File exceeds limit", + steps: []batches.Step{{ + Mount: []batches.Mount{{ + Path: "./hello.txt", + }}, + }}, + setup: func(workingDir string) error { + f, err := os.Create(filepath.Join(workingDir, "hello.txt")) + if err != nil { + return err + } + defer f.Close() + if _, err = io.Copy(f, io.LimitReader(neverEnding('a'), 11<<20)); err != nil { + return err + } + return nil + }, + mockInvokes: func(client *mockclient.Client) { + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Once() + + client.On("Do", mock.Anything). + Return(nil, errors.New("file exceeds limit")). + Once() + }, + expectedError: errors.New("file exceeds limit"), + }, + { + name: "Long mount path", + steps: []batches.Step{{ + Mount: []batches.Mount{{ + Path: "foo/../bar/../baz/../hello.txt", + }}, + }}, + setup: func(workingDir string) error { + dir := filepath.Join(workingDir, "foo") + if err := os.Mkdir(dir, os.ModePerm); err != nil { + return err + } + dir = filepath.Join(workingDir, "bar") + if err := os.Mkdir(dir, os.ModePerm); err != nil { + return err + } + dir = filepath.Join(workingDir, "baz") + if err := os.Mkdir(dir, os.ModePerm); err != nil { + return err + } + return writeTempFile(workingDir, "hello.txt", "hello world!") + }, + mockInvokes: func(client *mockclient.Client) { + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Once() + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + entry := &multipartFormEntry{ + fileName: "hello.txt", + content: "hello world!", + } + requestMatcher := multipartFormRequestMatcher(entry) + client.On("Do", mock.MatchedBy(requestMatcher)). + Return(resp, nil). + Once() + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // TODO: use TempDir when https://github.com/golang/go/issues/51442 is cherry-picked into 1.18 or upgrade to 1.19+ + //tempDir := t.TempDir() + workingDir, err := os.MkdirTemp("", test.name) + require.NoError(t, err) + t.Cleanup(func() { + os.RemoveAll(workingDir) + }) + + if test.setup != nil { + err := test.setup(workingDir) + require.NoError(t, err) + } + + client := new(mockclient.Client) + svc := service.New(&service.Opts{Client: client}) + + if test.mockInvokes != nil { + test.mockInvokes(client) + } + + err = svc.UploadBatchSpecWorkspaceFiles(context.Background(), workingDir, "123", test.steps) + if test.expectedError != nil { + assert.Equal(t, test.expectedError.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + + client.AssertExpectations(t) + }) + } +} + +func writeTempFile(dir string, name string, content string) error { + f, err := os.Create(filepath.Join(dir, name)) + if err != nil { + return err + } + defer f.Close() + if _, err = io.WriteString(f, content); err != nil { + return err + } + return nil +} + +// 2006-01-02 15:04:05.999999999 -0700 MST +var modtimeRegex = regexp.MustCompile("^[0-9]{4}-[0-9]{2}-[0-9]{2}\\s[0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{1,9} \\+0000 UTC$") + +func multipartFormRequestMatcher(entry *multipartFormEntry) func(*http.Request) bool { + return func(req *http.Request) bool { + // Prevent parsing the body for the wrong matcher - causes all kinds of havoc. + if entry.calls > 0 { + return false + } + // Clone the request. Running ParseMultipartForm changes the behavior of the request for any additional + // matchers by consuming the request body. + cloneReq, err := cloneRequest(req) + if err != nil { + fmt.Printf("failed to clone request: %s\n", err) + return false + } + contentType := cloneReq.Header.Get("Content-Type") + if !strings.HasPrefix(contentType, "multipart/form-data") { + return false + } + if err := cloneReq.ParseMultipartForm(32 << 20); err != nil { + fmt.Printf("failed to parse multipartform: %s\n", err) + return false + } + if cloneReq.Form.Get("filepath") != entry.path { + return false + } + if !modtimeRegex.MatchString(cloneReq.Form.Get("filemod")) { + return false + } + f, header, err := cloneReq.FormFile("file") + if err != nil { + fmt.Printf("failed to get form file: %s\n", err) + return false + } + if header.Filename != entry.fileName { + return false + } + b, err := io.ReadAll(f) + if err != nil { + fmt.Printf("failed to read file: %s\n", err) + return false + } + if string(b) != entry.content { + return false + } + entry.calls++ + return true + } +} + +type multipartFormEntry struct { + path string + fileName string + content string + // This prevents some weird behavior that causes the request body to get read and throw errors. + calls int +} + +type neverEnding byte + +func (b neverEnding) Read(p []byte) (n int, err error) { + for i := range p { + p[i] = byte(b) + } + return len(p), nil +} + +func cloneRequest(req *http.Request) (*http.Request, error) { + clone := req.Clone(context.TODO()) + var b bytes.Buffer + if _, err := b.ReadFrom(req.Body); err != nil { + return nil, err + } + req.Body = io.NopCloser(&b) + clone.Body = io.NopCloser(bytes.NewReader(b.Bytes())) + return clone, nil +} diff --git a/internal/batches/service/remote_windows_test.go b/internal/batches/service/remote_windows_test.go new file mode 100644 index 0000000000..f8e34a8be4 --- /dev/null +++ b/internal/batches/service/remote_windows_test.go @@ -0,0 +1,76 @@ +package service_test + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + mockclient "github.com/sourcegraph/src-cli/internal/api/mock" + "github.com/sourcegraph/src-cli/internal/batches/service" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/sourcegraph/sourcegraph/lib/batches" +) + +func TestService_UploadBatchSpecWorkspaceFiles_Windows_Path(t *testing.T) { + // TODO: use TempDir when https://github.com/golang/go/issues/51442 is cherry-picked into 1.18 or upgrade to 1.19+ + //tempDir := t.TempDir() + workingDir, err := os.MkdirTemp("", "windows_path") + require.NoError(t, err) + t.Cleanup(func() { + os.RemoveAll(workingDir) + }) + + // Set up test files and directories + dir := filepath.Join(workingDir, "scripts") + err = os.Mkdir(dir, os.ModePerm) + require.NoError(t, err) + + dir = filepath.Join(dir, "another-dir") + err = os.Mkdir(dir, os.ModePerm) + require.NoError(t, err) + + err = writeTempFile(dir, "hello.txt", "hello world!") + require.NoError(t, err) + + client := new(mockclient.Client) + svc := service.New(&service.Opts{Client: client}) + + // Body will get set with the body argument to NewHTTPRequest + req := httptest.NewRequest(http.MethodPost, "http://fake.com/.api/files/batch-changes/123", nil) + client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/files/batch-changes/123", mock.Anything). + Run(func(args mock.Arguments) { + req.Body = args[3].(*io.PipeReader) + }). + Return(req, nil). + Once() + + resp := &http.Response{ + StatusCode: http.StatusOK, + } + entry := &multipartFormEntry{ + path: "scripts/another-dir", + fileName: "hello.txt", + content: "hello world!", + } + requestMatcher := multipartFormRequestMatcher(entry) + client.On("Do", mock.MatchedBy(requestMatcher)). + Return(resp, nil). + Once() + + steps := []batches.Step{{ + Mount: []batches.Mount{{ + Path: ".\\scripts\\another-dir\\hello.txt", + }}, + }} + err = svc.UploadBatchSpecWorkspaceFiles(context.Background(), workingDir, "123", steps) + assert.NoError(t, err) + + client.AssertExpectations(t) +} diff --git a/internal/batches/service/service.go b/internal/batches/service/service.go index 960de35283..4231b945dd 100644 --- a/internal/batches/service/service.go +++ b/internal/batches/service/service.go @@ -466,46 +466,33 @@ func (e *duplicateBranchesErr) Error() string { return out.String() } -func (svc *Service) ParseBatchSpec(dir string, data []byte, isRemote bool) (*batcheslib.BatchSpec, error) { +func (svc *Service) ParseBatchSpec(dir string, data []byte) (*batcheslib.BatchSpec, error) { spec, err := batcheslib.ParseBatchSpec(data) if err != nil { return nil, errors.Wrap(err, "parsing batch spec") } - if err = validateMount(dir, spec, isRemote); err != nil { + if err = validateMount(dir, spec); err != nil { return nil, errors.Wrap(err, "handling mount") } return spec, nil } -func validateMount(batchSpecDir string, spec *batcheslib.BatchSpec, isRemote bool) error { +func validateMount(batchSpecDir string, spec *batcheslib.BatchSpec) error { for i, step := range spec.Steps { - for j, mount := range step.Mount { - if isRemote { - return errors.New("mounts are not supported for server-side processing") - } - p := mount.Path - if !filepath.IsAbs(p) { + for _, mount := range step.Mount { + if !filepath.IsAbs(mount.Path) { // Try to build the absolute path since Docker will only mount absolute paths - p = filepath.Join(batchSpecDir, p) + mount.Path = filepath.Join(batchSpecDir, mount.Path) } - pathInfo, err := os.Stat(p) + _, err := os.Stat(mount.Path) if os.IsNotExist(err) { - return errors.Newf("step %d mount path %s does not exist", i+1, p) + return errors.Newf("step %d mount path %s does not exist", i+1, mount.Path) } else if err != nil { return errors.Wrapf(err, "step %d mount path validation", i+1) } - if !strings.HasPrefix(p, batchSpecDir) { + if !strings.HasPrefix(mount.Path, batchSpecDir) { return errors.Newf("step %d mount path is not in the same directory or subdirectory as the batch spec", i+1) } - // Mounting a directory on Docker must end with the separator. So, append the file separator to make - // users' lives easier. - if pathInfo.IsDir() && !strings.HasSuffix(p, string(filepath.Separator)) { - p += string(filepath.Separator) - } - // Update the mount path to the absolute path so building the absolute path (above) does not need to be - // redone when adding the mount argument to the Docker container. - // TODO: Can this mess with caching? We wouldn't be doing that server-side. - step.Mount[j].Path = p } } return nil diff --git a/internal/batches/service/service_test.go b/internal/batches/service/service_test.go index 70bde9fff8..dc5aa09c48 100644 --- a/internal/batches/service/service_test.go +++ b/internal/batches/service/service_test.go @@ -234,7 +234,6 @@ func TestService_ParseBatchSpec(t *testing.T) { name string batchSpecDir string rawSpec string - isRemote bool expectedSpec *batcheslib.BatchSpec expectedErr error }{ @@ -327,7 +326,7 @@ changesetTemplate: Container: "alpine:3", Mount: []batcheslib.Mount{ { - Path: filepath.Join(tempDir, "sample.sh"), + Path: "./sample.sh", Mountpoint: "/tmp/sample.sh", }, }, @@ -371,7 +370,7 @@ changesetTemplate: Container: "alpine:3", Mount: []batcheslib.Mount{ { - Path: tempDir + string(filepath.Separator), + Path: tempDir, Mountpoint: "/tmp", }, }, @@ -415,7 +414,7 @@ changesetTemplate: Container: "alpine:3", Mount: []batcheslib.Mount{ { - Path: tempDir + string(filepath.Separator), + Path: "./", Mountpoint: "/tmp", }, }, @@ -461,11 +460,11 @@ changesetTemplate: Container: "alpine:3", Mount: []batcheslib.Mount{ { - Path: filepath.Join(tempDir, "sample.sh"), + Path: "./sample.sh", Mountpoint: "/tmp/sample.sh", }, { - Path: filepath.Join(tempDir, "another.sh"), + Path: "./another.sh", Mountpoint: "/tmp/another.sh", }, }, @@ -523,32 +522,10 @@ changesetTemplate: `, tempOutsideDir), expectedErr: errors.New("handling mount: step 1 mount path is not in the same directory or subdirectory as the batch spec"), }, - { - name: "mount remote processing", - batchSpecDir: tempDir, - rawSpec: ` -name: test-spec -description: A test spec -steps: - - run: /tmp/foo/bar/sample.sh - container: alpine:3 - mount: - - path: /valid/sample.sh - mountpoint: /tmp/foo/bar/sample.sh -changesetTemplate: - title: Test Mount - body: Test a mounted path - branch: test - commit: - message: Test -`, - isRemote: true, - expectedErr: errors.New("handling mount: mounts are not supported for server-side processing"), - }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - spec, err := svc.ParseBatchSpec(test.batchSpecDir, []byte(test.rawSpec), test.isRemote) + spec, err := svc.ParseBatchSpec(test.batchSpecDir, []byte(test.rawSpec)) if test.expectedErr != nil { assert.Equal(t, test.expectedErr.Error(), err.Error()) } else { diff --git a/internal/batches/ui/tui.go b/internal/batches/ui/tui.go index ed3e5e3117..ce1c310027 100644 --- a/internal/batches/ui/tui.go +++ b/internal/batches/ui/tui.go @@ -6,6 +6,7 @@ import ( "os/exec" "github.com/neelance/parallel" + "github.com/sourcegraph/sourcegraph/lib/errors" "github.com/sourcegraph/sourcegraph/lib/output" @@ -231,6 +232,14 @@ func (ui *TUI) SendingBatchSpecSuccess() { batchCompletePending(ui.pending, "Sending batch spec") } +func (ui *TUI) UploadingWorkspaceFiles() { + ui.pending = batchCreatePending(ui.Out, "Uploading workspace files") +} + +func (ui *TUI) UploadingWorkspaceFilesSuccess() { + batchCompletePending(ui.pending, "Uploading workspace files") +} + func (ui *TUI) ResolvingWorkspaces() { ui.pending = batchCreatePending(ui.Out, "Resolving workspaces") }