Skip to content

Commit

Permalink
Add config option to disable command modification check
Browse files Browse the repository at this point in the history
  • Loading branch information
DrJosh9000 committed Dec 16, 2024
1 parent 2138e80 commit c8baf91
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 56 deletions.
94 changes: 73 additions & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions charts/agent-stack-k8s/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
5 changes: 5 additions & 0 deletions cmd/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions internal/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
27 changes: 14 additions & 13 deletions internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 27 additions & 21 deletions internal/controller/scheduler/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/controller/scheduler/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit c8baf91

Please sign in to comment.