Skip to content

Commit 242d54f

Browse files
authored
campaigns: allow env usage in campaign steps (#392)
This is the src-cli component of sourcegraph/sourcegraph#15822. There's also a new feature flag in here to maintain backward compatibility with older Sourcegraph versions, at the cost of extra work when parsing the campaign spec. I think that's the right tradeoff.
1 parent 040ea13 commit 242d54f

File tree

12 files changed

+216
-23
lines changed

12 files changed

+216
-23
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ All notable changes to `src-cli` are documented in this file.
1313

1414
### Added
1515

16+
- 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)
17+
1618
### Changed
1719

1820
### Fixed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ require (
1919
github.com/olekukonko/tablewriter v0.0.4 // indirect
2020
github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4
2121
github.com/pkg/errors v0.9.1
22-
github.com/sourcegraph/campaignutils v0.0.0-20201016010611-63eb2bca27ad
22+
github.com/sourcegraph/campaignutils v0.0.0-20201124055807-2f9cfa9317e2
2323
github.com/sourcegraph/codeintelutils v0.0.0-20201118031531-b82ba3167b30
2424
github.com/sourcegraph/go-diff v0.6.1
2525
github.com/sourcegraph/jsonx v0.0.0-20200629203448-1a936bd500cf

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
5050
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
5151
github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e/go.mod h1:TDJrrUr11Vxrven61rcy3hJMUqaf/CLWYhHNPmT14Lk=
5252
github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041/go.mod h1:N5mDOmsrJOB+vfqUK+7DmDyjhSLIIBnXo9lvZJj3MWQ=
53-
github.com/sourcegraph/campaignutils v0.0.0-20201016010611-63eb2bca27ad h1:HeSWFpxau4Jqk0s4yEhOdep+KYrJDm0497uhb/hnsgU=
54-
github.com/sourcegraph/campaignutils v0.0.0-20201016010611-63eb2bca27ad/go.mod h1:xm6i78Mk2t4DBLQDqEFc/3x6IPf7yYZCgbNaTQGhJHA=
53+
github.com/sourcegraph/campaignutils v0.0.0-20201124055807-2f9cfa9317e2 h1:MJu/6WzWdPegzYnZLb04IS0u4VyUpPIAHQyWT5i2vR8=
54+
github.com/sourcegraph/campaignutils v0.0.0-20201124055807-2f9cfa9317e2/go.mod h1:xm6i78Mk2t4DBLQDqEFc/3x6IPf7yYZCgbNaTQGhJHA=
5555
github.com/sourcegraph/codeintelutils v0.0.0-20201118031531-b82ba3167b30 h1:HrRrPyskdkHc6MqQS3ehH+DSraFnOhtvBWQ6AzEJC1o=
5656
github.com/sourcegraph/codeintelutils v0.0.0-20201118031531-b82ba3167b30/go.mod h1:HplI8gRslTrTUUsSYwu28hSOderix7m5dHNca7xBzeo=
5757
github.com/sourcegraph/go-diff v0.6.1 h1:hmA1LzxW0n1c3Q4YbrFgg4P99GSnebYa3x8gr0HZqLQ=

