diff --git a/CHANGELOG.md b/CHANGELOG.md index 83e0219bad..bacfd97053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ All notable changes to `src-cli` are documented in this file. ### Added +- `steps` in campaign specs can now have [`outputs`](https://docs.sourcegraph.com/campaigns/references/campaign_spec_yaml_reference#steps-outputs) that support [templating](https://docs.sourcegraph.com/campaigns/references/campaign_spec_templating). [#424](https://github.com/sourcegraph/src-cli/pull/424) +- `changesetTemplate` fields in campaign specs now also support [templating](https://docs.sourcegraph.com/campaigns/references/campaign_spec_templating). [#424](https://github.com/sourcegraph/src-cli/pull/424) + ### Changed ### Fixed diff --git a/go.mod b/go.mod index ebe9209de1..e9f3a5a216 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,7 @@ require ( golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e golang.org/x/sys v0.0.0-20200622214017-ed371f2e16b4 gopkg.in/yaml.v2 v2.3.0 + gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 jaytaylor.com/html2text v0.0.0-20200412013138-3577fbdbcff7 ) diff --git a/internal/campaigns/campaign_spec.go b/internal/campaigns/campaign_spec.go index 8d6695cd9b..0aefd0cb52 100644 --- a/internal/campaigns/campaign_spec.go +++ b/internal/campaigns/campaign_spec.go @@ -71,10 +71,18 @@ type Step struct { Container string `json:"container,omitempty" yaml:"container"` Env env.Environment `json:"env,omitempty" yaml:"env"` Files map[string]string `json:"files,omitempty" yaml:"files,omitempty"` + Outputs Outputs `json:"outputs,omitempty" yaml:"outputs,omitempty"` image string } +type Outputs map[string]Output + +type Output struct { + Value string `json:"value,omitempty" yaml:"value,omitempty"` + Format string `json:"format,omitempty" yaml:"format,omitempty"` +} + type TransformChanges struct { Group []Group `json:"group,omitempty" yaml:"group"` } diff --git a/internal/campaigns/execution_cache.go b/internal/campaigns/execution_cache.go index 62017a8430..5a5be4a7b5 100644 --- a/internal/campaigns/execution_cache.go +++ b/internal/campaigns/execution_cache.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "os" "path/filepath" + "sort" "strings" "github.com/pkg/errors" @@ -62,8 +63,8 @@ func (key ExecutionCacheKey) Key() (string, error) { } type ExecutionCache interface { - Get(ctx context.Context, key ExecutionCacheKey) (diff string, found bool, err error) - Set(ctx context.Context, key ExecutionCacheKey, diff string) error + Get(ctx context.Context, key ExecutionCacheKey) (result ExecutionResult, found bool, err error) + Set(ctx context.Context, key ExecutionCacheKey, result ExecutionResult) error Clear(ctx context.Context, key ExecutionCacheKey) error } @@ -71,64 +72,174 @@ type ExecutionDiskCache struct { Dir string } +const cacheFileExt = ".v3.json" + func (c ExecutionDiskCache) cacheFilePath(key ExecutionCacheKey) (string, error) { keyString, err := key.Key() if err != nil { return "", errors.Wrap(err, "calculating execution cache key") } - return filepath.Join(c.Dir, keyString+".diff"), nil + return filepath.Join(c.Dir, keyString+cacheFileExt), nil } -func (c ExecutionDiskCache) Get(ctx context.Context, key ExecutionCacheKey) (string, bool, error) { +func (c ExecutionDiskCache) Get(ctx context.Context, key ExecutionCacheKey) (ExecutionResult, bool, error) { + var result ExecutionResult + path, err := c.cacheFilePath(key) if err != nil { - return "", false, err + return result, false, err } - data, err := ioutil.ReadFile(path) + // We try to be backwards compatible and see if we also find older cache + // files. + // + // There are three different cache versions out in the wild and to be + // backwards compatible we read all of them. + // + // In Sourcegraph/src-cli 3.26 we can remove the code here and simply read + // the cache from `path`, since all the old cache files should be deleted + // until then. + globPattern := strings.TrimSuffix(path, cacheFileExt) + ".*" + matches, err := filepath.Glob(globPattern) if err != nil { - if os.IsNotExist(err) { - err = nil // treat as not-found + return result, false, err + } + + switch len(matches) { + case 0: + // Nothing found + return result, false, nil + case 1: + // One cache file found + if err := c.readCacheFile(matches[0], &result); err != nil { + return result, false, err } - return "", false, err + + // If it's an old cache file, we rewrite the cache and delete the old file + if isOldCacheFile(matches[0]) { + if err := c.Set(ctx, key, result); err != nil { + return result, false, errors.Wrap(err, "failed to rewrite cache in new format") + } + if err := os.Remove(matches[0]); err != nil { + return result, false, errors.Wrap(err, "failed to remove old cache file") + } + } + + return result, true, err + + default: + // More than one cache file found. + // Sort them so that we'll can possibly read from the one with the most + // current version. + sortCacheFiles(matches) + + newest := matches[0] + toDelete := matches[1:] + + // Read from newest + if err := c.readCacheFile(newest, &result); err != nil { + return result, false, err + } + + // If the newest was also an older version, we write a new version... + if isOldCacheFile(newest) { + if err := c.Set(ctx, key, result); err != nil { + return result, false, errors.Wrap(err, "failed to rewrite cache in new format") + } + // ... and mark the file also as to-be-deleted + toDelete = append(toDelete, newest) + } + + // Now we clean up the old ones + for _, path := range toDelete { + if err := os.Remove(path); err != nil { + return result, false, errors.Wrap(err, "failed to remove old cache file") + } + } + + return result, true, nil } +} - // We previously cached complete ChangesetSpecs instead of just the diffs. - // To be backwards compatible, we keep reading these: - if strings.HasSuffix(path, ".json") { - var result ChangesetSpec - if err := json.Unmarshal(data, &result); err != nil { +// sortCacheFiles sorts cache file paths by their "version", so that files +// ending in `cacheFileExt` are first. +func sortCacheFiles(paths []string) { + sort.Slice(paths, func(i, j int) bool { + return !isOldCacheFile(paths[i]) && isOldCacheFile(paths[j]) + }) +} + +func isOldCacheFile(path string) bool { return !strings.HasSuffix(path, cacheFileExt) } + +func (c ExecutionDiskCache) readCacheFile(path string, result *ExecutionResult) error { + data, err := ioutil.ReadFile(path) + if err != nil { + return err + } + + switch { + case strings.HasSuffix(path, ".v3.json"): + // v3 of the cache: we cache the diff and the outputs produced by the step. + if err := json.Unmarshal(data, result); err != nil { + // Delete the invalid data to avoid causing an error for next time. + if err := os.Remove(path); err != nil { + return errors.Wrap(err, "while deleting cache file with invalid JSON") + } + return errors.Wrapf(err, "reading cache file %s", path) + } + return nil + + case strings.HasSuffix(path, ".diff"): + // v2 of the cache: we only cached the diff, since that's the + // only bit of data we were interested in. + result.Diff = string(data) + result.Outputs = map[string]interface{}{} + // Conversion is lossy, though: we don't populate result.StepChanges. + result.ChangedFiles = &StepChanges{} + + return nil + + case strings.HasSuffix(path, ".json"): + // v1 of the cache: we cached the complete ChangesetSpec instead of just the diffs. + var spec ChangesetSpec + if err := json.Unmarshal(data, &spec); err != nil { // Delete the invalid data to avoid causing an error for next time. if err := os.Remove(path); err != nil { - return "", false, errors.Wrap(err, "while deleting cache file with invalid JSON") + return errors.Wrap(err, "while deleting cache file with invalid JSON") } - return "", false, errors.Wrapf(err, "reading cache file %s", path) + return errors.Wrapf(err, "reading cache file %s", path) } - if len(result.Commits) != 1 { - return "", false, errors.New("cached result has no commits") + if len(spec.Commits) != 1 { + return errors.New("cached result has no commits") } - return result.Commits[0].Diff, true, nil - } - if strings.HasSuffix(path, ".diff") { - return string(data), true, nil + result.Diff = spec.Commits[0].Diff + result.Outputs = map[string]interface{}{} + result.ChangedFiles = &StepChanges{} + + return nil } - return "", false, fmt.Errorf("unknown file format for cache file %q", path) + return fmt.Errorf("unknown file format for cache file %q", path) } -func (c ExecutionDiskCache) Set(ctx context.Context, key ExecutionCacheKey, diff string) error { +func (c ExecutionDiskCache) Set(ctx context.Context, key ExecutionCacheKey, result ExecutionResult) error { path, err := c.cacheFilePath(key) if err != nil { return err } + raw, err := json.Marshal(&result) + if err != nil { + return errors.Wrap(err, "serializing execution result to JSON") + } + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { return err } - return ioutil.WriteFile(path, []byte(diff), 0600) + return ioutil.WriteFile(path, raw, 0600) } func (c ExecutionDiskCache) Clear(ctx context.Context, key ExecutionCacheKey) error { @@ -148,11 +259,11 @@ func (c ExecutionDiskCache) Clear(ctx context.Context, key ExecutionCacheKey) er // retrieve cache entries. type ExecutionNoOpCache struct{} -func (ExecutionNoOpCache) Get(ctx context.Context, key ExecutionCacheKey) (diff string, found bool, err error) { - return "", false, nil +func (ExecutionNoOpCache) Get(ctx context.Context, key ExecutionCacheKey) (result ExecutionResult, found bool, err error) { + return ExecutionResult{}, false, nil } -func (ExecutionNoOpCache) Set(ctx context.Context, key ExecutionCacheKey, diff string) error { +func (ExecutionNoOpCache) Set(ctx context.Context, key ExecutionCacheKey, result ExecutionResult) error { return nil } diff --git a/internal/campaigns/execution_cache_test.go b/internal/campaigns/execution_cache_test.go index 8ab99fa7a6..95c4581c6a 100644 --- a/internal/campaigns/execution_cache_test.go +++ b/internal/campaigns/execution_cache_test.go @@ -1,10 +1,16 @@ package campaigns import ( + "context" + "encoding/json" + "io/ioutil" "os" + "path/filepath" "testing" - "gopkg.in/yaml.v2" + "github.com/google/go-cmp/cmp" + "github.com/sourcegraph/src-cli/internal/campaigns/graphql" + "gopkg.in/yaml.v3" ) const testExecutionCacheKeyEnv = "TEST_EXECUTION_CACHE_KEY_ENV" @@ -86,3 +92,269 @@ func TestExecutionCacheKey(t *testing.T) { t.Errorf("unexpected change in key: initial=%q have=%q", initial, have) } } + +const testDiff = `diff --git a/README.md b/README.md +new file mode 100644 +index 0000000..3363c39 +--- /dev/null ++++ b/README.md +@@ -0,0 +1,3 @@ ++# README ++ ++This is the readme +` + +func TestExecutionDiskCache(t *testing.T) { + ctx := context.Background() + + cacheTmpDir := func(t *testing.T) string { + testTempDir, err := ioutil.TempDir("", "execution-disk-cache-test-*") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Remove(testTempDir) }) + + return testTempDir + } + + cacheKey1 := ExecutionCacheKey{Task: &Task{ + Repository: &graphql.Repository{Name: "src-cli"}, + Steps: []Step{ + {Run: "echo 'Hello World'", Container: "alpine:3"}, + }, + }} + + cacheKey2 := ExecutionCacheKey{Task: &Task{ + Repository: &graphql.Repository{Name: "documentation"}, + Steps: []Step{ + {Run: "echo 'Hello World'", Container: "alpine:3"}, + }, + }} + + value := ExecutionResult{ + Diff: testDiff, + ChangedFiles: &StepChanges{ + Added: []string{"README.md"}, + }, + Outputs: map[string]interface{}{}, + } + + onlyDiff := ExecutionResult{ + Diff: testDiff, + ChangedFiles: &StepChanges{}, + Outputs: map[string]interface{}{}, + } + + t.Run("cache contains v3 cache file", func(t *testing.T) { + cache := ExecutionDiskCache{Dir: cacheTmpDir(t)} + + // Empty cache, no hits + assertCacheMiss(t, cache, cacheKey1) + assertCacheMiss(t, cache, cacheKey2) + + // Set the cache + if err := cache.Set(ctx, cacheKey1, value); err != nil { + t.Fatalf("cache.Set returned unexpected error: %s", err) + } + + // Cache hit + assertCacheHit(t, cache, cacheKey1, value) + + // Cache miss due to different key + assertCacheMiss(t, cache, cacheKey2) + + // Cache miss due to cleared cache + if err := cache.Clear(ctx, cacheKey1); err != nil { + t.Fatalf("cache.Get returned unexpected error: %s", err) + } + assertCacheMiss(t, cache, cacheKey1) + }) + + t.Run("cache contains v1 cache file", func(t *testing.T) { + cache := ExecutionDiskCache{Dir: cacheTmpDir(t)} + + // Empty cache, no hit + assertCacheMiss(t, cache, cacheKey1) + + // Simulate old cache file lying around in cache + oldFilePath := writeV1CacheFile(t, cache, cacheKey1, testDiff) + + // Cache hit, but only for the diff + assertCacheHit(t, cache, cacheKey1, onlyDiff) + + // And the old file should be deleted + assertFileDeleted(t, oldFilePath) + // .. but we should still get a cache hit, because we rewrote the cache + assertCacheHit(t, cache, cacheKey1, onlyDiff) + }) + + t.Run("cache contains v2 cache file", func(t *testing.T) { + cache := ExecutionDiskCache{Dir: cacheTmpDir(t)} + + // Empty cache, no hit + assertCacheMiss(t, cache, cacheKey1) + + // Simulate old cache file lying around in cache + oldFilePath := writeV2CacheFile(t, cache, cacheKey1, testDiff) + + // Now we get a cache hit, but only for the diff + assertCacheHit(t, cache, cacheKey1, onlyDiff) + + // And the old file should be deleted + assertFileDeleted(t, oldFilePath) + // .. but we should still get a cache hit, because we rewrote the cache + assertCacheHit(t, cache, cacheKey1, onlyDiff) + }) + + t.Run("cache contains one old and one v3 cache file", func(t *testing.T) { + cache := ExecutionDiskCache{Dir: cacheTmpDir(t)} + + // Simulate v2 and v3 files in cache + oldFilePath := writeV1CacheFile(t, cache, cacheKey1, testDiff) + + if err := cache.Set(ctx, cacheKey1, value); err != nil { + t.Fatalf("cache.Set returned unexpected error: %s", err) + } + + // Cache hit + assertCacheHit(t, cache, cacheKey1, value) + + // And the old file should be deleted + assertFileDeleted(t, oldFilePath) + }) + + t.Run("cache contains multiple old cache files", func(t *testing.T) { + cache := ExecutionDiskCache{Dir: cacheTmpDir(t)} + + // Simulate v1 and v2 files in cache + oldFilePath1 := writeV1CacheFile(t, cache, cacheKey1, testDiff) + oldFilePath2 := writeV1CacheFile(t, cache, cacheKey1, testDiff) + + // Now we get a cache hit, but only for the diff + assertCacheHit(t, cache, cacheKey1, onlyDiff) + + // And the old files should be deleted + assertFileDeleted(t, oldFilePath1) + assertFileDeleted(t, oldFilePath2) + // .. but we should still get a cache hit, because we rewrote the cache + assertCacheHit(t, cache, cacheKey1, onlyDiff) + }) +} + +func TestSortCacheFiles(t *testing.T) { + tests := []struct { + paths []string + want []string + }{ + { + paths: []string{"file.v3.json", "file.diff", "file.json"}, + want: []string{"file.v3.json", "file.diff", "file.json"}, + }, + { + paths: []string{"file.json", "file.diff", "file.v3.json"}, + want: []string{"file.v3.json", "file.json", "file.diff"}, + }, + { + paths: []string{"file.diff", "file.v3.json"}, + want: []string{"file.v3.json", "file.diff"}, + }, + { + paths: []string{"file1.v3.json", "file2.v3.json"}, + want: []string{"file1.v3.json", "file2.v3.json"}, + }, + } + + for _, tt := range tests { + sortCacheFiles(tt.paths) + if diff := cmp.Diff(tt.paths, tt.want); diff != "" { + t.Errorf("wrong cached result (-have +want):\n\n%s", diff) + } + } +} + +func assertFileDeleted(t *testing.T, path string) { + t.Helper() + if _, err := os.Stat(path); err == nil { + t.Fatalf("file exists: %s", path) + } else if os.IsNotExist(err) { + // Seems to be deleted, all good + } else { + t.Fatalf("could not determine whether file exists: %s", err) + } +} + +func writeV1CacheFile(t *testing.T, c ExecutionDiskCache, k ExecutionCacheKey, diff string) (path string) { + t.Helper() + + hashedKey, err := k.Key() + if err != nil { + t.Fatalf("failed to hash cacheKey: %s", err) + } + // The v1 file format ended in .json + path = filepath.Join(c.Dir, hashedKey+".json") + + // v1 contained a fully serialized ChangesetSpec + spec := ChangesetSpec{CreatedChangeset: &CreatedChangeset{ + Commits: []GitCommitDescription{ + {Diff: testDiff}, + }, + }} + + raw, err := json.Marshal(&spec) + if err != nil { + t.Fatal(err) + } + + if err := ioutil.WriteFile(path, raw, 0600); err != nil { + t.Fatalf("writing the cache file failed: %s", err) + } + + return path +} + +func writeV2CacheFile(t *testing.T, c ExecutionDiskCache, k ExecutionCacheKey, diff string) (path string) { + t.Helper() + + hashedKey, err := k.Key() + if err != nil { + t.Fatalf("failed to hash cacheKey: %s", err) + } + + // The v2 file format ended in .json + path = filepath.Join(c.Dir, hashedKey+".diff") + + // v2 contained only a diff + if err := ioutil.WriteFile(path, []byte(diff), 0600); err != nil { + t.Fatalf("writing the cache file failed: %s", err) + } + + return path +} + +func assertCacheHit(t *testing.T, c ExecutionDiskCache, k ExecutionCacheKey, want ExecutionResult) { + t.Helper() + + have, found, err := c.Get(context.Background(), k) + if err != nil { + t.Fatalf("cache.Get returned unexpected error: %s", err) + } + if !found { + t.Fatalf("cache miss when hit was expected") + } + + if diff := cmp.Diff(have, want); diff != "" { + t.Errorf("wrong cached result (-have +want):\n\n%s", diff) + } +} + +func assertCacheMiss(t *testing.T, c ExecutionDiskCache, k ExecutionCacheKey) { + t.Helper() + + _, found, err := c.Get(context.Background(), k) + if err != nil { + t.Fatalf("cache.Get returned unexpected error: %s", err) + } + if found { + t.Fatalf("cache hit when miss was expected") + } +} diff --git a/internal/campaigns/executor.go b/internal/campaigns/executor.go index c41dec49a0..9897b4385e 100644 --- a/internal/campaigns/executor.go +++ b/internal/campaigns/executor.go @@ -57,6 +57,7 @@ type Executor interface { type Task struct { Repository *graphql.Repository Steps []Step + Outputs map[string]interface{} Template *ChangesetTemplate `json:"-"` TransformChanges *TransformChanges `json:"-"` @@ -177,7 +178,13 @@ func newExecutor(opts ExecutorOpts, client api.Client, features featureFlags) *e } func (x *executor) AddTask(repo *graphql.Repository, steps []Step, transform *TransformChanges, template *ChangesetTemplate) { - task := &Task{repo, steps, template, transform} + task := &Task{ + Repository: repo, + Steps: steps, + Template: template, + TransformChanges: transform, + } + x.tasks = append(x.tasks, task) x.statusesMu.Lock() @@ -249,11 +256,11 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) { } } else { var ( - diff string - found bool + result ExecutionResult + found bool ) - diff, found, err = x.cache.Get(ctx, cacheKey) + result, found, err = x.cache.Get(ctx, cacheKey) if err != nil { err = errors.Wrapf(err, "checking cache for %q", task.Repository.Name) return @@ -263,7 +270,7 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) { // add it to the list of specs that are displayed to the user and // send to the server. Instead, we can just report that the task is // complete and move on. - if len(diff) == 0 { + if len(result.Diff) == 0 { x.updateTaskStatus(task, func(status *TaskStatus) { status.Cached = true status.FinishedAt = time.Now() @@ -273,7 +280,7 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) { } var specs []*ChangesetSpec - specs, err = createChangesetSpecs(task, diff, x.features) + specs, err = createChangesetSpecs(task, result, x.features) if err != nil { return err } @@ -317,7 +324,7 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) { defer cancel() // Actually execute the steps. - diff, err := runSteps(runCtx, x.fetcher, x.creator, task.Repository, task.Steps, log, x.tempDir, func(currentlyExecuting string) { + result, err := runSteps(runCtx, x.fetcher, x.creator, task.Repository, task.Steps, log, x.tempDir, func(currentlyExecuting string) { x.updateTaskStatus(task, func(status *TaskStatus) { status.CurrentlyExecuting = currentlyExecuting }) @@ -331,20 +338,20 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) { } // Build the changeset specs. - specs, err := createChangesetSpecs(task, string(diff), x.features) + specs, err := createChangesetSpecs(task, result, x.features) if err != nil { return err } // Add to the cache. We don't use runCtx here because we want to write to // the cache even if we've now reached the timeout. - if err = x.cache.Set(ctx, cacheKey, string(diff)); err != nil { + if err = x.cache.Set(ctx, cacheKey, result); err != nil { err = errors.Wrapf(err, "caching result for %q", task.Repository.Name) } // If the steps didn't result in any diff, we don't need to add it to the // list of specs that are displayed to the user and send to the server. - if len(diff) == 0 { + if len(result.Diff) == 0 { return } @@ -397,9 +404,15 @@ func reachedTimeout(cmdCtx context.Context, err error) bool { return errors.Is(err, context.DeadlineExceeded) } -func createChangesetSpecs(task *Task, completeDiff string, features featureFlags) ([]*ChangesetSpec, error) { +func createChangesetSpecs(task *Task, result ExecutionResult, features featureFlags) ([]*ChangesetSpec, error) { repo := task.Repository.Name + tmplCtx := &ChangesetTemplateContext{ + Steps: result.ChangedFiles, + Outputs: result.Outputs, + Repository: *task.Repository, + } + var authorName string var authorEmail string @@ -410,8 +423,38 @@ func createChangesetSpecs(task *Task, completeDiff string, features featureFlags authorEmail = "campaigns@sourcegraph.com" } } else { - authorName = task.Template.Commit.Author.Name - authorEmail = task.Template.Commit.Author.Email + var err error + authorName, err = renderChangesetTemplateField("authorName", task.Template.Commit.Author.Name, tmplCtx) + if err != nil { + return nil, err + } + authorEmail, err = renderChangesetTemplateField("authorEmail", task.Template.Commit.Author.Email, tmplCtx) + if err != nil { + return nil, err + } + } + + title, err := renderChangesetTemplateField("title", task.Template.Title, tmplCtx) + if err != nil { + return nil, err + } + + body, err := renderChangesetTemplateField("body", task.Template.Body, tmplCtx) + if err != nil { + return nil, err + } + + message, err := renderChangesetTemplateField("message", task.Template.Commit.Message, tmplCtx) + if err != nil { + return nil, err + } + + // TODO: As a next step, we should extend the ChangesetTemplateContext to also include + // TransformChanges.Group and then change validateGroups and groupFileDiffs to, for each group, + // render the branch name *before* grouping the diffs. + defaultBranch, err := renderChangesetTemplateField("branch", task.Template.Branch, tmplCtx) + if err != nil { + return nil, err } newSpec := func(branch, diff string) *ChangesetSpec { @@ -422,11 +465,11 @@ func createChangesetSpecs(task *Task, completeDiff string, features featureFlags BaseRev: task.Repository.Rev(), HeadRepository: task.Repository.ID, HeadRef: "refs/heads/" + branch, - Title: task.Template.Title, - Body: task.Template.Body, + Title: title, + Body: body, Commits: []GitCommitDescription{ { - Message: task.Template.Commit.Message, + Message: message, AuthorName: authorName, AuthorEmail: authorEmail, Diff: diff, @@ -446,7 +489,8 @@ func createChangesetSpecs(task *Task, completeDiff string, features featureFlags return specs, err } - diffsByBranch, err := groupFileDiffs(completeDiff, task.Template.Branch, groups) + // TODO: Regarding 'defaultBranch', see comment above + diffsByBranch, err := groupFileDiffs(result.Diff, defaultBranch, groups) if err != nil { return specs, errors.Wrap(err, "grouping diffs failed") } @@ -455,7 +499,7 @@ func createChangesetSpecs(task *Task, completeDiff string, features featureFlags specs = append(specs, newSpec(branch, diff)) } } else { - specs = append(specs, newSpec(task.Template.Branch, string(completeDiff))) + specs = append(specs, newSpec(defaultBranch, result.Diff)) } return specs, nil diff --git a/internal/campaigns/executor_test.go b/internal/campaigns/executor_test.go index e280cebfaf..f0c714cd23 100644 --- a/internal/campaigns/executor_test.go +++ b/internal/campaigns/executor_test.go @@ -61,13 +61,20 @@ func TestExecutor_Integration(t *testing.T) { repos []*graphql.Repository archives []mockRepoArchive + template *ChangesetTemplate steps []Step transform *TransformChanges executorTimeout time.Duration - wantFilesChanged filesByRepository - wantErrInclude string + wantFilesChanged filesByRepository + wantTitle string + wantBody string + wantCommitMessage string + wantAuthorName string + wantAuthorEmail string + + wantErrInclude string }{ { name: "success", @@ -196,6 +203,73 @@ func TestExecutor_Integration(t *testing.T) { }, }, }, + { + name: "templated changesetTemplate", + repos: []*graphql.Repository{srcCLIRepo}, + archives: []mockRepoArchive{ + {repo: srcCLIRepo, files: map[string]string{ + "README.md": "# Welcome to the README\n", + "main.go": "package main\n\nfunc main() {\n\tfmt.Println( \"Hello World\")\n}\n", + }}, + }, + steps: []Step{ + { + Run: `go fmt main.go`, + Container: "doesntmatter:13", + Outputs: Outputs{ + "myOutputName1": Output{ + Value: "${{ index step.modified_files 0 }}", + }, + }, + }, + { + Run: `echo -n "Hello World!"`, + Container: "alpine:13", + Outputs: Outputs{ + "myOutputName2": Output{ + Value: `thisStepStdout: "${{ step.stdout }}"`, + Format: "yaml", + }, + "myOutputName3": Output{ + Value: "cool-suffix", + }, + }, + }, + }, + template: &ChangesetTemplate{ + Title: "myOutputName1=${{ outputs.myOutputName1}}", + Body: `myOutputName1=${{ outputs.myOutputName1}},myOutputName2=${{ outputs.myOutputName2.thisStepStdout }} +modified_files=${{ steps.modified_files }} +added_files=${{ steps.added_files }} +deleted_files=${{ steps.deleted_files }} +renamed_files=${{ steps.renamed_files }} +repository_name=${{ repository.name }}`, + Branch: "templated-branch-${{ outputs.myOutputName3 }}", + Commit: ExpandedGitCommitDescription{ + Message: "myOutputName1=${{ outputs.myOutputName1}},myOutputName2=${{ outputs.myOutputName2.thisStepStdout }}", + Author: &GitCommitAuthor{ + Name: "myOutputName1=${{ outputs.myOutputName1}}", + Email: "myOutputName1=${{ outputs.myOutputName1}}", + }, + }, + }, + + wantFilesChanged: filesByRepository{ + srcCLIRepo.ID: filesByBranch{ + "templated-branch-cool-suffix": []string{"main.go"}, + }, + }, + wantTitle: "myOutputName1=main.go", + wantBody: `myOutputName1=main.go,myOutputName2=Hello World! +modified_files=[main.go] +added_files=[] +deleted_files=[] +renamed_files=[] +repository_name=github.com/sourcegraph/src-cli`, + wantCommitMessage: "myOutputName1=main.go,myOutputName2=Hello World!", + wantAuthorName: "myOutputName1=main.go", + wantAuthorEmail: "myOutputName1=main.go", + }, } for _, tc := range tests { @@ -236,6 +310,9 @@ func TestExecutor_Integration(t *testing.T) { executor := newExecutor(opts, client, featuresAllEnabled()) template := &ChangesetTemplate{Branch: changesetTemplateBranch} + if tc.template != nil { + template = tc.template + } for _, r := range tc.repos { executor.AddTask(r, tc.steps, tc.transform, template) @@ -266,6 +343,19 @@ func TestExecutor_Integration(t *testing.T) { t.Fatalf("wrong number of commits. want=%d, have=%d", want, have) } + attrs := []struct{ name, want, have string }{ + {name: "title", want: tc.wantTitle, have: spec.Title}, + {name: "body", want: tc.wantBody, have: spec.Body}, + {name: "commit.Message", want: tc.wantCommitMessage, have: spec.Commits[0].Message}, + {name: "commit.AuthorEmail", want: tc.wantAuthorEmail, have: spec.Commits[0].AuthorEmail}, + {name: "commit.AuthorName", want: tc.wantAuthorName, have: spec.Commits[0].AuthorName}, + } + for _, attr := range attrs { + if attr.want != "" && attr.have != attr.want { + t.Errorf("wrong %q attribute. want=%q, have=%q", attr.name, attr.want, attr.have) + } + } + wantFiles, ok := tc.wantFilesChanged[spec.BaseRepository] if !ok { t.Fatalf("unexpected file changes in repo %s", spec.BaseRepository) @@ -548,13 +638,13 @@ func newZipArchivesMux(t *testing.T, callback http.HandlerFunc, archives ...mock // inMemoryExecutionCache provides an in-memory cache for testing purposes. type inMemoryExecutionCache struct { - cache map[string]string + cache map[string]ExecutionResult mu sync.RWMutex } func newInMemoryExecutionCache() *inMemoryExecutionCache { return &inMemoryExecutionCache{ - cache: make(map[string]string), + cache: make(map[string]ExecutionResult), } } @@ -565,22 +655,22 @@ func (c *inMemoryExecutionCache) size() int { return len(c.cache) } -func (c *inMemoryExecutionCache) Get(ctx context.Context, key ExecutionCacheKey) (string, bool, error) { +func (c *inMemoryExecutionCache) Get(ctx context.Context, key ExecutionCacheKey) (ExecutionResult, bool, error) { k, err := key.Key() if err != nil { - return "", false, err + return ExecutionResult{}, false, err } c.mu.RLock() defer c.mu.RUnlock() - if diff, ok := c.cache[k]; ok { - return diff, true, nil + if res, ok := c.cache[k]; ok { + return res, true, nil } - return "", false, nil + return ExecutionResult{}, false, nil } -func (c *inMemoryExecutionCache) Set(ctx context.Context, key ExecutionCacheKey, diff string) error { +func (c *inMemoryExecutionCache) Set(ctx context.Context, key ExecutionCacheKey, result ExecutionResult) error { k, err := key.Key() if err != nil { return err @@ -589,7 +679,7 @@ func (c *inMemoryExecutionCache) Set(ctx context.Context, key ExecutionCacheKey, c.mu.Lock() defer c.mu.Unlock() - c.cache[k] = diff + c.cache[k] = result return nil } diff --git a/internal/campaigns/run_steps.go b/internal/campaigns/run_steps.go index 9350dc2efc..d30b0a1129 100644 --- a/internal/campaigns/run_steps.go +++ b/internal/campaigns/run_steps.go @@ -3,6 +3,7 @@ package campaigns import ( "bytes" "context" + "encoding/json" "fmt" "io" "io/ioutil" @@ -14,29 +15,45 @@ import ( "github.com/hashicorp/go-multierror" "github.com/pkg/errors" "github.com/sourcegraph/src-cli/internal/campaigns/graphql" + + yamlv3 "gopkg.in/yaml.v3" ) -func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *graphql.Repository, steps []Step, logger *TaskLogger, tempDir string, reportProgress func(string)) ([]byte, error) { +type ExecutionResult struct { + // Diff is the produced by executing all steps. + Diff string `json:"diff"` + + // ChangedFiles are files that have been changed by all steps. + ChangedFiles *StepChanges `json:"changedFiles"` + + // Outputs are the outputs produced by all steps. + Outputs map[string]interface{} `json:"outputs"` +} + +func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *graphql.Repository, steps []Step, logger *TaskLogger, tempDir string, reportProgress func(string)) (result ExecutionResult, err error) { reportProgress("Downloading archive") zip, err := rf.Fetch(ctx, repo) if err != nil { - return nil, errors.Wrap(err, "fetching repo") + return ExecutionResult{}, errors.Wrap(err, "fetching repo") } defer zip.Close() reportProgress("Initializing workspace") workspace, err := wc.Create(ctx, repo, zip.Path()) if err != nil { - return nil, errors.Wrap(err, "creating workspace") + return ExecutionResult{}, errors.Wrap(err, "creating workspace") } defer workspace.Close(ctx) + execResult := ExecutionResult{ + Outputs: make(map[string]interface{}), + } results := make([]StepResult, len(steps)) for i, step := range steps { reportProgress(fmt.Sprintf("Preparing step %d", i+1)) - stepContext := StepContext{Repository: *repo} + stepContext := StepContext{Repository: *repo, Outputs: execResult.Outputs} if i > 0 { stepContext.PreviousStep = results[i-1] } @@ -46,7 +63,7 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr // container on a successful run, rather than leaving it dangling. cidFile, err := ioutil.TempFile(tempDir, repo.Slug()+"-container-id") if err != nil { - return nil, errors.Wrap(err, "Creating a CID file failed") + return execResult, errors.Wrap(err, "Creating a CID file failed") } // However, Docker will fail if the cidfile actually exists, so we need @@ -54,7 +71,7 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr // close it, even though that's unnecessary elsewhere. cidFile.Close() if err = os.Remove(cidFile.Name()); err != nil { - return nil, errors.Wrap(err, "removing cidfile") + return execResult, errors.Wrap(err, "removing cidfile") } // Since we went to all that effort, we can now defer a function that @@ -72,14 +89,14 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr // For now, we only support shell scripts provided via the Run field. shell, containerTemp, err := probeImageForShell(ctx, step.image) if err != nil { - return nil, errors.Wrapf(err, "probing image %q for shell", step.image) + return execResult, errors.Wrapf(err, "probing image %q for shell", step.image) } // Set up a temporary file on the host filesystem to contain the // script. runScriptFile, err := ioutil.TempFile(tempDir, "") if err != nil { - return nil, errors.Wrap(err, "creating temporary file") + return execResult, errors.Wrap(err, "creating temporary file") } defer os.Remove(runScriptFile.Name()) @@ -87,12 +104,12 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr // temp file we just created. var runScript bytes.Buffer out := io.MultiWriter(&runScript, runScriptFile) - if err := renderTemplate("step-run", step.Run, out, &stepContext); err != nil { - return nil, errors.Wrap(err, "parsing step run") + if err := renderStepTemplate("step-run", step.Run, out, &stepContext); err != nil { + return execResult, errors.Wrap(err, "parsing step run") } if err := runScriptFile.Close(); err != nil { - return nil, errors.Wrap(err, "closing temporary file") + return execResult, errors.Wrap(err, "closing temporary file") } // This file needs to be readable within the container regardless of the @@ -105,13 +122,13 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr // conditionally compiled files here, instead we'll just wait until the // file is closed to twiddle the permission bits. Which is now! if err := os.Chmod(runScriptFile.Name(), 0644); err != nil { - return nil, errors.Wrap(err, "setting permissions on the temporary file") + return execResult, errors.Wrap(err, "setting permissions on the temporary file") } // Parse and render the step.Files. - files, err := renderMap(step.Files, &stepContext) + files, err := renderStepMap(step.Files, &stepContext) if err != nil { - return nil, errors.Wrap(err, "parsing step files") + return execResult, errors.Wrap(err, "parsing step files") } // Create temp files with the rendered content of step.Files so that we @@ -120,16 +137,16 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr for name, content := range files { fp, err := ioutil.TempFile(tempDir, "") if err != nil { - return nil, errors.Wrap(err, "creating temporary file") + return execResult, errors.Wrap(err, "creating temporary file") } defer os.Remove(fp.Name()) if _, err := fp.WriteString(content); err != nil { - return nil, errors.Wrap(err, "writing to temporary file") + return execResult, errors.Wrap(err, "writing to temporary file") } if err := fp.Close(); err != nil { - return nil, errors.Wrap(err, "closing temporary file") + return execResult, errors.Wrap(err, "closing temporary file") } filesToMount[name] = fp @@ -138,20 +155,20 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr // Resolve step.Env given the current environment. stepEnv, err := step.Env.Resolve(os.Environ()) if err != nil { - return nil, errors.Wrap(err, "resolving step environment") + return execResult, errors.Wrap(err, "resolving step environment") } // Render the step.Env variables as templates. - env, err := renderMap(stepEnv, &stepContext) + env, err := renderStepMap(stepEnv, &stepContext) if err != nil { - return nil, errors.Wrap(err, "parsing step environment") + return execResult, errors.Wrap(err, "parsing step environment") } reportProgress(runScript.String()) const workDir = "/work" workspaceOpts, err := workspace.DockerRunOpts(ctx, workDir) if err != nil { - return nil, errors.Wrap(err, "getting Docker options for workspace") + return execResult, errors.Wrap(err, "getting Docker options for workspace") } args := append([]string{ "run", @@ -190,7 +207,7 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr if err != nil { logger.Logf("[Step %d] took %s; error running Docker container: %+v", i+1, elapsed, err) - return nil, stepFailedErr{ + return execResult, stepFailedErr{ Err: err, Args: cmd.Args, Run: runScript.String(), @@ -205,19 +222,61 @@ func runSteps(ctx context.Context, rf RepoFetcher, wc WorkspaceCreator, repo *gr changes, err := workspace.Changes(ctx) if err != nil { - return nil, errors.Wrap(err, "getting changed files in step") + return execResult, errors.Wrap(err, "getting changed files in step") } - results[i] = StepResult{files: changes, Stdout: &stdoutBuffer, Stderr: &stderrBuffer} + result := StepResult{files: changes, Stdout: &stdoutBuffer, Stderr: &stderrBuffer} + stepContext.Step = result + results[i] = result + + setOutputs(step.Outputs, execResult.Outputs, &stepContext) } reportProgress("Calculating diff") diffOut, err := workspace.Diff(ctx) if err != nil { - return nil, errors.Wrap(err, "git diff failed") + return execResult, errors.Wrap(err, "git diff failed") + } + + execResult.Diff = string(diffOut) + if len(results) > 0 && results[len(results)-1].files != nil { + execResult.ChangedFiles = results[len(results)-1].files + } + + return execResult, err +} + +func setOutputs(stepOutputs Outputs, global map[string]interface{}, stepCtx *StepContext) error { + for name, output := range stepOutputs { + var value bytes.Buffer + + if err := renderStepTemplate("outputs-"+name, output.Value, &value, stepCtx); err != nil { + return errors.Wrap(err, "parsing step run") + } + + switch output.Format { + case "yaml": + var out interface{} + // We use yamlv3 here, because it unmarshals YAML into + // map[string]interface{} which we need to serialize it back to + // JSON when we cache the results. + // See https://github.com/go-yaml/yaml/issues/139 for context + if err := yamlv3.NewDecoder(&value).Decode(&out); err != nil { + return err + } + global[name] = out + case "json": + var out interface{} + if err := json.NewDecoder(&value).Decode(&out); err != nil { + return err + } + global[name] = out + default: + global[name] = value.String() + } } - return diffOut, err + return nil } func probeImageForShell(ctx context.Context, image string) (shell, tempfile string, err error) { diff --git a/internal/campaigns/templating.go b/internal/campaigns/templating.go index f7c0620da9..417b5a8799 100644 --- a/internal/campaigns/templating.go +++ b/internal/campaigns/templating.go @@ -11,7 +11,7 @@ import ( "github.com/sourcegraph/src-cli/internal/campaigns/graphql" ) -func renderTemplate(name, tmpl string, out io.Writer, stepCtx *StepContext) error { +func renderStepTemplate(name, tmpl string, out io.Writer, stepCtx *StepContext) error { t, err := parseAsTemplate(name, tmpl, stepCtx) if err != nil { return errors.Wrap(err, "parsing step run") @@ -24,13 +24,13 @@ func parseAsTemplate(name, input string, stepCtx *StepContext) (*template.Templa return template.New(name).Delims("${{", "}}").Funcs(stepCtx.ToFuncMap()).Parse(input) } -func renderMap(m map[string]string, stepCtx *StepContext) (map[string]string, error) { +func renderStepMap(m map[string]string, stepCtx *StepContext) (map[string]string, error) { rendered := make(map[string]string, len(m)) for k, v := range rendered { var out bytes.Buffer - if err := renderTemplate(k, v, &out, stepCtx); err != nil { + if err := renderStepTemplate(k, v, &out, stepCtx); err != nil { return rendered, err } @@ -40,44 +40,64 @@ func renderMap(m map[string]string, stepCtx *StepContext) (map[string]string, er return rendered, nil } -// StepContext represents the contextual information available when executing a -// step that's defined in a campaign spec. +// StepContext represents the contextual information available when rendering a +// step's fields, such as "run" or "outputs", as templates. type StepContext struct { + // Outputs are the outputs set by the current and all previous steps. + Outputs map[string]interface{} + // Step is the result of the current step. Empty when evaluating the "run" field + // but filled when evaluating the "outputs" field. + Step StepResult + // PreviousStep is the result of the previous step. Empty when there is no + // previous step. PreviousStep StepResult - Repository graphql.Repository + // Repository is the Sourcegraph repository in which the steps are executed. + Repository graphql.Repository } // ToFuncMap returns a template.FuncMap to access fields on the StepContext in a // text/template. func (stepCtx *StepContext) ToFuncMap() template.FuncMap { - return template.FuncMap{ - "join": func(list []string, sep string) string { - return strings.Join(list, sep) - }, - "split": func(s string, sep string) []string { - return strings.Split(s, sep) - }, - "previous_step": func() map[string]interface{} { - result := map[string]interface{}{ - "modified_files": stepCtx.PreviousStep.ModifiedFiles(), - "added_files": stepCtx.PreviousStep.AddedFiles(), - "deleted_files": stepCtx.PreviousStep.DeletedFiles(), - "renamed_files": stepCtx.PreviousStep.RenamedFiles(), - } + newStepResult := func(res *StepResult) map[string]interface{} { + m := map[string]interface{}{ + "modified_files": "", + "added_files": "", + "deleted_files": "", + "renamed_files": "", + "stdout": "", + "stderr": "", + } + if res == nil { + return m + } - if stepCtx.PreviousStep.Stdout != nil { - result["stdout"] = stepCtx.PreviousStep.Stdout.String() - } else { - result["stdout"] = "" - } + m["modified_files"] = res.ModifiedFiles() + m["added_files"] = res.AddedFiles() + m["deleted_files"] = res.DeletedFiles() + m["renamed_files"] = res.RenamedFiles() - if stepCtx.PreviousStep.Stderr != nil { - result["stderr"] = stepCtx.PreviousStep.Stderr.String() - } else { - result["stderr"] = "" - } + if res.Stdout != nil { + m["stdout"] = res.Stdout.String() + } + + if res.Stderr != nil { + m["stderr"] = res.Stderr.String() + } - return result + return m + } + + return template.FuncMap{ + "join": strings.Join, + "split": strings.Split, + "previous_step": func() map[string]interface{} { + return newStepResult(&stepCtx.PreviousStep) + }, + "step": func() map[string]interface{} { + return newStepResult(&stepCtx.Step) + }, + "outputs": func() map[string]interface{} { + return stepCtx.Outputs }, "repository": func() map[string]interface{} { return map[string]interface{}{ @@ -101,10 +121,10 @@ type StepResult struct { // StepChanges are the changes made to files by a previous step in a repository. type StepChanges struct { - Modified []string - Added []string - Deleted []string - Renamed []string + Modified []string `json:"modified"` + Added []string `json:"added"` + Deleted []string `json:"deleted"` + Renamed []string `json:"renamed"` } // ModifiedFiles returns the files modified by a step. @@ -139,6 +159,64 @@ func (r StepResult) RenamedFiles() []string { return []string{} } +// ChangesetTemplateContext represents the contextual information available +// when rendering a field of the ChangesetTemplate as a template. +type ChangesetTemplateContext struct { + // Steps are the changes made by all steps that were executed. + Steps *StepChanges + + // Outputs are the outputs defined and initialized by the steps. + Outputs map[string]interface{} + + // Repository is the repository in which the steps were executed. + Repository graphql.Repository +} + +// ToFuncMap returns a template.FuncMap to access fields on the StepContext in a +// text/template. +func (tmplCtx *ChangesetTemplateContext) ToFuncMap() template.FuncMap { + return template.FuncMap{ + "join": strings.Join, + "split": strings.Split, + "repository": func() map[string]interface{} { + return map[string]interface{}{ + "search_result_paths": tmplCtx.Repository.SearchResultPaths(), + "name": tmplCtx.Repository.Name, + } + }, + "outputs": func() map[string]interface{} { + return tmplCtx.Outputs + }, + "steps": func() map[string]interface{} { + // Wrap the *StepChanges in a StepResult so we can use nil-safe + // methods. + res := StepResult{files: tmplCtx.Steps} + + return map[string]interface{}{ + "modified_files": res.ModifiedFiles(), + "added_files": res.AddedFiles(), + "deleted_files": res.DeletedFiles(), + "renamed_files": res.RenamedFiles(), + } + }, + } +} + +func renderChangesetTemplateField(name, tmpl string, tmplCtx *ChangesetTemplateContext) (string, error) { + var out bytes.Buffer + + t, err := template.New(name).Delims("${{", "}}").Funcs(tmplCtx.ToFuncMap()).Parse(tmpl) + if err != nil { + return "", err + } + + if err := t.Execute(&out, tmplCtx); err != nil { + return "", err + } + + return out.String(), nil +} + func parseGitStatus(out []byte) (StepChanges, error) { result := StepChanges{} diff --git a/internal/campaigns/templating_test.go b/internal/campaigns/templating_test.go index 5beeb28d30..ace3ff2997 100644 --- a/internal/campaigns/templating_test.go +++ b/internal/campaigns/templating_test.go @@ -6,6 +6,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/sourcegraph/src-cli/internal/campaigns/graphql" + "gopkg.in/yaml.v3" ) func TestParseGitStatus(t *testing.T) { @@ -33,7 +34,26 @@ R README.md -> README.markdown } } -func TestParsingAndRenderingTemplates(t *testing.T) { +const rawYaml = `dist: release +env: + - GO111MODULE=on + - CGO_ENABLED=0 +before: + hooks: + - go mod download + - go mod tidy + - go generate ./schema +` + +func TestRenderStepTemplate(t *testing.T) { + // To avoid bugs due to differences between test setup and actual code, we + // do the actual parsing of YAML here to get an interface{} which we'll put + // in the StepContext. + var parsedYaml interface{} + if err := yaml.Unmarshal([]byte(rawYaml), &parsedYaml); err != nil { + t.Fatalf("failed to parse YAML: %s", err) + } + stepCtx := &StepContext{ PreviousStep: StepResult{ files: &StepChanges{ @@ -42,8 +62,22 @@ func TestParsingAndRenderingTemplates(t *testing.T) { Deleted: []string{".DS_Store"}, Renamed: []string{"new-filename.txt"}, }, - Stdout: bytes.NewBufferString("this is stdout"), - Stderr: bytes.NewBufferString("this is stderr"), + Stdout: bytes.NewBufferString("this is previous step's stdout"), + Stderr: bytes.NewBufferString("this is previous step's stderr"), + }, + Outputs: map[string]interface{}{ + "lastLine": "lastLine is this", + "project": parsedYaml, + }, + Step: StepResult{ + files: &StepChanges{ + Modified: []string{"step-go.mod"}, + Added: []string{"step-main.go.swp"}, + Deleted: []string{"step-.DS_Store"}, + Renamed: []string{"step-new-filename.txt"}, + }, + Stdout: bytes.NewBufferString("this is current step's stdout"), + Stderr: bytes.NewBufferString("this is current step's stderr"), }, Repository: graphql.Repository{ Name: "github.com/sourcegraph/src-cli", @@ -61,70 +95,62 @@ func TestParsingAndRenderingTemplates(t *testing.T) { want string }{ { - name: "previous step file changes", + name: "lower-case aliases", stepCtx: stepCtx, - run: `${{ .PreviousStep.ModifiedFiles }} -${{ .PreviousStep.AddedFiles }} -${{ .PreviousStep.DeletedFiles }} -${{ .PreviousStep.RenamedFiles }} + run: `${{ repository.search_result_paths }} +${{ repository.name }} +${{ previous_step.modified_files }} +${{ previous_step.added_files }} +${{ previous_step.deleted_files }} +${{ previous_step.renamed_files }} +${{ previous_step.stdout }} +${{ previous_step.stderr}} +${{ outputs.lastLine }} +${{ index outputs.project.env 1 }} +${{ step.modified_files }} +${{ step.added_files }} +${{ step.deleted_files }} +${{ step.renamed_files }} +${{ step.stdout}} +${{ step.stderr}} `, - want: `[go.mod] + want: `README.md main.go +github.com/sourcegraph/src-cli +[go.mod] [main.go.swp] [.DS_Store] [new-filename.txt] +this is previous step's stdout +this is previous step's stderr +lastLine is this +CGO_ENABLED=0 +[step-go.mod] +[step-main.go.swp] +[step-.DS_Store] +[step-new-filename.txt] +this is current step's stdout +this is current step's stderr `, - }, - { - name: "previous step output", - stepCtx: stepCtx, - run: `${{ .PreviousStep.Stdout }} ${{ .PreviousStep.Stderr }}`, - want: `this is stdout this is stderr`, - }, - { - name: "repository name", - stepCtx: stepCtx, - run: `${{ .Repository.Name }}`, - want: `github.com/sourcegraph/src-cli`, - }, - { - name: "search result paths", - stepCtx: stepCtx, - run: `${{ .Repository.SearchResultPaths }}`, - want: `README.md main.go`, - }, - { - name: "lower-case aliases", - stepCtx: stepCtx, - run: `${{ repository.search_result_paths }} - ${{ repository.name }} - ${{ previous_step.modified_files }} - ${{ previous_step.added_files }} - ${{ previous_step.deleted_files }} - ${{ previous_step.renamed_files }} - ${{ previous_step.stdout }} - ${{ previous_step.stderr}} - `, - want: `README.md main.go - github.com/sourcegraph/src-cli - [go.mod] - [main.go.swp] - [.DS_Store] - [new-filename.txt] - this is stdout - this is stderr - `, }, { name: "empty context", stepCtx: &StepContext{}, - run: `${{ .Repository.SearchResultPaths }} -${{ .Repository.Name }} + run: `${{ repository.search_result_paths }} +${{ repository.name }} ${{ previous_step.modified_files }} ${{ previous_step.added_files }} ${{ previous_step.deleted_files }} ${{ previous_step.renamed_files }} ${{ previous_step.stdout }} ${{ previous_step.stderr}} +${{ outputs.lastLine }} +${{ outputs.project }} +${{ step.modified_files }} +${{ step.added_files }} +${{ step.deleted_files }} +${{ step.renamed_files }} +${{ step.stdout}} +${{ step.stderr}} `, want: ` @@ -134,46 +160,124 @@ ${{ previous_step.stderr}} [] + + +[] +[] +[] +[] + + `, }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var out bytes.Buffer + + err := renderStepTemplate("testing", tc.run, &out, tc.stepCtx) + if err != nil { + t.Fatal(err) + } + + if out.String() != tc.want { + t.Fatalf("wrong output:\n%s", cmp.Diff(tc.want, out.String())) + } + }) + } +} + +func TestRenderChangesetTemplateField(t *testing.T) { + // To avoid bugs due to differences between test setup and actual code, we + // do the actual parsing of YAML here to get an interface{} which we'll put + // in the StepContext. + var parsedYaml interface{} + if err := yaml.Unmarshal([]byte(rawYaml), &parsedYaml); err != nil { + t.Fatalf("failed to parse YAML: %s", err) + } + + tmplCtx := &ChangesetTemplateContext{ + Outputs: map[string]interface{}{ + "lastLine": "lastLine is this", + "project": parsedYaml, + }, + Repository: graphql.Repository{ + Name: "github.com/sourcegraph/src-cli", + FileMatches: map[string]bool{ + "README.md": true, + "main.go": true, + }, + }, + Steps: &StepChanges{ + Modified: []string{"modified-file.txt"}, + Added: []string{"added-file.txt"}, + Deleted: []string{"deleted-file.txt"}, + Renamed: []string{"renamed-file.txt"}, + }, + } + + tests := []struct { + name string + tmplCtx *ChangesetTemplateContext + run string + want string + }{ { - name: "empty context and aliases", - stepCtx: &StepContext{}, + name: "lower-case aliases", + tmplCtx: tmplCtx, run: `${{ repository.search_result_paths }} ${{ repository.name }} -${{ previous_step.modified_files }} -${{ previous_step.added_files }} -${{ previous_step.deleted_files }} -${{ previous_step.renamed_files }} -${{ previous_step.stdout }} -${{ previous_step.stderr}} +${{ outputs.lastLine }} +${{ index outputs.project.env 1 }} +${{ steps.modified_files }} +${{ steps.added_files }} +${{ steps.deleted_files }} +${{ steps.renamed_files }} +`, + want: `README.md main.go +github.com/sourcegraph/src-cli +lastLine is this +CGO_ENABLED=0 +[modified-file.txt] +[added-file.txt] +[deleted-file.txt] +[renamed-file.txt] +`, + }, + { + name: "empty context", + tmplCtx: &ChangesetTemplateContext{}, + run: `${{ repository.search_result_paths }} +${{ repository.name }} +${{ outputs.lastLine }} +${{ outputs.project }} +${{ steps.modified_files }} +${{ steps.added_files }} +${{ steps.deleted_files }} +${{ steps.renamed_files }} `, want: ` + + [] [] [] [] - - `, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - parsed, err := parseAsTemplate("testing", tc.run, tc.stepCtx) + out, err := renderChangesetTemplateField("testing", tc.run, tc.tmplCtx) if err != nil { t.Fatal(err) } - var out bytes.Buffer - if err := parsed.Execute(&out, tc.stepCtx); err != nil { - t.Fatalf("executing template failed: %s", err) - } - - if out.String() != tc.want { - t.Fatalf("wrong output:\n%s", cmp.Diff(tc.want, out.String())) + if out != tc.want { + t.Fatalf("wrong output:\n%s", cmp.Diff(tc.want, out)) } }) } diff --git a/schema/campaign_spec.schema.json b/schema/campaign_spec.schema.json index b57dc52bcd..bd2bfbb828 100644 --- a/schema/campaign_spec.schema.json +++ b/schema/campaign_spec.schema.json @@ -75,6 +75,26 @@ "description": "The Docker image used to launch the Docker container in which the shell command is run.", "examples": ["alpine:3"] }, + "outputs": { + "type": "object", + "description": "Output variables of this step that can be referenced in the changesetTemplate or other steps via outputs.", + "additionalProperties": { + "type": "object", + "required": ["value"], + "properties": { + "value": { + "type": "string", + "description": "The value of the output, which can be a template string.", + "examples": ["hello world", "${{ step.stdout }}", "${{ repository.name }}" ] + }, + "format": { + "type": "string", + "description": "The expected format of the output. If set, the output is being parsed in that format before being stored in the var. If not set, 'text' is assumed to the format.", + "enum": ["json", "yaml", "text"] + } + } + } + }, "env": { "description": "Environment variables to set in the step environment.", "oneOf": [ diff --git a/schema/campaign_spec_stringdata.go b/schema/campaign_spec_stringdata.go index 131ed382c9..6d850af9ea 100644 --- a/schema/campaign_spec_stringdata.go +++ b/schema/campaign_spec_stringdata.go @@ -80,6 +80,26 @@ const CampaignSpecJSON = `{ "description": "The Docker image used to launch the Docker container in which the shell command is run.", "examples": ["alpine:3"] }, + "outputs": { + "type": "object", + "description": "Output variables of this step that can be referenced in the changesetTemplate or other steps via outputs.", + "additionalProperties": { + "type": "object", + "required": ["value"], + "properties": { + "value": { + "type": "string", + "description": "The value of the output, which can be a template string.", + "examples": ["hello world", "${{ step.stdout }}", "${{ repository.name }}" ] + }, + "format": { + "type": "string", + "description": "The expected format of the output. If set, the output is being parsed in that format before being stored in the var. If not set, 'text' is assumed to the format.", + "enum": ["json", "yaml", "text"] + } + } + } + }, "env": { "description": "Environment variables to set in the step environment.", "oneOf": [