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

Upgrade libgit2 to 1.3.0 and git2go to V33 #573

Merged
merged 5 commits into from
Feb 16, 2022
Merged

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Feb 8, 2022

Fixes #399 fluxcd/image-automation-controller#298
Relates to #397 #490 fluxcd/image-automation-controller#186 fluxcd/image-automation-controller#281
Depends on fluxcd/golang-with-libgit2#17

@hiddeco hiddeco added area/ci CI related issues and pull requests area/git Git related issues and pull requests labels Feb 9, 2022
@pjbgf pjbgf force-pushed the bump-libgit2 branch 2 times, most recently from a13a419 to 396c862 Compare February 9, 2022 11:43
@pjbgf pjbgf changed the title Bump libgit2 to 1.3.0 and git2go to V33 Upgrade libgit2 to 1.3.0 and git2go to V33 Feb 9, 2022
@pjbgf pjbgf marked this pull request as ready for review February 10, 2022 09:44
pkg/git/libgit2/transport.go Outdated Show resolved Hide resolved
}
select {
case <-ctx.Done():
return git2go.ErrorCodeUser
return fmt.Errorf("transport close - potentially due to a timeout")
Copy link
Member

Choose a reason for hiding this comment

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

I feel for some reason weird about the - in the error message, but not really sure if something like this would be better:

Suggested change
return fmt.Errorf("transport close - potentially due to a timeout")
return fmt.Errorf("transport close (potentially due to a timeout)")

roots := x509.NewCertPool()
if ok := roots.AppendCertsFromPEM(caBundle); !ok {
return git2go.ErrorCodeCertificate
return fmt.Errorf("x509 cert could not be appended")
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if something like this would be more useful to the end-user:

Suggested change
return fmt.Errorf("x509 cert could not be appended")
return fmt.Errorf("PEM CA bundle could not be appended to x509 cert pool")

@@ -167,20 +167,20 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback {
CurrentTime: now(),
}
if _, err := cert.X509.Verify(opts); err != nil {
return git2go.ErrorCodeCertificate
return fmt.Errorf("x509 cert could not be verified")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("x509 cert could not be verified")
return fmt.Errorf("x509 cert could not be verified: %w", err)

pkg/git/libgit2/checkout_test.go Outdated Show resolved Hide resolved
@pjbgf pjbgf force-pushed the bump-libgit2 branch 2 times, most recently from 5ef9655 to 9b1c489 Compare February 16, 2022 09:26
Paulo Gomes added 4 commits February 16, 2022 10:17
Downstream breaking changes introduced since git2go@V31:
- git2go.ErrorCode was deprecated in favour of the native error type.
- FetchOptions no longer expects a pointer, but rather the actual value of git2go.FetchOptions.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
This adds a test to detect any regression in libgit2's ED25519 key
support. go-git supports ED25519 but not the current version of
libgit2 used in flux. The updates to libgit2 in v1.2.0 adds support
for ED25519. This test would help ensure the right version of libgit2
is used.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
The environment variables set at the Makefile were causing go install to
yield a corrupted file for setup-envtest. To fix the issue, such operation
is now always executed in a clean bash.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
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.

Thanks a lot for following through on the nitpicks, and the upgrade itself 🙇 💯 🥇

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

Successfully merging this pull request may close these issues.

"Unable to extract public key from private key" for ed25519 & libgit2
2 participants