Enable dynamic changesetTemplates by adding templating & steps outputs (#424)

* Add templating to changesetTemplate and steps.outputs

* Fix parsing of outputs

* Remove leftovers in campaign spec schema

* Remove debug output

* Rename test function

* Add a test for renderChangesetTemplateField

* Update docs/examples in campaign spec schema

* Get dynamic changeset templates working with cache

* Simplify ExecutionCache by using ExecutionResult

* Change interface of runSteps to return ExecutionResult

* Add 'steps' to changesetTemplate template variables

* Support templating in changesetTemplate.author fields

* Fix doc comment

* Only use yaml.v3 in internal/campaigns

* Add tests for ExecutionCacheTest

* Add proper backwards-compatibility to ExecutionDiskCache

* Remove unneeded code

* Add changelog entry

* Add comment about lossiness of cache conversion

Author: mrnugget

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

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
 )
 

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"`
 }

internal/campaigns/execution_cache.go

@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -62,73 +63,183 @@ 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
 }
 
 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
 }