Merge pull request #280 from buildkite/PDP-2081-redaction-config

Add redacted-vars config

Author: DrJosh9000

charts/agent-stack-k8s/values.schema.json

@@ -204,6 +204,15 @@
           "default": "",
           "title": "The UUID of the Buildkite cluster to pull Jobs from",
           "examples": [""]
+        },
+        "additional-redacted-vars": {
+          "type": "array",
+          "default": [],
+          "title": "Additional environment variables to redact values from logs",
+          "items": {
+            "type": "string"
+          },
+          "examples": [["SECRET_RECIPE"]]
         }
       },
       "examples": [

internal/controller/config/config.go

@@ -15,17 +15,18 @@ const (
 )
 
 type Config struct {
-	Debug            bool          `mapstructure:"debug"`
-	AgentTokenSecret string        `mapstructure:"agent-token-secret" validate:"required"`
-	BuildkiteToken   string        `mapstructure:"buildkite-token"    validate:"required"`
-	Image            string        `mapstructure:"image"              validate:"required"`
-	JobTTL           time.Duration `mapstructure:"job-ttl"`
-	MaxInFlight      int           `mapstructure:"max-in-flight"      validate:"min=0"`
-	Namespace        string        `mapstructure:"namespace"          validate:"required"`
-	Org              string        `mapstructure:"org"                validate:"required"`
-	Tags             stringSlice   `mapstructure:"tags"               validate:"min=1"`
-	ProfilerAddress  string        `mapstructure:"profiler-address"   validate:"omitempty,hostname_port"`
-	ClusterUUID      string        `mapstructure:"cluster-uuid"       validate:"omitempty"`
+	Debug                  bool          `mapstructure:"debug"`
+	AgentTokenSecret       string        `mapstructure:"agent-token-secret"       validate:"required"`
+	BuildkiteToken         string        `mapstructure:"buildkite-token"          validate:"required"`
+	Image                  string        `mapstructure:"image"                    validate:"required"`
+	JobTTL                 time.Duration `mapstructure:"job-ttl"`
+	MaxInFlight            int           `mapstructure:"max-in-flight"            validate:"min=0"`
+	Namespace              string        `mapstructure:"namespace"                validate:"required"`
+	Org                    string        `mapstructure:"org"                      validate:"required"`
+	Tags                   stringSlice   `mapstructure:"tags"                     validate:"min=1"`
+	ProfilerAddress        string        `mapstructure:"profiler-address"         validate:"omitempty,hostname_port"`
+	ClusterUUID            string        `mapstructure:"cluster-uuid"             validate:"omitempty"`
+	AdditionalRedactedVars stringSlice   `mapstructure:"additional-redacted-vars" validate:"omitempty"`
 }
 
 type stringSlice []string
@@ -47,5 +48,8 @@ func (c Config) MarshalLogObject(enc zapcore.ObjectEncoder) error {
 	enc.AddString("org", c.Org)
 	enc.AddString("profiler-address", c.ProfilerAddress)
 	enc.AddString("cluster-uuid", c.ClusterUUID)
+	if err := enc.AddArray("additional-redacted-vars", c.AdditionalRedactedVars); err != nil {
+		return err
+	}
 	return enc.AddArray("tags", c.Tags)
 }

internal/controller/controller.go

@@ -49,10 +49,11 @@ func Run(
 	}
 
 	sched := scheduler.New(logger.Named("scheduler"), k8sClient, scheduler.Config{
-		Namespace:  cfg.Namespace,
-		Image:      cfg.Image,
-		AgentToken: cfg.AgentTokenSecret,
-		JobTTL:     cfg.JobTTL,
+		Namespace:              cfg.Namespace,
+		Image:                  cfg.Image,
+		AgentToken:             cfg.AgentTokenSecret,
+		JobTTL:                 cfg.JobTTL,
+		AdditionalRedactedVars: cfg.AdditionalRedactedVars,
 	})
 	limiter := scheduler.NewLimiter(logger.Named("limiter"), sched, cfg.MaxInFlight)
 

internal/controller/scheduler/scheduler.go

@@ -30,10 +30,11 @@ const (
 )
 
 type Config struct {
-	Namespace  string
-	Image      string
-	AgentToken string
-	JobTTL     time.Duration
+	Namespace              string
+	Image                  string
+	AgentToken             string
+	JobTTL                 time.Duration
+	AdditionalRedactedVars []string
 }
 
 func New(logger *zap.Logger, client kubernetes.Interface, cfg Config) *worker {
@@ -228,6 +229,8 @@ func (w *jobWrapper) Build(skipCheckout bool) (*batchv1.Job, error) {
 		}
 	}
 
+	redactedVars := append(w.cfg.AdditionalRedactedVars, clicommand.RedactedVars.Value.Value()...)
+
 	volumeMounts := []corev1.VolumeMount{{Name: "workspace", MountPath: "/workspace"}}
 	volumeMounts = append(volumeMounts, w.k8sPlugin.ExtraVolumeMounts...)
 
@@ -277,7 +280,7 @@ func (w *jobWrapper) Build(skipCheckout bool) (*batchv1.Job, error) {
 			},
 			corev1.EnvVar{
 				Name:  clicommand.RedactedVars.EnvVar,
-				Value: strings.Join(clicommand.RedactedVars.Value.Value(), ","),
+				Value: strings.Join(redactedVars, ","),
 			},
 			corev1.EnvVar{
 				Name:  "BUILDKITE_SHELL",

internal/integration/fixtures/redacted-vars.yaml

@@ -0,0 +1,20 @@
+steps:
+  - label: ":earth_asia:"
+    agents:
+      queue: {{.queue}}
+    env:
+      BUILDKITE_SHELL: /bin/sh -ec
+      # Note that this is example is a bit contrived, since environment
+      # variables and values can usually be browsed from the Buildkite UI.
+      ELEVEN_HERBS_AND_SPICES: white pepper and 10 others
+    plugins:
+      - kubernetes:
+          podSpec:
+            containers:
+              - image: alpine:latest
+                command:
+                - echo
+                args:
+                - >-
+                  This should be redacted:
+                  $${ELEVEN_HERBS_AND_SPICES}

internal/integration/integration_test.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/buildkite/agent-stack-k8s/v2/api"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -53,6 +54,29 @@ func TestControllerPicksUpJobsWithSubsetOfAgentTags(t *testing.T) {
 	tc.AssertSuccess(ctx, build)
 }
 
+func TestControllerSetsAdditionalRedactedVars(t *testing.T) {
+	tc := testcase{
+		T:       t,
+		Fixture: "redacted-vars.yaml",
+		Repo:    repoHTTP,
+		GraphQL: api.NewClient(cfg.BuildkiteToken),
+	}.Init()
+
+	ctx := context.Background()
+	pipelineID, cleanup := tc.CreatePipeline(ctx)
+	t.Cleanup(cleanup)
+
+	cfg := cfg
+	cfg.AdditionalRedactedVars = []string{"ELEVEN_HERBS_AND_SPICES"}
+
+	tc.StartController(ctx, cfg)
+	build := tc.TriggerBuild(ctx, pipelineID)
+	tc.AssertSuccess(ctx, build)
+	logs := tc.FetchLogs(build)
+	assert.Contains(t, logs, "This should be redacted:")
+	assert.NotContains(t, logs, "white pepper and 10 others")
+}
+
 func TestChown(t *testing.T) {
 	tc := testcase{
 		T:       t,

internal/integration/testcase_test.go

@@ -158,7 +158,7 @@ func (t testcase) AssertSuccess(ctx context.Context, build api.Build) {
 	require.Equal(t, api.BuildStatesPassed, t.waitForBuild(ctx, build))
 }
 
-func (t testcase) AssertLogsContain(build api.Build, content string) {
+func (t testcase) FetchLogs(build api.Build) string {
 	t.Helper()
 
 	config, err := buildkite.NewTokenConfig(cfg.BuildkiteToken, false)
@@ -187,7 +187,13 @@ func (t testcase) AssertLogsContain(build api.Build, content string) {
 		assert.NoError(t, err)
 	}
 
-	assert.Contains(t, logs.String(), content)
+	return logs.String()
+}
+
+func (t testcase) AssertLogsContain(build api.Build, content string) {
+	t.Helper()
+
+	assert.Contains(t, t.FetchLogs(build), content)
 }
 
 func (t testcase) AssertArtifactsContain(build api.Build, expected ...string) {