Skip to content

fix(webhook.go): Consider 0th index - #34

Merged
Souheil-Yazji merged 13 commits into
masterfrom
sidecar-patch
Feb 24, 2026
Merged

Souheil-Yazji merged 13 commits into
masterfrom
sidecar-patch

Conversation

@Souheil-Yazji

@Souheil-Yazji Souheil-Yazji commented Feb 17, 2026 •

Copy link
Copy Markdown
Collaborator

Webhook Bug Fix: Multiple Sidecars Not Being Injected

Problem Description

When deploying with multiple volumes/shares to mount, only the last injected sidecar initContainer appeared in the mutated Pod.

Root Cause

Root Cause A — Repeated “first add” overwrote earlier additions

The original injection logic called helpers like this on every loop iteration:

// BUGGY CODE - called in each loop iteration
patch = append(patch, addContainer(pod.Spec.InitContainers, sidecarConfig.Containers, "/spec/initContainers")...)

The helper functions determine “first vs append” like this:

first := len(target) == 0

The problem: Each loop iteration passed the original, unchanged pod.Spec.InitContainers Those slices were never updated during patch construction, so len(target) stayed 0 every time.

That caused patches like:

[
  {"op": "add", "path": "/spec/initContainers", "value": [container1]},
  {"op": "add", "path": "/spec/initContainers", "value": [container2]},
  {"op": "add", "path": "/spec/initContainers", "value": [container3]}
]

In JSON Patch, op: add on an existing field path effectively replaces the value, so only the last one survived.

Root Cause B — Appending to an array that doesn’t exist

After switching to append semantics:

{"op":"add","path":"/spec/initContainers/-","value":{...}}

Kubernetes rejects this when /spec/initContainers is missing (not present at all), producing:

doc is missing path: "/spec/initContainers/-"

This happens commonly for Pods created that don’t define initContainers upfront.

Solution

Step 1 — Ensure the arrays exist once (initContainers)

Before appending any entries, the webhook ensures these fields exist:

  • If spec.initContainers is missing, add it as an empty array []

Example patches:

{"op":"add","path":"/spec/initContainers","value":[]}

Step 2 — Always append with /-

Once the arrays exist, every injected initContainer is appended using /-:

{"op":"add","path":"/spec/initContainers/-","value":{...}}

This removes the need to track “first vs subsequent” across loop iterations and prevents overwrites.

Correct Behavior Now

With N shares, the webhook generates:

  • At most 1 patch to create /spec/initContainers (only if missing)
  • N patches to append initContainers

Example (simplified):

[
  {"op":"add","path":"/spec/initContainers","value":[]},
  {"op":"add","path":"/spec/initContainers/-","value":container1},
  {"op":"add","path":"/spec/initContainers/-","value":container2},
]

Files Changed

  • webhook.go

    • Added “ensure array exists” logic for:

      • /spec/initContainers
    • Updated injection to append using /- consistently

    • Removed reliance on len(target) == 0 for correctness across loop iterations

Testing Recommendations

  1. Create a ConfigMap with multiple shares for a single SVM

  2. Create a ConfigMap with multiple SVMs, each with multiple shares

  3. Verify the mutated pod contains all expected initContainers and volumes

  4. Check webhook logs to confirm:

    • add /spec/initContainers [] appears only when missing
    • subsequent initContainer ops use /spec/initContainers/-
  5. Verify all volumes mount correctly in the running pod

Additional Notes

  • Each share injects:

    • 1 initContainer
    • 2 volumes (fuse-fd-passing + fuse-csi-ephemeral)
  • Ensuring arrays exist prevents:

    • overwrite-by-repeated-add on /spec/initContainers
    • missing-path failures when appending with /-

Comment thread cmd/webhook.go Resolved
Comment thread cmd/webhook.go Resolved

@mathis-marcotte mathis-marcotte left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Souheil-Yazji

Copy link
Copy Markdown
Collaborator Author
image

@Souheil-Yazji

Copy link
Copy Markdown
Collaborator Author

Volumes fixed, merging.

@Souheil-Yazji
Souheil-Yazji merged commit 001be4b into master Feb 24, 2026
1 check passed
@Souheil-Yazji
Souheil-Yazji deleted the sidecar-patch branch February 24, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants