diff --git a/README.md b/README.md index 21f13dcb..440e50ff 100644 --- a/README.md +++ b/README.md @@ -3,27 +3,38 @@ [![Build status](https://badge.buildkite.com/d58c90abfe8b48f8d8750dac8e911fc0b6afe026631b4dc97c.svg?branch=main)](https://buildkite.com/buildkite-kubernetes-stack/kubernetes-agent-stack) ## Table of Contents -- [Overview](#Overview) -- [How does it work](#How-does-it-work) -- [Architecture](#Architecture) -- [Installation](#Installation) - - [Requirements](#Requirements) - - [Deploy with Helm](#Deploy-with-Helm) - - [Options](#Options) -- [Sample Buildkite Pipeline](#Sample-Buildkite-Pipelines) - - [Cloning repos via SSH](#Cloning-repos-via-SSH) - - [Cloning repos via HTTPS](#Cloning-repos-via-HTTPS) - - [Pod Spec Patch](#Pod-Spec-Patch) - - [Sidecars](#Sidecars) - - [Extra volume mounts](#Extra-volume-mounts) - - [Skipping checkout](#Skipping-checkout) - - [Overriding flags for git clone/fetch](#Overriding-flags-for-git-clonefetch) - - [Validating your pipeline](#Validating-your-pipeline) -- [Securing the stack](#securing-the-stack) - - [Prohibiting the kubernetes plugin (v0.13.0 and later)](#prohibiting-the-kubernetes-plugin-v0130-and-later) -- [How to setup agent hooks](#How-to-setup-agent-hooks) -- [Debugging](#Debugging) -- [Open Questions](#Open-Questions) +- [Overview](#overview) +- [How does it work](#how-does-it-work) +- [Architecture](#architecture) +- [Installation](#installation) + - [Requirements](#requirements) + - [Deploy with Helm](#deploy-with-helm) + - [Options](#options) +- [Sample Buildkite Pipelines](#sample-buildkite-pipelines) + - [PodSpec command and args interpretation](#podspec-command-and-args-interpretation) + - [Cloning repos via SSH](#cloning-repos-via-ssh) + - [Cloning repos via HTTPS](#cloning-repos-via-https) + - [Default job metadata](#default-job-metadata) + - [Pod Spec Patch](#pod-spec-patch) + - [Sidecars](#sidecars) + - [The workspace volume](#the-workspace-volume) + - [Extra volume mounts](#extra-volume-mounts) + - [Skipping checkout (v0.13.0 and later)](#skipping-checkout-v0130-and-later) + - [Overriding flags for git clone and git fetch (v0.13.0 and later)](#overriding-flags-for-git-clone-and-git-fetch-v0130-and-later) + - [Overriding other git settings (v0.16.0 and later)](#overriding-other-git-settings-v0160-and-later) + - [Default envFrom](#default-envfrom) +- [Setting agent configuration (v0.16.0 and later)](#setting-agent-configuration-v0160-and-later) +- [How to set up pipeline signing (v0.16.0 and later)](#how-to-set-up-pipeline-signing-v0160-and-later) +- [How to set up agent hooks and plugins (v0.16.0 and later)](#how-to-set-up-agent-hooks-and-plugins-v0160-and-later) +- [How to set up agent hooks (v0.15.0 and earlier)](#how-to-set-up-agent-hooks-v0150-and-earlier) + - [Validating your pipeline](#validating-your-pipeline) +- [Securing the stack](#securing-the-stack) + - [Prohibiting the kubernetes plugin (v0.13.0 and later)](#prohibiting-the-kubernetes-plugin-v0130-and-later) +- [Debugging](#debugging) + - [Prerequisites](#prerequisites) + - [Inputs to the script](#inputs-to-the-script) + - [Data/logs gathered:](#datalogs-gathered) +- [Open questions](#open-questions) ## Overview @@ -553,6 +564,47 @@ steps: command: echo Hello World! ``` +#### Overriding commands + +By default, PodSpecPatch is prevented from modifying a container's `command` or +`args`. Attempting to do so results in an error. + +If this is something you want to do, first consider other potential solutions: + +* To override checkout behaviour, consider writing a `checkout` hook, or + disabling the checkout container entirely with `checkout: skip: true`. +* To run additional containers without `buildkite-agent` in them, consider using + a sidecar. + +We are continually investigating ways to make the stack more flexible while +ensuring core functionality. + +> [!CAUTION] +> Avoid using PodSpecPatch to override `command` or `args`. Such modifications, +> if not done with extreme care and detailed knowledge about how agent-stack-k8s +> constructs podspecs, are very likely to break how the agent within the pod +> works. +> +> If the replacement command for a checkout or command container does not invoke +> `buildkite-agent bootstrap`: +> +> * the container will not connect to the `agent` container, and the agent will +> not finish the job normally because there was not an expected number of +> other containers connecting to it +> * logs from the container will not be visible in Buildkite +> * hooks will not be executed automatically +> * plugins will not be checked out or executed automatically +> +> and various other functions provided by `buildkite-agent` may not work. +> +> If the command for the `agent` container is overridden, and the replacement +> command does not invoke `buildkite-agent start`, then the job will not be +> acquired on Buildkite at all. + +If you still wish to disable this precaution, and override the raw `command` or +`args` of your containers using PodSpecPatch, you may do so with the +`allow-pod-spec-patch-raw-command-modification` config option. + ### Sidecars Sidecar containers can be added to your job by specifying them under the top-level `sidecars` key. See [this example](internal/integration/fixtures/sidecars.yaml) for a simple job that runs `nginx` as a sidecar, and accesses the nginx server from the main job. diff --git a/charts/agent-stack-k8s/values.schema.json b/charts/agent-stack-k8s/values.schema.json index 5b637d32..c4b7edd5 100644 --- a/charts/agent-stack-k8s/values.schema.json +++ b/charts/agent-stack-k8s/values.schema.json @@ -262,6 +262,12 @@ "title": "Causes the controller to prohibit the kubernetes plugin specified within jobs (pipeline YAML) - enabling this causes jobs with a kubernetes plugin to fail, preventing the pipeline YAML from having any influence over the podSpec", "examples": [true] }, + "allow-pod-spec-patch-raw-command-modification": { + "type": "boolean", + "default": false, + "title": "Permits PodSpecPatch to modify the command or args fields of containers. See the warning in the README before enabling this option", + "examples": [false] + }, "workspaceVolume": { "$ref": "https://kubernetesjsonschema.dev/master/_definitions.json#/definitions/io.k8s.api.core.v1.Volume" }, diff --git a/cmd/controller/controller.go b/cmd/controller/controller.go index 370ba057..1ff9f7ad 100644 --- a/cmd/controller/controller.go +++ b/cmd/controller/controller.go @@ -120,6 +120,11 @@ func AddConfigFlags(cmd *cobra.Command) { false, "Causes the controller to prohibit the kubernetes plugin specified within jobs (pipeline YAML) - enabling this causes jobs with a kubernetes plugin to fail, preventing the pipeline YAML from having any influence over the podSpec", ) + cmd.Flags().Bool( + "allow-pod-spec-patch-raw-command-modification", + false, + "Permits PodSpecPatch to modify the command or args fields of containers. See the warning in the README before enabling this option", + ) } // ReadConfigFromFileArgsAndEnv reads the config from the file, env and args in that order. diff --git a/internal/controller/config/config.go b/internal/controller/config/config.go index b5bb5ed2..fb5da95e 100644 --- a/internal/controller/config/config.go +++ b/internal/controller/config/config.go @@ -68,6 +68,12 @@ type Config struct { // from the job (the kubernetes "plugin" in pipeline.yml). If enabled, // jobs with a "kubernetes" plugin will fail. ProhibitKubernetesPlugin bool `json:"prohibit-kubernetes-plugin" validate:"omitempty"` + + // AllowPodSpecPatchRawCmdMod can be used to allow podSpecPatch to change + // container commands. Normally this is prevented, because if the + // replacement command does not execute buildkite-agent in the right way, + // then the pod will malfunction. + AllowPodSpecPatchRawCmdMod bool `json:"allow-pod-spec-patch-raw-command-modification" validate:"omitempty"` } type stringSlice []string @@ -97,6 +103,7 @@ func (c Config) MarshalLogObject(enc zapcore.ObjectEncoder) error { enc.AddUint16("prometheus-port", c.PrometheusPort) enc.AddString("cluster-uuid", c.ClusterUUID) enc.AddBool("prohibit-kubernetes-plugin", c.ProhibitKubernetesPlugin) + enc.AddBool("allow-pod-spec-patch-raw-command-modification", c.AllowPodSpecPatchRawCmdMod) if err := enc.AddArray("additional-redacted-vars", c.AdditionalRedactedVars); err != nil { return err } diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 3cca6362..3ce656e1 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -87,19 +87,20 @@ func Run( // Scheduler does the complicated work of converting a Buildkite job into // a pod to run that job. It talks to the k8s API to create pods. sched := scheduler.New(logger.Named("scheduler"), k8sClient, scheduler.Config{ - Namespace: cfg.Namespace, - Image: cfg.Image, - AgentTokenSecretName: cfg.AgentTokenSecret, - JobTTL: cfg.JobTTL, - AdditionalRedactedVars: cfg.AdditionalRedactedVars, - WorkspaceVolume: cfg.WorkspaceVolume, - AgentConfig: cfg.AgentConfig, - DefaultCheckoutParams: cfg.DefaultCheckoutParams, - DefaultCommandParams: cfg.DefaultCommandParams, - DefaultSidecarParams: cfg.DefaultSidecarParams, - DefaultMetadata: cfg.DefaultMetadata, - PodSpecPatch: cfg.PodSpecPatch, - ProhibitK8sPlugin: cfg.ProhibitKubernetesPlugin, + Namespace: cfg.Namespace, + Image: cfg.Image, + AgentTokenSecretName: cfg.AgentTokenSecret, + JobTTL: cfg.JobTTL, + AdditionalRedactedVars: cfg.AdditionalRedactedVars, + WorkspaceVolume: cfg.WorkspaceVolume, + AgentConfig: cfg.AgentConfig, + DefaultCheckoutParams: cfg.DefaultCheckoutParams, + DefaultCommandParams: cfg.DefaultCommandParams, + DefaultSidecarParams: cfg.DefaultSidecarParams, + DefaultMetadata: cfg.DefaultMetadata, + PodSpecPatch: cfg.PodSpecPatch, + ProhibitK8sPlugin: cfg.ProhibitKubernetesPlugin, + AllowPodSpecPatchRawCmdMod: cfg.AllowPodSpecPatchRawCmdMod, }) informerFactory, err := NewInformerFactory(k8sClient, cfg.Namespace, cfg.Tags) diff --git a/internal/controller/scheduler/scheduler.go b/internal/controller/scheduler/scheduler.go index 23052031..c9d40087 100644 --- a/internal/controller/scheduler/scheduler.go +++ b/internal/controller/scheduler/scheduler.go @@ -40,19 +40,20 @@ const ( var errK8sPluginProhibited = errors.New("the kubernetes plugin is prohibited by this controller, but was configured on this job") type Config struct { - Namespace string - Image string - AgentTokenSecretName string - JobTTL time.Duration - AdditionalRedactedVars []string - WorkspaceVolume *corev1.Volume - AgentConfig *config.AgentConfig - DefaultCheckoutParams *config.CheckoutParams - DefaultCommandParams *config.CommandParams - DefaultSidecarParams *config.SidecarParams - DefaultMetadata config.Metadata - PodSpecPatch *corev1.PodSpec - ProhibitK8sPlugin bool + Namespace string + Image string + AgentTokenSecretName string + JobTTL time.Duration + AdditionalRedactedVars []string + WorkspaceVolume *corev1.Volume + AgentConfig *config.AgentConfig + DefaultCheckoutParams *config.CheckoutParams + DefaultCommandParams *config.CommandParams + DefaultSidecarParams *config.SidecarParams + DefaultMetadata config.Metadata + PodSpecPatch *corev1.PodSpec + ProhibitK8sPlugin bool + AllowPodSpecPatchRawCmdMod bool } func New(logger *zap.Logger, client kubernetes.Interface, cfg Config) *worker { @@ -662,7 +663,7 @@ func (w *worker) Build(podSpec *corev1.PodSpec, skipCheckout bool, inputs buildI // Patch from the agent is applied first if w.cfg.PodSpecPatch != nil { - patched, err := PatchPodSpec(podSpec, w.cfg.PodSpecPatch) + patched, err := PatchPodSpec(podSpec, w.cfg.PodSpecPatch, w.cfg.AllowPodSpecPatchRawCmdMod) if err != nil { return nil, fmt.Errorf("failed to apply podSpec patch from agent: %w", err) } @@ -671,7 +672,7 @@ func (w *worker) Build(podSpec *corev1.PodSpec, skipCheckout bool, inputs buildI } if inputs.k8sPlugin != nil && inputs.k8sPlugin.PodSpecPatch != nil { - patched, err := PatchPodSpec(podSpec, inputs.k8sPlugin.PodSpecPatch) + patched, err := PatchPodSpec(podSpec, inputs.k8sPlugin.PodSpecPatch, w.cfg.AllowPodSpecPatchRawCmdMod) if err != nil { return nil, fmt.Errorf("failed to apply podSpec patch from k8s plugin: %w", err) } @@ -686,12 +687,17 @@ func (w *worker) Build(podSpec *corev1.PodSpec, skipCheckout bool, inputs buildI var ErrNoCommandModification = errors.New("modifying container commands or args via podSpecPatch is not supported. Specify the command in the job's command field instead") -func PatchPodSpec(original *corev1.PodSpec, patch *corev1.PodSpec) (*corev1.PodSpec, error) { - // We do special stuff™️ with container commands to make them run as buildkite agent things under the hood, so don't - // let users mess with them via podSpecPatch. - for _, c := range patch.Containers { - if len(c.Command) != 0 || len(c.Args) != 0 { - return nil, ErrNoCommandModification +func PatchPodSpec(original, patch *corev1.PodSpec, allowCmdMod bool) (*corev1.PodSpec, error) { + if !allowCmdMod { + // We do special stuff™️ with container commands to make them run as + // buildkite agent things/ under the hood, so don't let users mess with + // them via podSpecPatch... + // Unless they specifically configure the stack to allow it and have + // read the big red warning in the readme. + for _, c := range patch.Containers { + if len(c.Command) != 0 || len(c.Args) != 0 { + return nil, ErrNoCommandModification + } } } diff --git a/internal/controller/scheduler/scheduler_test.go b/internal/controller/scheduler/scheduler_test.go index a23d7ae1..5785638d 100644 --- a/internal/controller/scheduler/scheduler_test.go +++ b/internal/controller/scheduler/scheduler_test.go @@ -97,7 +97,7 @@ func TestPatchPodSpec(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - result, err := scheduler.PatchPodSpec(c.podspec, c.patch) + result, err := scheduler.PatchPodSpec(c.podspec, c.patch, false) if c.err != nil { require.Error(t, err) require.ErrorIs(t, c.err, err)