Merge pull request #2013 from buildkite/threadsafe-env

Make agent environment threadsafe

ACKNOWLEDGEMENTS.md

@@ -1,6 +1,6 @@
 # Buildkite Agent OSS Attributions
 
-The Buildkite Agent would not be possible without open-source software. 
+The Buildkite Agent would not be possible without open-source software.
 Licenses for the libraries used are reproduced below.
 
 
@@ -3363,6 +3363,36 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 ```
 
 
+---
+
+## github.com/puzpuzpuz/xsync/v2/LICENSE
+
+```
+MIT License
+
+Copyright (c) 2021 Andrey Pechkurov
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+
+```
+
+
 ---
 
 ## github.com/qri-io/jsonpointer/LICENSE
@@ -7357,4 +7387,4 @@ limitations under the License.
 ---
 
 File generated using ./scripts/generate-acknowledgements.sh
-Wed Mar  8 15:33:07 AEDT 2023
+Tue 14 Mar 2023 11:57:14 NZDT

agent/pipeline_parser.go

@@ -17,7 +17,7 @@ import (
 // PipelineParser parses a pipeline, optionally interpolating values from
 // a given environment.
 type PipelineParser struct {
-	Env             env.Environment
+	Env             *env.Environment
 	Filename        string
 	Pipeline        []byte
 	NoInterpolation bool

agent/plugin/plugin.go

@@ -244,7 +244,7 @@ func walkConfigValues(prefix string, v any, into *[]string) error {
 
 // ConfigurationToEnvironment converts the plugin configuration values to
 // environment variables.
-func (p *Plugin) ConfigurationToEnvironment() (env.Environment, error) {
+func (p *Plugin) ConfigurationToEnvironment() (*env.Environment, error) {
 	envSlice := []string{}
 	envPrefix := fmt.Sprintf("BUILDKITE_PLUGIN_%s", formatEnvKey(p.Name()))
 

agent/plugin/plugin_test.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/buildkite/agent/v3/env"
 	"github.com/google/go-cmp/cmp"
 )
 
@@ -372,19 +371,19 @@ func TestConfigurationToEnvironment(t *testing.T) {
 
 	tests := []struct {
 		configJSON string
-		wantEnvMap env.Environment
+		wantEnvMap map[string]string
 	}{
 		{
 			configJSON: `{ "config-key": 42 }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":             `{"config-key":42}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_CONFIG_KEY": "42",
 				"BUILDKITE_PLUGIN_NAME":                      "DOCKER_COMPOSE",
 			},
 		},
 		{
 			configJSON: `{ "container": "app", "some-other-setting": "else right here" }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":                     `{"container":"app","some-other-setting":"else right here"}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_CONTAINER":          "app",
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_SOME_OTHER_SETTING": "else right here",
@@ -393,15 +392,15 @@ func TestConfigurationToEnvironment(t *testing.T) {
 		},
 		{
 			configJSON: `{ "and _ with a    - number": 12 }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":                    `{"and _ with a    - number":12}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_AND_WITH_A_NUMBER": "12",
 				"BUILDKITE_PLUGIN_NAME":                             "DOCKER_COMPOSE",
 			},
 		},
 		{
 			configJSON: `{ "bool-true-key": true, "bool-false-key": false }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":                 `{"bool-false-key":false,"bool-true-key":true}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_BOOL_FALSE_KEY": "false",
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_BOOL_TRUE_KEY":  "true",
@@ -410,7 +409,7 @@ func TestConfigurationToEnvironment(t *testing.T) {
 		},
 		{
 			configJSON: `{ "array-key": [ "array-val-1", "array-val-2" ] }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":              `{"array-key":["array-val-1","array-val-2"]}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_0": "array-val-1",
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_1": "array-val-2",
@@ -419,7 +418,7 @@ func TestConfigurationToEnvironment(t *testing.T) {
 		},
 		{
 			configJSON: `{ "array-key": [ 42, 43, 44 ] }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":              `{"array-key":[42,43,44]}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_0": "42",
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_1": "43",
@@ -429,7 +428,7 @@ func TestConfigurationToEnvironment(t *testing.T) {
 		},
 		{
 			configJSON: `{ "array-key": [ 42, 43, "foo" ] }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":              `{"array-key":[42,43,"foo"]}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_0": "42",
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_1": "43",
@@ -439,15 +438,15 @@ func TestConfigurationToEnvironment(t *testing.T) {
 		},
 		{
 			configJSON: `{ "array-key": [ { "subkey": "subval" } ] }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":                     `{"array-key":[{"subkey":"subval"}]}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_0_SUBKEY": "subval",
 				"BUILDKITE_PLUGIN_NAME":                              "DOCKER_COMPOSE",
 			},
 		},
 		{
 			configJSON: `{ "array-key": [ { "subkey": [1, 2, "llamas"] } ] }`,
-			wantEnvMap: env.Environment{
+			wantEnvMap: map[string]string{
 				"BUILDKITE_PLUGIN_CONFIGURATION":                       `{"array-key":[{"subkey":[1,2,"llamas"]}]}`,
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_0_SUBKEY_0": "1",
 				"BUILDKITE_PLUGIN_DOCKER_COMPOSE_ARRAY_KEY_0_SUBKEY_1": "2",
@@ -465,10 +464,11 @@ func TestConfigurationToEnvironment(t *testing.T) {
 			if err != nil {
 				t.Fatalf("pluginFromConfig(%q) error = %v", tc.configJSON, err)
 			}
-			envMap, err := plugin.ConfigurationToEnvironment()
+			env, err := plugin.ConfigurationToEnvironment()
 			if err != nil {
 				t.Errorf("plugin.ConfigurationToEnvironment() error = %v", err)
 			}
+			envMap := env.Dump()
 			if diff := cmp.Diff(envMap, tc.wantEnvMap); diff != "" {
 				t.Errorf("plugin.ConfigurationToEnvironment() envMap diff (-got +want)\n%s", diff)
 			}
@@ -508,25 +508,27 @@ func TestConfigurationToEnvironment_DuplicatePlugin(t *testing.T) {
 	if err != nil {
 		t.Errorf("plugins[0].ConfigurationToEnvironment() error = %v", err)
 	}
-	wantEnv1 := env.Environment{
+	wantEnv1 := map[string]string{
 		"BUILDKITE_PLUGIN_CONFIGURATION":             `{"config-key":41}`,
 		"BUILDKITE_PLUGIN_DOCKER_COMPOSE_CONFIG_KEY": "41",
 		"BUILDKITE_PLUGIN_NAME":                      "DOCKER_COMPOSE",
 	}
-	if diff := cmp.Diff(envMap1, wantEnv1); diff != "" {
+	if diff := cmp.Diff(envMap1.Dump(), wantEnv1); diff != "" {
 		t.Errorf("plugins[0].ConfigurationToEnvironment() envMap diff (-got +want)\n%s", diff)
 	}
 
 	envMap2, err := plugins[1].ConfigurationToEnvironment()
 	if err != nil {
 		t.Errorf("plugins[1].ConfigurationToEnvironment() error = %v", err)
 	}
-	wantEnv2 := env.Environment{
+
+	wantEnv2 := map[string]string{
 		"BUILDKITE_PLUGIN_CONFIGURATION":                 `{"second-ref-key":42}`,
 		"BUILDKITE_PLUGIN_DOCKER_COMPOSE_SECOND_REF_KEY": "42",
 		"BUILDKITE_PLUGIN_NAME":                          "DOCKER_COMPOSE",
 	}
-	if diff := cmp.Diff(envMap2, wantEnv2); diff != "" {
+
+	if diff := cmp.Diff(envMap2.Dump(), wantEnv2); diff != "" {
 		t.Errorf("plugins[0].ConfigurationToEnvironment() envMap diff (-got +want)\n%s", diff)
 	}
 }

bootstrap/bootstrap.go

@@ -229,7 +229,7 @@ type HookConfig struct {
 	Name           string
 	Scope          string
 	Path           string
-	Env            env.Environment
+	Env            *env.Environment
 	SpanAttributes map[string]string
 	PluginName     string
 }
@@ -369,7 +369,7 @@ func (b *Bootstrap) applyEnvironmentChanges(changes hook.HookScriptChanges, reda
 
 	// reset output redactors based on new environment variable values
 	redactors.Flush()
-	redactors.Reset(redaction.GetValuesToRedact(b.shell, b.Config.RedactedVars, mergedEnv))
+	redactors.Reset(redaction.GetValuesToRedact(b.shell, b.Config.RedactedVars, mergedEnv.Dump()))
 
 	// First, let see any of the environment variables are supposed
 	// to change the bootstrap configuration at run time.
@@ -1957,7 +1957,7 @@ func (b *Bootstrap) ignoredEnv() []string {
 // matching environment vars.
 // redaction.RedactorMux (possibly empty) is returned so the caller can `defer redactor.Flush()`
 func (b *Bootstrap) setupRedactors() redaction.RedactorMux {
-	valuesToRedact := redaction.GetValuesToRedact(b.shell, b.Config.RedactedVars, b.shell.Env)
+	valuesToRedact := redaction.GetValuesToRedact(b.shell, b.Config.RedactedVars, b.shell.Env.Dump())
 	if len(valuesToRedact) == 0 {
 		return nil
 	}

bootstrap/config.go

@@ -158,7 +158,7 @@ type Config struct {
 
 // ReadFromEnvironment reads configuration from the Environment, returns a map
 // of the env keys that changed and the new values
-func (c *Config) ReadFromEnvironment(environ env.Environment) map[string]string {
+func (c *Config) ReadFromEnvironment(environ *env.Environment) map[string]string {
 	changed := map[string]string{}
 
 	// Use reflection for the type and values

bootstrap/shell/shell.go

@@ -45,7 +45,7 @@ type Shell struct {
 	Logger
 
 	// The running environment for the shell
-	Env env.Environment
+	Env *env.Environment
 
 	// Whether the shell is a PTY
 	PTY bool
@@ -327,13 +327,13 @@ func (s *Shell) RunAndCapture(ctx context.Context, command string, arg ...string
 
 // injectTraceCtx adds tracing information to the given env vars to support
 // distributed tracing across jobs/builds.
-func (s *Shell) injectTraceCtx(ctx context.Context, env env.Environment) {
+func (s *Shell) injectTraceCtx(ctx context.Context, env *env.Environment) {
 	span := opentracing.SpanFromContext(ctx)
 	// Not all shell runs will have tracing (nor do they really need to).
 	if span == nil {
 		return
 	}
-	if err := tracetools.EncodeTraceContext(span, env); err != nil {
+	if err := tracetools.EncodeTraceContext(span, env.Dump()); err != nil {
 		if s.Debug {
 			s.Logger.Warningf("Failed to encode trace context: %v", err)
 		}
@@ -344,7 +344,7 @@ func (s *Shell) injectTraceCtx(ctx context.Context, env env.Environment) {
 // RunScript is like Run, but the target is an interpreted script which has
 // some extra checks to ensure it gets to the correct interpreter. Extra environment vars
 // can also be passed the script
-func (s *Shell) RunScript(ctx context.Context, path string, extra env.Environment) error {
+func (s *Shell) RunScript(ctx context.Context, path string, extra *env.Environment) error {
 	var command string
 	var args []string
 

bootstrap/shell/test.go

@@ -26,7 +26,7 @@ func NewTestShell(t *testing.T) *Shell {
 
 	// Windows requires certain env variables to be present
 	if runtime.GOOS == "windows" {
-		sh.Env = env.Environment{
+		sh.Env = env.FromMap(map[string]string{
 			//"PATH":        os.Getenv("PATH"),
 			"SystemRoot":  os.Getenv("SystemRoot"),
 			"WINDIR":      os.Getenv("WINDIR"),
@@ -35,11 +35,11 @@ func NewTestShell(t *testing.T) *Shell {
 			"TMP":         os.Getenv("TMP"),
 			"TEMP":        os.Getenv("TEMP"),
 			"ProgramData": os.Getenv("ProgramData"),
-		}
+		})
 	} else {
-		sh.Env = env.Environment{
+		sh.Env = env.FromMap(map[string]string{
 			"PATH": os.Getenv("PATH"),
-		}
+		})
 	}
 
 	return sh

bootstrap/tracing.go

@@ -100,7 +100,7 @@ func (b *Bootstrap) startTracingDatadog(ctx context.Context) (tracetools.Span, c
 // extractTraceCtx pulls encoded distributed tracing information from the env vars.
 // Note: This should match the injectTraceCtx code in shell.
 func (b *Bootstrap) extractDDTraceCtx() opentracing.SpanContext {
-	sctx, err := tracetools.DecodeTraceContext(b.shell.Env)
+	sctx, err := tracetools.DecodeTraceContext(b.shell.Env.Dump())
 	if err != nil {
 		// Return nil so a new span will be created
 		return nil
@@ -182,7 +182,7 @@ func (b *Bootstrap) startTracingOpenTelemetry(ctx context.Context) (tracetools.S
 	return tracetools.NewOpenTelemetrySpan(span), ctx, stop
 }
 
-func GenericTracingExtras(b *Bootstrap, env env.Environment) map[string]any {
+func GenericTracingExtras(b *Bootstrap, env *env.Environment) map[string]any {
 	buildID, _ := env.Get("BUILDKITE_BUILD_ID")
 	buildNumber, _ := env.Get("BUILDKITE_BUILD_NUMBER")
 	buildURL, _ := env.Get("BUILDKITE_BUILD_URL")

clicommand/pipeline_upload.go

@@ -246,7 +246,7 @@ var PipelineUploadCommand = cli.Command{
 		}
 
 		if len(cfg.RedactedVars) > 0 {
-			needles := redaction.GetKeyValuesToRedact(shell.StderrLogger, cfg.RedactedVars, env.FromSlice(os.Environ()))
+			needles := redaction.GetKeyValuesToRedact(shell.StderrLogger, cfg.RedactedVars, env.FromSlice(os.Environ()).Dump())
 
 			serialisedPipeline, err := result.MarshalJSON()
 			if err != nil {