Skip to content

fix: guard empty Values in injected anti-affinity check#4832

Open
AruneshDwivedi wants to merge 1 commit into
argoproj:masterfrom
AruneshDwivedi:fix/anti-affinity-empty-values-panic
Open

fix: guard empty Values in injected anti-affinity check#4832
AruneshDwivedi wants to merge 1 commit into
argoproj:masterfrom
AruneshDwivedi:fix/anti-affinity-empty-values-panic

Conversation

@AruneshDwivedi

Copy link
Copy Markdown

IfInjectedAntiAffinityRuleNeedsUpdate keyed a pod affinity term match by label only (via HasInjectedAntiAffinityRule) and then indexed Values[0]. A user-defined anti-affinity term using the rollouts-pod-template-hash key with Operator: Exists (no Values) made HasInjectedAntiAffinityRule match it, causing an index-out-of-range panic during reconcile.

Guard the access with a length check so such terms are treated as not needing an update.

Added a regression test in TestIfInjectedAntiAffinityRuleNeedsUpdate covering a term with no Values.

IfInjectedAntiAffinityRuleNeedsUpdate keyed a pod affinity term match by
label only (via HasInjectedAntiAffinityRule) and then indexed Values[0]. A
user-defined anti-affinity term using the rollouts-pod-template-hash key with
Operator: Exists (no Values) made HasInjectedAntiAffinityRule match it, causing
an index-out-of-range panic during reconcile. Guard the access with a length
check so such terms are treated as not needing an update.

Signed-off-by: Arunesh Dwivedi <arunesh.devops@gmail.com>
@AruneshDwivedi AruneshDwivedi requested a review from a team as a code owner July 9, 2026 05:31
@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

@AruneshDwivedi AruneshDwivedi changed the title fix(replicaset): guard empty Values in injected anti-affinity check fix: guard empty Values in injected anti-affinity check Jul 9, 2026
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.01%. Comparing base (bc55226) to head (3aa0c07).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4832      +/-   ##
==========================================
- Coverage   85.04%   85.01%   -0.04%     
==========================================
  Files         166      166              
  Lines       19136    19136              
==========================================
- Hits        16275    16269       -6     
- Misses       2009     2012       +3     
- Partials      852      855       +3     
Flag Coverage Δ
e2e 52.80% <0.00%> (-0.04%) ⬇️
unit-tests 81.39% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Published E2E Test Results

  4 files    4 suites   3h 51m 35s ⏱️
122 tests 112 ✅  7 💤  3 ❌
506 runs  462 ✅ 28 💤 16 ❌

For more details on these failures, see this check.

Results for commit 3aa0c07.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Published Unit Test Results

2 516 tests   2 516 ✅  3m 24s ⏱️
  130 suites      0 💤
    1 files        0 ❌

Results for commit 3aa0c07.

@kostis-codefresh kostis-codefresh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there an open issue about this?

Could you please post an example that showcases the problem first?

@AruneshDwivedi

Copy link
Copy Markdown
Author

There isn't a separate issue open — this is the fix for a crash we hit in reconcile. Here's what triggers it.

HasInjectedAntiAffinityRule matches a pod anti-affinity term purely by key (it checks labelSelectorRequirement.Key == v1alpha1.DefaultRolloutUniqueLabelKey), without looking at the operator or values. So a user-supplied term like this gets matched:

affinity:
  podAntiAffinity:
    requiredDuringSchedulingIgnoredDuringExecution:
      - labelSelector:
          matchExpressions:
            - key: rollouts-pod-template-hash
              operator: Exists        # no values
        topologyKey: kubernetes.io/hostname

Then IfInjectedAntiAffinityRuleNeedsUpdate does:

if labelSelectorRequirement.Key == v1alpha1.DefaultRolloutUniqueLabelKey && labelSelectorRequirement.Values[0] != rollout.Status.StableRS {

operator: Exists produces a MatchExpression with an empty Values slice, so Values[0] is an index-out-of-range panic — not a "this is wrong" check, an actual crash on every reconcile once a user adds such a term.

The fix guards with len(labelSelectorRequirement.Values) > 0 so terms without values are treated as not needing an update (matching the existing regression test added in TestIfInjectedAntiAffinityRuleNeedsUpdate).

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.

2 participants