Skip to content

fix(cronworkflow): prevent metadata overwrite due to stale controller state#15880

Open
GeunSam2 wants to merge 3 commits intoargoproj:mainfrom
GeunSam2:fix/cronworkflow-persistupdate-label-overwrite
Open

fix(cronworkflow): prevent metadata overwrite due to stale controller state#15880
GeunSam2 wants to merge 3 commits intoargoproj:mainfrom
GeunSam2:fix/cronworkflow-persistupdate-label-overwrite

Conversation

@GeunSam2
Copy link
Copy Markdown
Contributor

@GeunSam2 GeunSam2 commented Apr 8, 2026

Summary

Fix a race condition in the CronWorkflow controller where metadata.labels and metadata.annotations can be overwritten with stale values.

Previously, the controller used in-memory objects when performing updates after workflow execution, which could revert newer metadata changes made by external actors.

This PR ensures that:

  • The controller does not overwrite user-managed metadata fields
  • Only controller-owned metadata is updated when necessary
  • Status updates remain unaffected

Fixes #15877


Motivation

In certain scenarios, the controller may overwrite newer metadata changes due to stale in-memory state:

  1. CronWorkflow execution starts
  2. External actor updates metadata (labels/annotations)
  3. Workflow completes
  4. Controller calls persistUpdate() using stale metadata
  5. Newer metadata gets overwritten

This leads to unexpected behavior for systems relying on labels/annotations for:

  • Ownership tracking
  • Deployment/versioning
  • Automation / pruning logic

Changes

  • Avoid full overwrite of metadata.labels and metadata.annotations
  • Limit metadata updates to controller-owned fields only
  • Ensure patch logic does not use stale in-memory values
  • Preserve external/user-managed metadata changes

How to test

  1. Apply a CronWorkflow:
apiVersion: argoproj.io/v1alpha1
kind: CronWorkflow
metadata:
  name: metadata-race-test
  labels:
    test-label: "initial"
spec:
  schedule: "* * * * *"
  concurrencyPolicy: "Forbid"
  workflowSpec:
    entrypoint: main
    templates:
    - name: main
      container:
        image: busybox
        command: ["sh", "-c"]
        args: ["sleep 10"]
  1. While workflow is running, update labels:
kubectl patch cronworkflow metadata-race-test \
  -p '{"metadata":{"labels":{"test-label":"updated"}}}' \
  --type=merge
  1. Wait for workflow completion

Expected behavior

  • test-label=updated is preserved

Before this PR

  • test-label is reverted to "initial"

Checklist

  • I have tested with the :latest image tag
  • I have added/updated tests if applicable
  • I have verified that existing functionality is not broken
  • I have searched for similar issues and confirmed this is not a duplicate

Additional context

This issue is related to controller-side ownership of metadata fields.

The fix aligns with Kubernetes best practices:

  • Controllers should only manage fields they own
  • Avoid blind overwrite of user-defined metadata

…odified labels

persistUpdate() was patching the full metadata.labels and metadata.annotations
maps from a stale in-memory snapshot, silently reverting any external changes
made between the informer cache read and the patch call. This changes
persistUpdate() to only include metadata keys that the controller explicitly
modified during the current operation (e.g. completed label, schedule annotation).

Signed-off-by: gray_karrot <gray@daangn.com>
@GeunSam2 GeunSam2 force-pushed the fix/cronworkflow-persistupdate-label-overwrite branch 3 times, most recently from 53e89ce to 7908684 Compare April 8, 2026 08:56
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.

CronWorkflow controller may overwrite newer metadata.labels/annotations with stale state during persistUpdate()

2 participants