diff --git a/charts/agent-stack-k8s/values.schema.json b/charts/agent-stack-k8s/values.schema.json index 4a80862b..27d34b94 100644 --- a/charts/agent-stack-k8s/values.schema.json +++ b/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": [ diff --git a/internal/controller/config/config.go b/internal/controller/config/config.go index 081acb03..d3fb2d5e 100644 --- a/internal/controller/config/config.go +++ b/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) } diff --git a/internal/controller/controller.go b/internal/controller/controller.go index e5a14def..bc87679f 100644 --- a/internal/controller/controller.go +++ b/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) diff --git a/internal/controller/scheduler/scheduler.go b/internal/controller/scheduler/scheduler.go index 7177fc3b..7e267bc0 100644 --- a/internal/controller/scheduler/scheduler.go +++ b/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", diff --git a/internal/integration/fixtures/redacted-vars.yaml b/internal/integration/fixtures/redacted-vars.yaml new file mode 100644 index 00000000..826a4468 --- /dev/null +++ b/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} diff --git a/internal/integration/integration_test.go b/internal/integration/integration_test.go index 41f920b5..5adae9af 100644 --- a/internal/integration/integration_test.go +++ b/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, diff --git a/internal/integration/testcase_test.go b/internal/integration/testcase_test.go index 4da0b16e..1140f592 100644 --- a/internal/integration/testcase_test.go +++ b/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) {