Skip to content

fix: use StrategicMergePatchType for pod condition updates to avoid TOCTOU race - #2756

Open
WorrierKhushal wants to merge 1 commit into
openyurtio:masterfrom
WorrierKhushal:fix/2754-toctou-race-pod-image
Open

fix: use StrategicMergePatchType for pod condition updates to avoid TOCTOU race#2756
WorrierKhushal wants to merge 1 commit into
openyurtio:masterfrom
WorrierKhushal:fix/2754-toctou-race-pod-image

Conversation

@WorrierKhushal

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind bug

What this PR does / why we need it:

Background

The PullPodImage handler in pkg/yurthub/otaupdate/ota.go injects a
PodImageReady condition onto a Pod during OTA image pull. It fetched the
current Pod, locally mutated the full Status.Conditions array via
podutil.UpdatePodCondition, and patched the Pod with the entire array
using types.MergePatchType.

The Bug

MergePatchType (RFC 7386 JSON Merge Patch) does not support array
merging — it replaces arrays wholesale. If the Kubelet or another
controller updated a Pod condition (e.g. Ready=True) in the window
between this handler's getPod() call and its Patch() call, that
update was silently overwritten by the stale array this handler had
fetched earlier. This could leave a Pod stuck reporting as not-ready on
the control plane even after the Kubelet had confirmed it was running.

Verification of the fix approach

Checked k8s.io/api v0.34.0's PodStatus.Conditions field definition
directly — it carries patchStrategy:"merge" patchMergeKey:"type",
confirming the API server natively supports a strategic merge patch that
merges condition entries by their Type field. Also confirmed the fake
clientset used in this package's tests fully supports
StrategicMergePatchType.

The Fix

  • Removed the local podutil.UpdatePodCondition full-array mutation
    (and the now-unused podutil import).
  • Changed the patch payload to contain only the single new condition
    instead of the full conditions array.
  • Changed the patch type from types.MergePatchType to
    types.StrategicMergePatchType, letting the API server perform the
    merge natively by condition Type, eliminating the TOCTOU window
    entirely.

Added TestPullPodImageStrategicMergePatch, verifying the patch sent is
StrategicMergePatchType and contains exactly one condition.

Which issue(s) this PR fixes:

Fixes #2754

Special notes for your reviewer:

  • go test -v ./pkg/yurthub/otaupdate/... — all tests pass (verified on Linux/WSL), including the new test
  • go build/go vet on the affected package — clean
  • No exported function signatures changed; scoped entirely to the patch construction inside PullPodImage

Does this PR introduce a user-facing change?

NONE

other Note

@WorrierKhushal
WorrierKhushal requested a review from a team as a code owner August 16, 2026 18:27
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] TOCTOU race in OTA update PullPodImage wipes out concurrent Pod condition updates from Kubelet

1 participant