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

Prevent panic when cloning empty git repository #1021

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

relu
Copy link
Member

@relu relu commented Feb 6, 2023

This covers the edge case in which a user creates a GitRepository CR referencing an empty Git repository. Currently, the controller will panic in this situation since the returned commit pointer is nil.

@hiddeco
Copy link
Member

hiddeco commented Feb 7, 2023

Think this needs to be solved in the pkg/git library, as it appears to be a regression: fluxcd/pkg#393

This actually does the right thing but was not taken into account while updating the package, as code doc clearly states:

If the repository is empty, it returns a nil Commit.

@hiddeco hiddeco added bug Something isn't working area/git Git related issues and pull requests labels Feb 7, 2023
@relu relu force-pushed the handle-empty-git-repository branch from b054fbd to c8fdc36 Compare February 7, 2023 12:21
@relu relu force-pushed the handle-empty-git-repository branch from c8fdc36 to 2f69bf0 Compare February 7, 2023 12:30
This covers the edge case in which a user creates a GitRepository CR
referencing an empty Git repository. Currently, the controller will panic
in this situation since the returned commit pointer is nil.

Signed-off-by: Aurel Canciu <[email protected]>
@relu relu force-pushed the handle-empty-git-repository branch from 2f69bf0 to 14a4a5e Compare February 7, 2023 12:53
Copy link
Contributor

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

Based on fluxcd/pkg#393 (comment), if and when the git package is updated, we can update the fix to adapt to it.

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

+1 for @darkowlzz comment

Thanks @relu 🙇

@relu relu merged commit 5a01112 into main Feb 7, 2023
@relu relu deleted the handle-empty-git-repository branch February 7, 2023 13:52
@thebearingedge
Copy link

i did not just spend 6 hours syncing with the wrong remote 🥲

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants