campaigns: allow env usage in campaign steps

CHANGELOG.md

@@ -13,6 +13,8 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Added
 
+- Campaign steps may now include environment variables from outside of the campaign spec using [array syntax](http://docs.sourcegraph.com/campaigns/references/campaign_spec_yaml_reference#environment-array). [#392](https://github.com/sourcegraph/src-cli/pull/392)
+
 ### Changed
 
 ### Fixed

go.mod

@@ -19,7 +19,7 @@ require (
 	github.com/olekukonko/tablewriter v0.0.4 // indirect
 	github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4
 	github.com/pkg/errors v0.9.1
-	github.com/sourcegraph/campaignutils v0.0.0-20201016010611-63eb2bca27ad
+	github.com/sourcegraph/campaignutils v0.0.0-20201124055807-2f9cfa9317e2
 	github.com/sourcegraph/codeintelutils v0.0.0-20201118031531-b82ba3167b30
 	github.com/sourcegraph/go-diff v0.6.1
 	github.com/sourcegraph/jsonx v0.0.0-20200629203448-1a936bd500cf

go.sum

@@ -50,8 +50,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e/go.mod h1:TDJrrUr11Vxrven61rcy3hJMUqaf/CLWYhHNPmT14Lk=
 github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041/go.mod h1:N5mDOmsrJOB+vfqUK+7DmDyjhSLIIBnXo9lvZJj3MWQ=
-github.com/sourcegraph/campaignutils v0.0.0-20201016010611-63eb2bca27ad h1:HeSWFpxau4Jqk0s4yEhOdep+KYrJDm0497uhb/hnsgU=
-github.com/sourcegraph/campaignutils v0.0.0-20201016010611-63eb2bca27ad/go.mod h1:xm6i78Mk2t4DBLQDqEFc/3x6IPf7yYZCgbNaTQGhJHA=
+github.com/sourcegraph/campaignutils v0.0.0-20201124055807-2f9cfa9317e2 h1:MJu/6WzWdPegzYnZLb04IS0u4VyUpPIAHQyWT5i2vR8=
+github.com/sourcegraph/campaignutils v0.0.0-20201124055807-2f9cfa9317e2/go.mod h1:xm6i78Mk2t4DBLQDqEFc/3x6IPf7yYZCgbNaTQGhJHA=
 github.com/sourcegraph/codeintelutils v0.0.0-20201118031531-b82ba3167b30 h1:HrRrPyskdkHc6MqQS3ehH+DSraFnOhtvBWQ6AzEJC1o=
 github.com/sourcegraph/codeintelutils v0.0.0-20201118031531-b82ba3167b30/go.mod h1:HplI8gRslTrTUUsSYwu28hSOderix7m5dHNca7xBzeo=
 github.com/sourcegraph/go-diff v0.6.1 h1:hmA1LzxW0n1c3Q4YbrFgg4P99GSnebYa3x8gr0HZqLQ=

internal/campaigns/campaign_spec.go

@@ -3,6 +3,9 @@ package campaigns
 import (
 	"fmt"
 
+	"github.com/hashicorp/go-multierror"
+	"github.com/pkg/errors"
+	"github.com/sourcegraph/campaignutils/env"
 	"github.com/sourcegraph/campaignutils/overridable"
 	"github.com/sourcegraph/campaignutils/yaml"
 	"github.com/sourcegraph/src-cli/schema"
@@ -65,18 +68,31 @@ type OnQueryOrRepository struct {
 type Step struct {
 	Run       string            `json:"run,omitempty" yaml:"run"`
 	Container string            `json:"container,omitempty" yaml:"container"`
-	Env       map[string]string `json:"env,omitempty" yaml:"env"`
+	Env       env.Environment   `json:"env,omitempty" yaml:"env"`
 	Files     map[string]string `json:"files,omitempty" yaml:"files,omitempty"`
 
 	image string
 }
 
-func ParseCampaignSpec(data []byte) (*CampaignSpec, error) {
+func ParseCampaignSpec(data []byte, features featureFlags) (*CampaignSpec, error) {
 	var spec CampaignSpec
 	if err := yaml.UnmarshalValidate(schema.CampaignSpecJSON, data, &spec); err != nil {
 		return nil, err
 	}
 
+	if !features.allowArrayEnvironments {
+		var errs *multierror.Error
+		for i, step := range spec.Steps {
+			if !step.Env.IsStatic() {
+				errs = multierror.Append(errs, errors.Errorf("step %d includes one or more dynamic environment variables, which are unsupported in this Sourcegraph version", i+1))
+			}
+		}
+
+		if err := errs.ErrorOrNil(); err != nil {
+			return nil, err
+		}
+	}
+
 	return &spec, nil
 }
 

internal/campaigns/execution_cache.go

@@ -24,6 +24,41 @@ type ExecutionCacheKey struct {
 	*Task
 }
 
+// Key converts the key into a string form that can be used to uniquely identify
+// the cache key in a more concise form than the entire Task.
+func (key ExecutionCacheKey) Key() (string, error) {
+	// We have to resolve the step environments and include them in the cache
+	// key to ensure that the cache is properly invalidated when an environment
+	// variable changes.
+	//
+	// Note that we don't base the cache key on the entire global environment:
+	// if an unrelated environment variable changes, that's fine. We're only
+	// interested in the ones that actually make it into the step container.
+	global := os.Environ()
+	envs := make([]map[string]string, len(key.Task.Steps))
+	for i, step := range key.Task.Steps {
+		env, err := step.Env.Resolve(global)
+		if err != nil {
+			return "", errors.Wrapf(err, "resolving environment for step %d", i)
+		}
+		envs[i] = env
+	}
+
+	raw, err := json.Marshal(struct {
+		*Task
+		Environments []map[string]string
+	}{
+		Task:         key.Task,
+		Environments: envs,
+	})
+	if err != nil {
+		return "", err
+	}
+
+	hash := sha256.Sum256(raw)
+	return base64.RawURLEncoding.EncodeToString(hash[:16]), nil
+}
+
 type ExecutionCache interface {
 	Get(ctx context.Context, key ExecutionCacheKey) (result *ChangesetSpec, err error)
 	Set(ctx context.Context, key ExecutionCacheKey, result *ChangesetSpec) error
@@ -35,14 +70,11 @@ type ExecutionDiskCache struct {
 }
 
 func (c ExecutionDiskCache) cacheFilePath(key ExecutionCacheKey) (string, error) {
-	keyJSON, err := json.Marshal(key)
+	keyString, err := key.Key()
 	if err != nil {
-		return "", errors.Wrap(err, "Failed to marshal JSON when generating action cache key")
+		return "", errors.Wrap(err, "calculating execution cache key")
 	}
 
-	b := sha256.Sum256(keyJSON)
-	keyString := base64.RawURLEncoding.EncodeToString(b[:16])
-
 	return filepath.Join(c.Dir, keyString+".json"), nil
 }
 

internal/campaigns/execution_cache_test.go

@@ -0,0 +1,88 @@
+package campaigns
+
+import (
+	"os"
+	"testing"
+
+	"gopkg.in/yaml.v2"
+)
+
+const testExecutionCacheKeyEnv = "TEST_EXECUTION_CACHE_KEY_ENV"
+
+func TestExecutionCacheKey(t *testing.T) {
+	// Let's set up an array of steps that we can test with. One step will
+	// depend on an environment variable outside the spec.
+	var steps []Step
+	if err := yaml.Unmarshal([]byte(`
+- run: foo
+  env:
+    FOO: BAR
+
+- run: bar
+  env:
+    - FOO: BAR
+    - `+testExecutionCacheKeyEnv+`
+`), &steps); err != nil {
+		t.Fatal(err)
+	}
+
+	// And now we can set up a key to work with.
+	key := ExecutionCacheKey{&Task{Steps: steps}}
+
+	// All righty. Let's get ourselves a baseline cache key here.
+	initial, err := key.Key()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	// Let's set an unrelated environment variable and ensure we still have the
+	// same key.
+	if err := os.Setenv(testExecutionCacheKeyEnv+"_UNRELATED", "foo"); err != nil {
+		t.Fatal(err)
+	}
+	have, err := key.Key()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if string(initial) != string(have) {
+		t.Errorf("unexpected change in key: initial=%q have=%q", initial, have)
+	}
+
+	// Let's now set the environment variable referenced in the steps and verify
+	// that the cache key does change.
+	if err := os.Setenv(testExecutionCacheKeyEnv, "foo"); err != nil {
+		t.Fatal(err)
+	}
+	have, err = key.Key()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if string(initial) == string(have) {
+		t.Errorf("unexpected lack of change in key: %q", have)
+	}
+
+	// And, just to be sure, let's change it again.
+	if err := os.Setenv(testExecutionCacheKeyEnv, "bar"); err != nil {
+		t.Fatal(err)
+	}
+	again, err := key.Key()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if string(initial) == string(again) || string(have) == string(again) {
+		t.Errorf("unexpected lack of change in key: %q", again)
+	}
+
+	// Finally, if we unset the environment variable again, we should get a key
+	// that matches the initial key.
+	if err := os.Unsetenv(testExecutionCacheKeyEnv); err != nil {
+		t.Fatal(err)
+	}
+	have, err = key.Key()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if string(initial) != string(have) {
+		t.Errorf("unexpected change in key: initial=%q have=%q", initial, have)
+	}
+}

internal/campaigns/features.go

@@ -8,6 +8,7 @@ import (
 // featureFlags represent features that are only available on certain
 // Sourcegraph versions and we therefore have to detect at runtime.
 type featureFlags struct {
+	allowArrayEnvironments   bool
 	includeAutoAuthorDetails bool
 	useGzipCompression       bool
 }
@@ -18,6 +19,7 @@ func (ff *featureFlags) setFromVersion(version string) error {
 		constraint string
 		minDate    string
 	}{
+		{&ff.allowArrayEnvironments, ">= 3.23.0", "2020-11-24"},
 		{&ff.includeAutoAuthorDetails, ">= 3.20.0", "2020-09-10"},
 		{&ff.useGzipCompression, ">= 3.21.0", "2020-10-12"},
 	} {

internal/campaigns/features_test.go

@@ -2,6 +2,7 @@ package campaigns
 
 func featuresAllEnabled() featureFlags {
 	return featureFlags{
+		allowArrayEnvironments:   true,
 		includeAutoAuthorDetails: true,
 		useGzipCompression:       true,
 	}

internal/campaigns/run_steps.go

@@ -168,10 +168,16 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor
 			filesToMount[name] = fp
 		}
 
+		// 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")
+		}
+
 		// Render the step.Env variables as templates.
-		env, err := renderStepEnv(step.Env, &stepContext)
+		env, err := renderStepEnv(stepEnv, &stepContext)
 		if err != nil {
-			return nil, errors.Wrap(err, "parsing step files")
+			return nil, errors.Wrap(err, "parsing step environment")
 		}
 
 		reportProgress(runScript.String())

internal/campaigns/service.go

@@ -293,7 +293,7 @@ func (svc *Service) ParseCampaignSpec(in io.Reader) (*CampaignSpec, string, erro
 		return nil, "", errors.Wrap(err, "reading campaign spec")
 	}
 
-	spec, err := ParseCampaignSpec(data)
+	spec, err := ParseCampaignSpec(data, svc.features)
 	if err != nil {
 		return nil, "", errors.Wrap(err, "parsing campaign spec")
 	}

schema/campaign_spec.schema.json

@@ -76,11 +76,34 @@
             "examples": ["alpine:3"]
           },
           "env": {
-            "type": "object",
-            "description": "Environment variables to set in the environment when running this command.",
-            "additionalProperties": {
-              "type": "string"
-            }
+            "description": "Environment variables to set in the step environment.",
+            "oneOf": [
+              {
+                "type": "object",
+                "description": "Environment variables to set in the step environment.",
+                "additionalProperties": {
+                  "type": "string"
+                }
+              },
+              {
+                "type": "array",
+                "items": {
+                  "oneOf": [
+                    {
+                      "type": "string",
+                      "description": "An environment variable to set in the step environment: the value will be passed through from the environment src is running within."
+                    },
+                    {
+                      "type": "object",
+                      "description": "An environment variable to set in the step environment: the value will be passed through from the environment src is running within.",
+                      "additionalProperties": { "type": "string" },
+                      "minProperties": 1,
+                      "maxProperties": 1
+                    }
+                  ]
+                }
+              }
+            ]
           },
           "files": {
             "type": "object",