Skip to content

Conversation

@ChristianCiach
Copy link
Contributor

@ChristianCiach ChristianCiach commented Jun 22, 2025

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

This is basically an improved version of #15661. That PR fixed the case where an empty yaml file wasn't distinguished from a missing file, which could easily lead to deleted applications when a user commented out all entries from their yaml file.

Unfortunately, that PR didn't distinguish between an empty yaml file from yaml files containing a literal empty json array ([]) as the root element. Using arrays as the top-level structure for yaml files has long been supported by the git generator, but it seems to be an undocumented feature (or at least I couldn't find any documentation).

If a yaml file contains an array as the top level element, the Git generator outputs a separate params-object for each element of the array. In other words, this feature can be used to easily template multiple applications from a single yaml file without the use of a matrix generator, as a user does this here:

Since this is a supported use-case, it stands to reason that an empty array ([]) should result in an empty slice of objects returned by the git generator. But this has been broken since v2.10 due to #15661 failing to distinguish between an empty file (or {}) from an empty array, because the code previously tried to unmarshal the data into a slice, which even succeeds for empty files.

To fix this I've just reversed the order of parsing attempts: This PR first tries to unmarshal the data into a map, which even succeeds for empty files! This also makes the code a bit cleaner, since there is no need for constructing a dummy slice entry anymore. Only if parsing as a map fails (which is also the case for []) we fall back to trying to unmarshal the data into a slice.

I think it's also nice that the most common use case (yaml containing an object) is now tried first, while arrays are only attempted as a fallback. This slightly changed the returned error messsages, but no tests needed to be adjusted aside from that. I've added a unit test that fails with the previous implementation.

Kindly pinging @agaudreault because this builds on his previous PR.

Please see the comments of the linked issue for more details about the use-case and the cause of this issue.

Closes #23500

@ChristianCiach ChristianCiach requested a review from a team as a code owner June 22, 2025 12:02
@bunnyshell
Copy link

bunnyshell bot commented Jun 22, 2025

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

@ChristianCiach ChristianCiach force-pushed the git-generator-fix-empty-array branch 2 times, most recently from 25d29bb to eb40c3d Compare June 22, 2025 12:09
@ChristianCiach ChristianCiach force-pushed the git-generator-fix-empty-array branch from eb40c3d to 7935218 Compare June 22, 2025 12:10
@codecov
Copy link

codecov bot commented Jun 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.25%. Comparing base (5c9a5ef) to head (7935218).
⚠️ Report is 431 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #23513      +/-   ##
==========================================
- Coverage   60.25%   60.25%   -0.01%     
==========================================
  Files         344      344              
  Lines       59084    59082       -2     
==========================================
- Hits        35604    35597       -7     
+ Misses      20611    20610       -1     
- Partials     2869     2875       +6     

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

Copy link
Member

@rumstead rumstead left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@nitishfy nitishfy left a comment

Choose a reason for hiding this comment

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

LGTM!

@rumstead rumstead merged commit 460111f into argoproj:master Jun 24, 2025
28 checks passed
cjcocokrisp pushed a commit to cjcocokrisp/argo-cd that referenced this pull request Jun 24, 2025
enneitex pushed a commit to enneitex/argo-cd that referenced this pull request Aug 24, 2025
…teral empty json array (argoproj#23500) (argoproj#23513)

Signed-off-by: Christian Ciach <[email protected]>
Signed-off-by: enneitex <[email protected]>
Mangaal pushed a commit to Mangaal/argo-cd that referenced this pull request Sep 10, 2025
…teral empty json array (argoproj#23500) (argoproj#23513)

Signed-off-by: Christian Ciach <[email protected]>
Signed-off-by: Mangaal <[email protected]>
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.

Git Generator with empty config.json file not working

3 participants