Skip to content

Commit

Permalink
Make this a flag to the Interpolate function
Browse files Browse the repository at this point in the history 
Prefer a simpler, breaking change, over an unnecessary backwards compatibility change
  • Loading branch information
@patrobinson
patrobinson committed Apr 24, 2024
1 parent 34c1a81 commit eb09bcc
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 34 deletions.
2 changes: 1 addition & 1 deletion interpolate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func TestInterpolator(t *testing.T) {
t.Parallel()

runtimeEnv := env.New(env.CaseSensitive(tc.caseSensitive), env.FromMap(tc.runtimeEnv))
err := tc.input.Interpolate(runtimeEnv)
err := tc.input.Interpolate(runtimeEnv, false)
assert.NilError(t, err)
if diff := diffPipeline(tc.input, tc.expected); diff != "" {
t.Errorf("parsed pipeline diff (-got +want):\n%s", diff)
Expand Down
15 changes: 1 addition & 14 deletions parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@ import (
// Options are functional options for creating a new Env.
type Options func(*Pipeline)

// By default if an environment variable exists in both the runtime and pipeline env
// we will substitute with the pipeline env IF the pipeline env is defined first.
// Setting this option to true instead preferres the runtime environment to pipeline
// environment variables when both are defined.
func RuntimeEnvPropagation(preferRuntimeEnv bool) Options {
return func(e *Pipeline) {
e.preferRuntimeEnv = preferRuntimeEnv
}
}

// Parse parses a pipeline. It does not apply interpolation.
// Warnings are passed through the err return:
//
Expand All @@ -34,7 +24,7 @@ func RuntimeEnvPropagation(preferRuntimeEnv bool) Options {
// return err
// }
// // Use p
func Parse(src io.Reader, opts ...Options) (*Pipeline, error) {
func Parse(src io.Reader) (*Pipeline, error) {
// First get yaml.v3 to give us a raw document (*yaml.Node).
n := new(yaml.Node)
if err := yaml.NewDecoder(src).Decode(n); err != nil {
Expand All @@ -49,9 +39,6 @@ func Parse(src io.Reader, opts ...Options) (*Pipeline, error) {
// configuration. Then decode _that_ into a pipeline.
p := new(Pipeline)

for _, o := range opts {
o(p)
}
return p, ordered.Unmarshal(n, p)
}

Expand Down
2 changes: 1 addition & 1 deletion parser_matrix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ steps:
if err != nil {
t.Fatalf("Parse(%q) error = %v", test.input, err)
}
if err := got.Interpolate(nil); err != nil {
if err := got.Interpolate(nil, false); err != nil {
t.Fatalf("Pipeline.Interpolate(nil) = %v", err)
}
if diff := diffPipeline(got, test.want); diff != "" {
Expand Down
23 changes: 11 additions & 12 deletions parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ import (
"github.com/buildkite/go-pipeline/ordered"
"github.com/buildkite/go-pipeline/warning"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"gopkg.in/yaml.v3"
)

func ptr[T any](x T) *T { return &x }

func diffPipeline(got *Pipeline, want *Pipeline) string {
return cmp.Diff(got, want, cmpopts.IgnoreUnexported(*got), cmp.Comparer(ordered.EqualSS), cmp.Comparer(ordered.EqualSA))
return cmp.Diff(got, want, cmp.Comparer(ordered.EqualSS), cmp.Comparer(ordered.EqualSA))
}

func TestParserParsesYAML(t *testing.T) {
Expand All @@ -29,7 +28,7 @@ func TestParserParsesYAML(t *testing.T) {
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, false); err != nil {
t.Fatalf("p.Interpolate(%v) error = %v", runtimeEnv, err)
}

Expand Down Expand Up @@ -166,12 +165,12 @@ steps:

for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
got, err := Parse(test.input, RuntimeEnvPropagation(test.runtimePreferred))
got, err := Parse(test.input)
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
runtimeEnv := env.New(env.FromMap(test.runtimeEnv))
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, test.runtimePreferred); err != nil {
t.Fatalf("p.Interpolate(nil) error = %v", err)
}

Expand Down Expand Up @@ -668,7 +667,7 @@ steps:
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, false); err != nil {
t.Fatalf("p.Interpolate(%v) error = %v", runtimeEnv, err)
}

Expand Down Expand Up @@ -847,7 +846,7 @@ func TestParserParsesJSON(t *testing.T) {
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, false); err != nil {
t.Fatalf("p.Interpolate(%v) error = %v", runtimeEnv, err)
}

Expand Down Expand Up @@ -896,7 +895,7 @@ func TestParserParsesJSONArrays(t *testing.T) {
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, false); err != nil {
t.Fatalf("p.Interpolate(%v) error = %v", runtimeEnv, err)
}

Expand Down Expand Up @@ -1465,7 +1464,7 @@ func TestParserInterpolatesKeysAsWellAsValues(t *testing.T) {
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, false); err != nil {
t.Fatalf("p.Interpolate(%v) error = %v", runtimeEnv, err)
}
want := &Pipeline{
Expand Down Expand Up @@ -1501,7 +1500,7 @@ steps:
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, false); err != nil {
t.Fatalf("p.Interpolate(%v) error = %v", runtimeEnv, err)
}
want := &Pipeline{
Expand Down Expand Up @@ -1547,7 +1546,7 @@ func TestParserLoadsGlobalEnvBlockFirst(t *testing.T) {
if err != nil {
t.Fatalf("Parse(input) error = %v", err)
}
if err := got.Interpolate(runtimeEnv); err != nil {
if err := got.Interpolate(runtimeEnv, false); err != nil {
t.Fatalf("p.Interpolate(%v) error = %v", runtimeEnv, err)
}
want := &Pipeline{
Expand Down Expand Up @@ -1862,7 +1861,7 @@ steps:
t.Fatalf("Parse(input) error = %v", err)
}
if test.interpolate {
if err := got.Interpolate(nil); err != nil {
if err := got.Interpolate(nil, false); err != nil {
t.Fatalf("p.Interpolate(nil) error = %v", err)
}
}
Expand Down
16 changes: 10 additions & 6 deletions pipeline.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ type Pipeline struct {
// RemainingFields stores any other top-level mapping items so they at least
// survive an unmarshal-marshal round-trip.
RemainingFields map[string]any `yaml:",inline"`

preferRuntimeEnv bool `yaml:",omitempty"`
}

// MarshalJSON marshals a pipeline to JSON. Special handling is needed because
Expand Down Expand Up @@ -81,14 +79,20 @@ type InterpolationEnv interface {
// - Interpolate pipeline.Env and copy the results into interpolationEnv, provided they don't
// conflict, to apply later.
// - Interpolate any string value in the rest of the pipeline.
func (p *Pipeline) Interpolate(interpolationEnv InterpolationEnv) error {
//
// /
// By default if an environment variable exists in both the runtime and pipeline env
// we will substitute with the pipeline env IF the pipeline env is defined first.
// Setting the preferRuntimeEnv option to true instead prefers the runtime environment to pipeline
// environment variables when both are defined.
func (p *Pipeline) Interpolate(interpolationEnv InterpolationEnv, preferRuntimeEnv bool) error {
if interpolationEnv == nil {
interpolationEnv = env.New()
}

// Preprocess any env that are defined in the top level block and place them
// into env for later interpolation into the rest of the pipeline.
if err := p.interpolateEnvBlock(interpolationEnv); err != nil {
if err := p.interpolateEnvBlock(interpolationEnv, preferRuntimeEnv); err != nil {
return err
}

Expand All @@ -108,7 +112,7 @@ func (p *Pipeline) Interpolate(interpolationEnv InterpolationEnv) error {
// results back into p.Env. Since each environment variable in p.Env can
// be interpolated into later environment variables, we also add the results
// to interpolationEnv, making the input ordering of p.Env potentially important.
func (p *Pipeline) interpolateEnvBlock(interpolationEnv InterpolationEnv) error {
func (p *Pipeline) interpolateEnvBlock(interpolationEnv InterpolationEnv, preferRuntimeEnv bool) error {
return p.Env.Range(func(k, v string) error {
// We interpolate both keys and values.
intk, err := interpolate.Interpolate(interpolationEnv, k)
Expand All @@ -125,7 +129,7 @@ func (p *Pipeline) interpolateEnvBlock(interpolationEnv InterpolationEnv) error
p.Env.Replace(k, intk, intv)

// If the variable already existed and we prefer the runtime environment then don't overwrite it
if _, exists := interpolationEnv.Get(intk); !(p.preferRuntimeEnv && exists) {
if _, exists := interpolationEnv.Get(intk); !(preferRuntimeEnv && exists) {
interpolationEnv.Set(intk, intv)
}

Expand Down

0 comments on commit eb09bcc

Please sign in to comment.