Skip to content

Conversation

@sujay2306
Copy link

@sujay2306 sujay2306 commented May 23, 2025

Description

This PR fixes an issue where server-side diff in Argo CD fails to detect changes in custom resources when fields are added or removed. Specifically, it addresses the problem where removing the autoRestart field from a KafkaConnector resource wasn't being detected as a change.

Changes

  1. Modified the diffArrayCached function in util/argo/diff/diff.go to handle custom resources properly:
    • Added detection for custom resources by comparing API versions
    • Forces diff calculation for custom resources instead of using cached results
    • Maintains caching behavior for non-custom resources

Impact

  • Proper detection of field-level changes (additions/removals) in custom resources
  • Maintains performance benefits of caching for non-custom resources
  • Ensures accurate diff detection when modifying fields like autoRestart in KafkaConnector

Testing

  • Added test case for KafkaConnector field removal detection
  • Verified that other custom resource field changes are detected correctly
  • Ensured non-custom resources still benefit from caching

Related Issues

#23127
Closes #23127

Documentation

  • No additional documentation changes required as this is a bug fix

Checklist

  • I've created an enhancement proposal and discussed it with the community
  • The title of the PR states what changed and the related issues number
  • I've updated both the CLI and UI to expose my feature
  • I have signed off all my commits as required by DCO
  • I have written unit and e2e tests for my change
  • My build is green
  • I have added a brief description of why this PR is necessary

@sujay2306 sujay2306 requested a review from a team as a code owner May 23, 2025 11:02
@bunnyshell
Copy link

bunnyshell bot commented May 23, 2025

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

@sujay2306 sujay2306 changed the title Fix server-side diff detection for custom resources #23127 Fix server-side diff detection for custom resources May 23, 2025
@sujay2306 sujay2306 changed the title Fix server-side diff detection for custom resources fix(diff): improve server-side diff detection for custom resources May 23, 2025
@blakepettersson
Copy link
Member

Hasn't this been fixed with #23066? Can you verify if running a build from master still has the initial issue or not?

@codecov
Copy link

codecov bot commented May 23, 2025

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@0484f9f). Learn more about missing BASE report.
⚠️ Report is 428 commits behind head on master.

Files with missing lines Patch % Lines
util/argo/diff/diff.go 78.57% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #23128   +/-   ##
=========================================
  Coverage          ?   60.04%           
=========================================
  Files             ?      343           
  Lines             ?    57896           
  Branches          ?        0           
=========================================
  Hits              ?    34763           
  Misses            ?    20362           
  Partials          ?     2771           

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

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.

ServerSideDiff not detecting diff in manifests

2 participants