internal/campaigns/campaign_spec.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ package campaigns
33
import (
44
"fmt"
55

6+
"github.com/hashicorp/go-multierror"
7+
"github.com/pkg/errors"
8+
"github.com/sourcegraph/campaignutils/env"
69
"github.com/sourcegraph/campaignutils/overridable"
710
"github.com/sourcegraph/campaignutils/yaml"
811
"github.com/sourcegraph/src-cli/schema"
@@ -65,18 +68,31 @@ type OnQueryOrRepository struct {
6568
type Step struct {
6669
Run string `json:"run,omitempty" yaml:"run"`
6770
Container string `json:"container,omitempty" yaml:"container"`
68-
Env map[string]string `json:"env,omitempty" yaml:"env"`
71+
Env env.Environment `json:"env,omitempty" yaml:"env"`
6972
Files map[string]string `json:"files,omitempty" yaml:"files,omitempty"`
7073

7174
image string
7275
}
7376

74-
func ParseCampaignSpec(data []byte) (*CampaignSpec, error) {
77+
func ParseCampaignSpec(data []byte, features featureFlags) (*CampaignSpec, error) {
7578
var spec CampaignSpec
7679
if err := yaml.UnmarshalValidate(schema.CampaignSpecJSON, data, &spec); err != nil {
7780
return nil, err
7881
}
7982

83+
if !features.allowArrayEnvironments {
84+
var errs *multierror.Error
85+
for i, step := range spec.Steps {
86+
if !step.Env.IsStatic() {
87+
errs = multierror.Append(errs, errors.Errorf("step %d includes one or more dynamic environment variables, which are unsupported in this Sourcegraph version", i+1))
88+
}
89+
}
90+
91+
if err := errs.ErrorOrNil(); err != nil {
92+
return nil, err
93+
}
94+
}
95+
8096
return &spec, nil
8197
}
8298

internal/campaigns/execution_cache.go

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,41 @@ type ExecutionCacheKey struct {
2424
*Task
2525
}
2626

27+
// Key converts the key into a string form that can be used to uniquely identify
28+
// the cache key in a more concise form than the entire Task.
29+
func (key ExecutionCacheKey) Key() (string, error) {
30+
// We have to resolve the step environments and include them in the cache
31+
// key to ensure that the cache is properly invalidated when an environment
32+
// variable changes.
33+
//
34+
// Note that we don't base the cache key on the entire global environment:
35+
// if an unrelated environment variable changes, that's fine. We're only
36+
// interested in the ones that actually make it into the step container.
37+
global := os.Environ()
38+
envs := make([]map[string]string, len(key.Task.Steps))
39+
for i, step := range key.Task.Steps {
40+
env, err := step.Env.Resolve(global)
41+
if err != nil {
42+
return "", errors.Wrapf(err, "resolving environment for step %d", i)
43+
}
44+
envs[i] = env
45+
}
46+
47+
raw, err := json.Marshal(struct {
48+
*Task
49+
Environments []map[string]string
50+
}{
51+
Task: key.Task,
52+
Environments: envs,
53+
})
54+
if err != nil {
55+
return "", err
56+
}
57+
58+
hash := sha256.Sum256(raw)
59+
return base64.RawURLEncoding.EncodeToString(hash[:16]), nil
60+
}
61+
2762
type ExecutionCache interface {
2863
Get(ctx context.Context, key ExecutionCacheKey) (result *ChangesetSpec, err error)
2964
Set(ctx context.Context, key ExecutionCacheKey, result *ChangesetSpec) error
@@ -35,14 +70,11 @@ type ExecutionDiskCache struct {
3570
}
3671

3772
func (c ExecutionDiskCache) cacheFilePath(key ExecutionCacheKey) (string, error) {
38-
keyJSON, err := json.Marshal(key)
73+
keyString, err := key.Key()
3974
if err != nil {
40-
return "", errors.Wrap(err, "Failed to marshal JSON when generating action cache key")
75+
return "", errors.Wrap(err, "calculating execution cache key")
4176
}
4277

43-
b := sha256.Sum256(keyJSON)
44-
keyString := base64.RawURLEncoding.EncodeToString(b[:16])
45-
4678
return filepath.Join(c.Dir, keyString+".json"), nil
4779
}
4880

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package campaigns
2+
3+
import (
4+
"os"
5+
"testing"
6+
7+
"gopkg.in/yaml.v2"
8+
)
9+
10+
const testExecutionCacheKeyEnv = "TEST_EXECUTION_CACHE_KEY_ENV"
11+
12+
func TestExecutionCacheKey(t *testing.T) {
13+
// Let's set up an array of steps that we can test with. One step will
14+
// depend on an environment variable outside the spec.
15+
var steps []Step
16+
if err := yaml.Unmarshal([]byte(`
17+
- run: foo
18+
env:
19+
FOO: BAR
20+
21+
- run: bar
22+
env:
23+
- FOO: BAR
24+
- `+testExecutionCacheKeyEnv+`
25+
`), &steps); err != nil {
26+
t.Fatal(err)
27+
}
28+
29+
// And now we can set up a key to work with.
30+
key := ExecutionCacheKey{&Task{Steps: steps}}
31+
32+
// All righty. Let's get ourselves a baseline cache key here.
33+
initial, err := key.Key()
34+
if err != nil {
35+
t.Errorf("unexpected error: %v", err)
36+
}
37+
38+
// Let's set an unrelated environment variable and ensure we still have the
39+
// same key.
40+
if err := os.Setenv(testExecutionCacheKeyEnv+"_UNRELATED", "foo"); err != nil {
41+
t.Fatal(err)
42+
}
43+
have, err := key.Key()
44+
if err != nil {
45+
t.Errorf("unexpected error: %v", err)
46+
}
47+
if string(initial) != string(have) {
48+
t.Errorf("unexpected change in key: initial=%q have=%q", initial, have)
49+
}
50+
51+
// Let's now set the environment variable referenced in the steps and verify
52+
// that the cache key does change.
53+
if err := os.Setenv(testExecutionCacheKeyEnv, "foo"); err != nil {
54+
t.Fatal(err)
55+
}
56+
have, err = key.Key()
57+
if err != nil {
58+
t.Errorf("unexpected error: %v", err)
59+
}
60+
if string(initial) == string(have) {
61+
t.Errorf("unexpected lack of change in key: %q", have)
62+
}
63+
64+
// And, just to be sure, let's change it again.
65+
if err := os.Setenv(testExecutionCacheKeyEnv, "bar"); err != nil {
66+
t.Fatal(err)
67+
}
68+
again, err := key.Key()
69+
if err != nil {
70+
t.Errorf("unexpected error: %v", err)
71+
}
72+
if string(initial) == string(again) || string(have) == string(again) {
73+
t.Errorf("unexpected lack of change in key: %q", again)
74+
}
75+
76+
// Finally, if we unset the environment variable again, we should get a key
77+
// that matches the initial key.
78+
if err := os.Unsetenv(testExecutionCacheKeyEnv); err != nil {
79+
t.Fatal(err)
80+
}
81+
have, err = key.Key()
82+
if err != nil {
83+
t.Errorf("unexpected error: %v", err)
84+
}
85+
if string(initial) != string(have) {
86+
t.Errorf("unexpected change in key: initial=%q have=%q", initial, have)
87+
}
88+
}

internal/campaigns/features.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
// featureFlags represent features that are only available on certain
99
// Sourcegraph versions and we therefore have to detect at runtime.
1010
type featureFlags struct {
11+
allowArrayEnvironments bool
1112
includeAutoAuthorDetails bool
1213
useGzipCompression bool
1314
}
@@ -18,6 +19,7 @@ func (ff *featureFlags) setFromVersion(version string) error {
1819
constraint string
1920
minDate string
2021
}{
22+
{&ff.allowArrayEnvironments, ">= 3.23.0", "2020-11-24"},
2123
{&ff.includeAutoAuthorDetails, ">= 3.20.0", "2020-09-10"},
2224
{&ff.useGzipCompression, ">= 3.21.0", "2020-10-12"},
2325
} {

internal/campaigns/features_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package campaigns
22

33
func featuresAllEnabled() featureFlags {
44
return featureFlags{
5+
allowArrayEnvironments: true,
56
includeAutoAuthorDetails: true,
67
useGzipCompression: true,
78
}

internal/campaigns/run_steps.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,16 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor
168168
filesToMount[name] = fp
169169
}
170170

171+
// Resolve step.Env given the current environment.
172+
stepEnv, err := step.Env.Resolve(os.Environ())
173+
if err != nil {
174+
return nil, errors.Wrap(err, "resolving step environment")
175+
}
176+
171177
// Render the step.Env variables as templates.
172-
env, err := renderStepEnv(step.Env, &stepContext)
178+
env, err := renderStepEnv(stepEnv, &stepContext)
173179
if err != nil {
174-
return nil, errors.Wrap(err, "parsing step files")
180+
return nil, errors.Wrap(err, "parsing step environment")
175181
}
176182

177183
reportProgress(runScript.String())

internal/campaigns/service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func (svc *Service) ParseCampaignSpec(in io.Reader) (*CampaignSpec, string, erro
293293
return nil, "", errors.Wrap(err, "reading campaign spec")
294294
}
295295

296-
spec, err := ParseCampaignSpec(data)
296+
spec, err := ParseCampaignSpec(data, svc.features)
297297
if err != nil {
298298
return nil, "", errors.Wrap(err, "parsing campaign spec")
299299
}

0 commit comments

Comments
 (0)