Skip to content

Conversation

Ongy
Copy link
Contributor

@Ongy Ongy commented Jan 25, 2025

I'm not entirely sure when this happens, but the kustomize can be nil. Accessing Images then leads to a nil-deref.
With this approach, the updater defaults to empty images when it cannot get the real ones.

I'm not entirely sure when this happens, but the kustomize can be nil.
Accessing Images then leads to a nil-deref.
With this approach, the updater defaults to empty images when it cannot get the real ones.

Signed-off-by: Markus Ongyerth <ongyerth@google.com>
@Ongy Ongy force-pushed the dont-access-nil-kustomize branch from a019603 to 2fd1703 Compare January 25, 2025 11:43
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.27%. Comparing base (980eff5) to head (2fd1703).

Files with missing lines Patch % Lines
pkg/argocd/git.go 70.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1020      +/-   ##
==========================================
+ Coverage   63.26%   63.27%   +0.01%     
==========================================
  Files          15       15              
  Lines        2243     2252       +9     
==========================================
+ Hits         1419     1425       +6     
- Misses        733      735       +2     
- Partials       91       92       +1     

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

@chengfang
Copy link
Collaborator

@Ongy thanks for the fix. Can you please fix the DCO check failure by adding git sign-off?

@Ongy
Copy link
Contributor Author

Ongy commented Feb 4, 2025

I already added Signed-off-by: Markus Ongyerth <ongyerth@google.com> to the commit itself. Does it check the message here?

Though it might also be unhappy because that's not the same email as in the commit itself. I'll update that later today and see.

@jannfis jannfis changed the title Prevent segfault with empty kustomization fix: Prevent segfault with empty kustomization Feb 12, 2025
Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

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

I've set the DCO to pass, as I believe we're going to deprecate it soon anyway.

Change LGTM. Thanks!

@jannfis jannfis merged commit add409e into argoproj-labs:master Feb 12, 2025
10 of 11 checks passed
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.

4 participants