Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Fix a nil pointer crash for repo server #10696

Merged
merged 2 commits into from
Sep 26, 2022
Merged

fix: Fix a nil pointer crash for repo server #10696

merged 2 commits into from
Sep 26, 2022

Conversation

cleverhu
Copy link
Contributor

@cleverhu cleverhu commented Sep 25, 2022 •
edited

Fixes: #10614

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:

  • 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).
  • 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.
  • Optional. My organization is added to USERS.md.
  • 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).

@codecov
Copy link

codecov bot commented Sep 25, 2022 •
edited

Codecov Report

Base: 45.75% // Head: 45.76% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (4c0ee38) compared to base (02bf903).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #10696   +/-   ##
=======================================
  Coverage   45.75%   45.76%           
=======================================
  Files         236      236           
  Lines       28527    28529    +2     
=======================================
+ Hits        13053    13055    +2     
  Misses      13669    13669           
  Partials     1805     1805
Impacted Files Coverage Δ
reposerver/repository/repository.go 61.11% <100.00%> (+0.07%) ⬆️
util/settings/settings.go 51.36% <0.00%> (ø)

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jannfis
Copy link
Member

jannfis commented Sep 25, 2022

Thanks! Would you also be able to add a unit test for this fix, to ensure there'll be no regression in the future?

@cleverhu
Copy link
Contributor Author

Thanks! Would you also be able to add a unit test for this fix, to ensure there'll be no regression in the future?

Okay, I'll try to add a unit test for this fix.

@cleverhu cleverhu changed the title add nil pointer judgment for repo server fix nil pointer crash off for repo server Sep 25, 2022
@cleverhu cleverhu changed the title fix nil pointer crash off for repo server fix nil pointer crashoff for repo server Sep 25, 2022
add nil pointer judgment for repo server
8dd614e 
Fixes: argoproj#10614
Signed-off-by: cleverhu <shouping.hu@daocloud.io>
@cleverhu
Copy link
Contributor Author

cleverhu commented Sep 25, 2022 •
edited

@jannfis I have modified the file to cover this situation which actually is a bug. PTAL. Thanks.

@jannfis jannfis changed the title fix nil pointer crashoff for repo server fix: Fix a nil pointer crash for repo server Sep 26, 2022
@jannfis jannfis added the cherry-pick/2.4 Candidate for cherry picking into the 2.4 release branch label Sep 26, 2022
jannfis
jannfis approved these changes Sep 26, 2022
View reviewed changes
Copy link
Member

@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.

LGTM!

Thanks @cleverhu

@jannfis
Copy link
Member

jannfis commented Sep 26, 2022

@crenshaw-dev @alexmt FYI, will cherry-pick this into release-2.4 once merged.

@jannfis jannfis enabled auto-merge (squash) September 26, 2022 06:57
@jannfis jannfis merged commit 49487ea into argoproj:master Sep 26, 2022
jannfis added a commit that referenced this pull request Sep 26, 2022
@cleverhu @jannfis
fix: Fix a nil pointer crash for repo server (#10696)
712df99 
Fixes: #10614
Signed-off-by: cleverhu <shouping.hu@daocloud.io>

Signed-off-by: cleverhu <shouping.hu@daocloud.io>
Co-authored-by: jannfis <jann@mistrust.net>
ashutosh16 pushed a commit to ashutosh16/argo-cd that referenced this pull request Oct 7, 2022
@cleverhu @jannfis @ashutosh16
fix: Fix a nil pointer crash for repo server (argoproj#10696)
b3ef34b 
Fixes: argoproj#10614
Signed-off-by: cleverhu <shouping.hu@daocloud.io>

Signed-off-by: cleverhu <shouping.hu@daocloud.io>
Co-authored-by: jannfis <jann@mistrust.net>
crenshaw-dev pushed a commit that referenced this pull request Dec 2, 2022
@cleverhu @jannfis @crenshaw-dev
fix: Fix a nil pointer crash for repo server (#10696)
ff8cd75 
Fixes: #10614
Signed-off-by: cleverhu <shouping.hu@daocloud.io>

Signed-off-by: cleverhu <shouping.hu@daocloud.io>
Co-authored-by: jannfis <jann@mistrust.net>
@crenshaw-dev
Copy link
Collaborator

Cherry-picked onto release-2.3 for 2.3.12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jannfis jannfis jannfis approved these changes

Assignees
No one assigned
Labels
cherry-pick/2.4 Candidate for cherry picking into the 2.4 release branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Repo server crashing (segmentation violation) on null values
3 participants
@cleverhu @jannfis @crenshaw-dev