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.

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: handle nil repo in GitHub event payloads #1873

Merged
merged 4 commits into from
Dec 19, 2024

Conversation

chmouel
Copy link
Member

@chmouel chmouel commented Dec 18, 2024

We were getting a crash on the controller due to a nil pointer dereference
when the repository was nil in the payload. This commit adds checks to
ensure the repository is not nil before accessing its properties.

TestSkippedEvent shows the issue but it wasn't detected that the controller
was crashing and happily returned ok.

Signed-off-by: Chmouel Boudjnah [email protected]

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 40.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 65.72%. Comparing base (7db78c0) to head (8110393).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
pkg/provider/github/parse_payload.go 40.00% 6 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1873      +/-   ##
==========================================
- Coverage   65.74%   65.72%   -0.03%     
==========================================
  Files         178      178              
  Lines       13836    13851      +15     
==========================================
+ Hits         9097     9103       +6     
- Misses       4124     4133       +9     
  Partials      615      615              

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

@chmouel chmouel force-pushed the fix-crash-with-bad-payload branch from a701846 to 8110393 Compare December 19, 2024 08:47
@vdemeester vdemeester force-pushed the fix-crash-with-bad-payload branch from 6924457 to 8230ae3 Compare December 19, 2024 11:35
- Add logging for pipeline run creation and cancellation
- Update wait options for pipeline run creation
- Replace check run status check with pipeline run status check
- Ensure proper handling of pipeline run cancellation status
- Introduced DEFAULT_GO_TEST_FLAGS for default test flags
- Added comments for vendor and other targets
- Removed redundant test-unit targets
- Updated test-e2e to use DEFAULT_GO_TEST_FLAGS
- Added gitlint target for commit message linting
- Minor formatting and cleanup
@chmouel chmouel force-pushed the fix-crash-with-bad-payload branch from 8230ae3 to 7fe2797 Compare December 19, 2024 12:28
We were getting a crash on the controller due to a nil pointer dereference
when the repository was nil in the payload. This commit adds checks to
ensure the repository is not nil before accessing its properties.

TestSkippedEvent shows the issue but it wasn't detected that the controller
was crashing and happily returned ok.
printing the whole log file is not useful, as it can be very long to
scoll at times.
@chmouel chmouel force-pushed the fix-crash-with-bad-payload branch from 7fe2797 to 7bf19a0 Compare December 19, 2024 13:35
@chmouel chmouel merged commit 3042980 into openshift-pipelines:main Dec 19, 2024
5 checks passed
@chmouel chmouel deleted the fix-crash-with-bad-payload branch December 19, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants