From 4a6d1b11a5e6b6045e32acc1a8b5f7524938262a Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Mon, 25 Mar 2024 16:41:24 +1100 Subject: [PATCH 1/2] Add redacted-vars config --- charts/agent-stack-k8s/values.schema.json | 9 +++++++ internal/controller/config/config.go | 4 ++++ internal/controller/controller.go | 9 +++---- internal/controller/scheduler/scheduler.go | 13 ++++++---- .../integration/fixtures/redacted-vars.yaml | 20 ++++++++++++++++ internal/integration/integration_test.go | 24 +++++++++++++++++++ internal/integration/testcase_test.go | 16 +++++++++++-- 7 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 internal/integration/fixtures/redacted-vars.yaml diff --git a/charts/agent-stack-k8s/values.schema.json b/charts/agent-stack-k8s/values.schema.json index 4a80862b..c957cea6 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": [""] + }, + "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..b8649e34 100644 --- a/internal/controller/config/config.go +++ b/internal/controller/config/config.go @@ -26,6 +26,7 @@ type Config struct { Tags stringSlice `mapstructure:"tags" validate:"min=1"` ProfilerAddress string `mapstructure:"profiler-address" validate:"omitempty,hostname_port"` ClusterUUID string `mapstructure:"cluster-uuid" validate:"omitempty"` + RedactedVars stringSlice `mapstructure:"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("redacted-vars", c.RedactedVars); err != nil { + return err + } return enc.AddArray("tags", c.Tags) } diff --git a/internal/controller/controller.go b/internal/controller/controller.go index e5a14def..33dec087 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, + RedactedVars: cfg.RedactedVars, }) 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..96689adb 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 + RedactedVars []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.RedactedVars, 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..21ca1d83 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 TestControllerSetsRedactedVars(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.RedactedVars = []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..0a70266d 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,19 @@ 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) AssertLogsDoNotContain(build api.Build, content string) { + t.Helper() + + assert.NotContains(t, t.FetchLogs, content) } func (t testcase) AssertArtifactsContain(build api.Build, expected ...string) { From c5a057c31f6419e44c51e4ac62faba531a2dc26e Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Tue, 26 Mar 2024 10:57:11 +1100 Subject: [PATCH 2/2] additional-redacted-vars Also remove unused AssertLogsDoNotContain --- charts/agent-stack-k8s/values.schema.json | 2 +- internal/controller/config/config.go | 26 +++++++++++----------- internal/controller/controller.go | 10 ++++----- internal/controller/scheduler/scheduler.go | 12 +++++----- internal/integration/integration_test.go | 4 ++-- internal/integration/testcase_test.go | 6 ----- 6 files changed, 27 insertions(+), 33 deletions(-) diff --git a/charts/agent-stack-k8s/values.schema.json b/charts/agent-stack-k8s/values.schema.json index c957cea6..27d34b94 100644 --- a/charts/agent-stack-k8s/values.schema.json +++ b/charts/agent-stack-k8s/values.schema.json @@ -205,7 +205,7 @@ "title": "The UUID of the Buildkite cluster to pull Jobs from", "examples": [""] }, - "redacted-vars": { + "additional-redacted-vars": { "type": "array", "default": [], "title": "Additional environment variables to redact values from logs", diff --git a/internal/controller/config/config.go b/internal/controller/config/config.go index b8649e34..d3fb2d5e 100644 --- a/internal/controller/config/config.go +++ b/internal/controller/config/config.go @@ -15,18 +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"` - RedactedVars stringSlice `mapstructure:"redacted-vars" 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 @@ -48,7 +48,7 @@ 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("redacted-vars", c.RedactedVars); err != nil { + 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 33dec087..bc87679f 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -49,11 +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, - RedactedVars: cfg.RedactedVars, + 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 96689adb..7e267bc0 100644 --- a/internal/controller/scheduler/scheduler.go +++ b/internal/controller/scheduler/scheduler.go @@ -30,11 +30,11 @@ const ( ) type Config struct { - Namespace string - Image string - AgentToken string - JobTTL time.Duration - RedactedVars []string + Namespace string + Image string + AgentToken string + JobTTL time.Duration + AdditionalRedactedVars []string } func New(logger *zap.Logger, client kubernetes.Interface, cfg Config) *worker { @@ -229,7 +229,7 @@ func (w *jobWrapper) Build(skipCheckout bool) (*batchv1.Job, error) { } } - redactedVars := append(w.cfg.RedactedVars, clicommand.RedactedVars.Value.Value()...) + redactedVars := append(w.cfg.AdditionalRedactedVars, clicommand.RedactedVars.Value.Value()...) volumeMounts := []corev1.VolumeMount{{Name: "workspace", MountPath: "/workspace"}} volumeMounts = append(volumeMounts, w.k8sPlugin.ExtraVolumeMounts...) diff --git a/internal/integration/integration_test.go b/internal/integration/integration_test.go index 21ca1d83..5adae9af 100644 --- a/internal/integration/integration_test.go +++ b/internal/integration/integration_test.go @@ -54,7 +54,7 @@ func TestControllerPicksUpJobsWithSubsetOfAgentTags(t *testing.T) { tc.AssertSuccess(ctx, build) } -func TestControllerSetsRedactedVars(t *testing.T) { +func TestControllerSetsAdditionalRedactedVars(t *testing.T) { tc := testcase{ T: t, Fixture: "redacted-vars.yaml", @@ -67,7 +67,7 @@ func TestControllerSetsRedactedVars(t *testing.T) { t.Cleanup(cleanup) cfg := cfg - cfg.RedactedVars = []string{"ELEVEN_HERBS_AND_SPICES"} + cfg.AdditionalRedactedVars = []string{"ELEVEN_HERBS_AND_SPICES"} tc.StartController(ctx, cfg) build := tc.TriggerBuild(ctx, pipelineID) diff --git a/internal/integration/testcase_test.go b/internal/integration/testcase_test.go index 0a70266d..1140f592 100644 --- a/internal/integration/testcase_test.go +++ b/internal/integration/testcase_test.go @@ -196,12 +196,6 @@ func (t testcase) AssertLogsContain(build api.Build, content string) { assert.Contains(t, t.FetchLogs(build), content) } -func (t testcase) AssertLogsDoNotContain(build api.Build, content string) { - t.Helper() - - assert.NotContains(t, t.FetchLogs, content) -} - func (t testcase) AssertArtifactsContain(build api.Build, expected ...string) { t.Helper() config, err := buildkite.NewTokenConfig(cfg.BuildkiteToken, false)