New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: setting spec.SyncPolicy crashes 'argocd appset get' output (#12424) #12425
fix: setting spec.SyncPolicy crashes 'argocd appset get' output (#12424) #12425
Conversation
Codecov ReportBase: 47.71% // Head: 47.77% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #12425 +/- ##
==========================================
+ Coverage 47.71% 47.77% +0.05%
==========================================
Files 246 246
Lines 41908 41911 +3
==========================================
+ Hits 19996 20021 +25
+ Misses 19914 19891 -23
- Partials 1998 1999 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This makes sense to me, is there any chance you could add unit tests for this fix (I'm aware that most of the CLI commands are lacking unit tests, but the boy scout rule and all of that? 😄)? |
95c93ee
to
9ea0a9a
Compare
Thanks @blakepettersson for eyeing this! I updated the code with a test to test all permutations of syncPolicies just in case. :) Please let me know if anything else needs to be changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 😄
58d06cf
to
06a5cd2
Compare
…proj#12424) Signed-off-by: Vaibhav Bhembre <vaibhav@digitalocean.com>
06a5cd2
to
1b13627
Compare
…) (#12425) Signed-off-by: Vaibhav Bhembre <vaibhav@digitalocean.com>
…) (#12425) Signed-off-by: Vaibhav Bhembre <vaibhav@digitalocean.com>
Cherry-picked onto release-2.5 for 2.5.12, and release-2.6 for 2.6.3. |
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist:
Fixes: #12424
Previously we didn't explicitly check for the presence of
spec.template.spec.SyncPolicy
before accessing its.Automated
struct variable which led to a nil pointer dereference issue.