From 5a1fcc213bba793ffcbe0a6e321d1be02f48ddde Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Sat, 23 Oct 2021 19:41:49 +0200 Subject: [PATCH] git: standardise commit and (PGP) verification This commit refactors the previous `Commit` interface into a standardised `Commit` struct. This object contains sufficient information for referencing, observating and (PGP) verification. - `libgit2` commit checkout does now return `HEAD/` as the branch is not taken into account. - `git2go` objects are now properly `Free`d everywhere - `Verify` logic is tested. Signed-off-by: Hidde Beydals --- controllers/gitrepository_controller.go | 16 +- controllers/gitrepository_controller_test.go | 6 +- controllers/helmchart_controller.go | 4 +- go.mod | 1 + pkg/git/git.go | 74 ++++++- pkg/git/git_test.go | 220 +++++++++++++++++++ pkg/git/gogit/checkout.go | 124 +++++++---- pkg/git/gogit/checkout_test.go | 44 ++-- pkg/git/gogit/commit.go | 51 ----- pkg/git/libgit2/checkout.go | 102 +++++---- pkg/git/libgit2/checkout_test.go | 45 ++-- pkg/git/libgit2/commit.go | 65 ------ 12 files changed, 497 insertions(+), 255 deletions(-) create mode 100644 pkg/git/git_test.go delete mode 100644 pkg/git/gogit/commit.go delete mode 100644 pkg/git/libgit2/commit.go diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index c3381dce..b0939974 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -264,12 +264,11 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour gitCtx, cancel := context.WithTimeout(ctx, repository.Spec.Timeout.Duration) defer cancel() - commit, revision, err := checkoutStrategy.Checkout(gitCtx, tmpGit, repository.Spec.URL, authOpts) + commit, err := checkoutStrategy.Checkout(gitCtx, tmpGit, repository.Spec.URL, authOpts) if err != nil { return sourcev1.GitRepositoryNotReady(repository, sourcev1.GitOperationFailedReason, err.Error()), err } - - artifact := r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), revision, fmt.Sprintf("%s.tar.gz", commit.Hash())) + artifact := r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), commit.String(), fmt.Sprintf("%s.tar.gz", commit.Hash.String())) // copy all included repository into the artifact includedArtifacts := []*sourcev1.Artifact{} @@ -298,14 +297,17 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour Namespace: repository.Namespace, Name: repository.Spec.Verification.SecretRef.Name, } - var secret corev1.Secret - if err := r.Client.Get(ctx, publicKeySecret, &secret); err != nil { + var secret *corev1.Secret + if err := r.Client.Get(ctx, publicKeySecret, secret); err != nil { err = fmt.Errorf("PGP public keys secret error: %w", err) return sourcev1.GitRepositoryNotReady(repository, sourcev1.VerificationFailedReason, err.Error()), err } - err := commit.Verify(secret) - if err != nil { + var keyRings []string + for _, v := range secret.Data { + keyRings = append(keyRings, string(v)) + } + if _, err = commit.Verify(keyRings...); err != nil { return sourcev1.GitRepositoryNotReady(repository, sourcev1.VerificationFailedReason, err.Error()), err } } diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index c647727c..0ff13da5 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -23,11 +23,9 @@ import ( "net/http" "net/url" "os" - "os/exec" "path" "path/filepath" - "strings" "time" @@ -251,7 +249,7 @@ var _ = Describe("GitRepositoryReconciler", func() { reference: &sourcev1.GitRepositoryRef{SemVer: "1.2.3.4"}, waitForReason: sourcev1.GitOperationFailedReason, expectStatus: metav1.ConditionFalse, - expectMessage: "semver parse range error: improper constraint: 1.2.3.4", + expectMessage: "semver parse error: improper constraint: 1.2.3.4", }), Entry("semver no match", refTestCase{ reference: &sourcev1.GitRepositoryRef{SemVer: "1.0.0"}, @@ -284,7 +282,7 @@ var _ = Describe("GitRepositoryReconciler", func() { }, waitForReason: sourcev1.GitOperationFailedReason, expectStatus: metav1.ConditionFalse, - expectMessage: "git commit 'invalid' not found: object not found", + expectMessage: "failed to resolve commit object for 'invalid': object not found", }), ) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index ee6b93e9..5d4f952c 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -529,7 +529,7 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context, v, err := semver.NewVersion(helmChart.Metadata.Version) if err != nil { - err = fmt.Errorf("semver error: %w", err) + err = fmt.Errorf("semver parse error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } @@ -539,7 +539,7 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context, splitRev := strings.Split(artifact.Revision, "/") v, err := v.SetMetadata(splitRev[len(splitRev)-1]) if err != nil { - err = fmt.Errorf("semver error: %w", err) + err = fmt.Errorf("semver parse error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } diff --git a/go.mod b/go.mod index 2dbe9806..5ec05b08 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( cloud.google.com/go v0.93.3 // indirect cloud.google.com/go/storage v1.16.0 github.com/Masterminds/semver/v3 v3.1.1 + github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7 github.com/cyphar/filepath-securejoin v0.2.2 github.com/fluxcd/pkg/apis/meta v0.10.0 github.com/fluxcd/pkg/gittestserver v0.3.0 diff --git a/pkg/git/git.go b/pkg/git/git.go index aa41f7f1..5fae158b 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -17,16 +17,80 @@ limitations under the License. package git import ( + "bytes" "context" + "fmt" + "strings" + "time" - corev1 "k8s.io/api/core/v1" + "github.com/ProtonMail/go-crypto/openpgp" ) -type Commit interface { - Verify(secret corev1.Secret) error - Hash() string +type Hash []byte + +// String returns the SHA1 Hash as a string. +func (h Hash) String() string { + return string(h) +} + +type Signature struct { + Name string + Email string + When time.Time +} + +type Commit struct { + // Hash is the SHA1 hash of the commit. + Hash Hash + // Reference is the original reference of the commit, for example: + // 'refs/tags/foo'. + Reference string + // Author is the original author of the commit. + Author Signature + // Committer is the one performing the commit, might be different from + // Author. + Committer Signature + // Signature is the PGP signature of the commit. + Signature string + // Encoded is the encoded commit, without any signature. + Encoded []byte + // Message is the commit message, contains arbitrary text. + Message string +} + +// String returns a string representation of the Commit, composed +// out the last part of the Reference element, and/or Hash. +// For example: +// 'tags/a0c14dc8580a23f79bc654faa79c4f62b46c2c22'. +func (c *Commit) String() string { + if short := strings.SplitAfterN(c.Reference, "/", 3); len(short) == 3 { + return fmt.Sprintf("%s/%s", short[2], c.Hash) + } + return fmt.Sprintf("HEAD/%s", c.Hash) +} + +// Verify the Signature of the commit with the given key rings. +// It returns the fingerprint of the key the signature was verified +// with, or an error. +func (c *Commit) Verify(keyRing ...string) (string, error) { + if c.Signature == "" { + return "", fmt.Errorf("commit does not have a PGP signature") + } + + for _, r := range keyRing { + reader := strings.NewReader(r) + keyring, err := openpgp.ReadArmoredKeyRing(reader) + if err != nil { + return "", fmt.Errorf("failed to read armored key ring: %w", err) + } + signer, err := openpgp.CheckArmoredDetachedSignature(keyring, bytes.NewBuffer(c.Encoded), bytes.NewBufferString(c.Signature), nil) + if err == nil { + return fmt.Sprintf("%X", signer.PrimaryKey.Fingerprint[12:20]), nil + } + } + return "", fmt.Errorf("failed to verify commit with any of the given key rings") } type CheckoutStrategy interface { - Checkout(ctx context.Context, path, url string, config *AuthOptions) (Commit, string, error) + Checkout(ctx context.Context, path, url string, config *AuthOptions) (*Commit, error) } diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go new file mode 100644 index 00000000..98894d34 --- /dev/null +++ b/pkg/git/git_test.go @@ -0,0 +1,220 @@ +/* +Copyright 2021 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package git + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +const ( + encodedCommitFixture = `tree f0c522d8cc4c90b73e2bc719305a896e7e3c108a +parent eb167bc68d0a11530923b1f24b4978535d10b879 +author Stefan Prodan 1633681364 +0300 +committer Stefan Prodan 1633681364 +0300 + +Update containerd and runc to fix CVEs + +Signed-off-by: Stefan Prodan +` + + malformedEncodedCommitFixture = `parent eb167bc68d0a11530923b1f24b4978535d10b879 +author Stefan Prodan 1633681364 +0300 +committer Stefan Prodan 1633681364 +0300 + +Update containerd and runc to fix CVEs + +Signed-off-by: Stefan Prodan +` + + signatureCommitFixture = `-----BEGIN PGP SIGNATURE----- + +iHUEABEIAB0WIQQHgExUr4FrLdKzpNYyma6w5AhbrwUCYV//1AAKCRAyma6w5Ahb +r7nJAQCQU4zEJu04/Q0ac/UaL6htjhq/wTDNMeUM+aWG/LcBogEAqFUea1oR2BJQ +JCJmEtERFh39zNWSazQmxPAFhEE0kbc= +=+Wlj +-----END PGP SIGNATURE-----` + + armoredKeyRingFixture = `-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQSuBF9+HgMRDADKT8UBcSzpTi4JXt/ohhVW3x81AGFPrQvs6MYrcnNJfIkPTJD8 +mY5T7j1fkaN5wcf1wnxM9qTcW8BodkWNGEoEYOtVuigLSxPFqIncxK0PHvdU8ths +TEInBrgZv9t6xIVa4QngOEUd2D/aYni7M+75z7ntgj6eU1xLZ60upRFn05862OvJ +rZFUvzjsZXMAO3enCu2VhG/2axCY/5uI8PgWjyiKV2TH4LBJgzlb0v6SyI+fYf5K +Bg2WzDuLKvQBi9tFSwnUbQoFFlOeiGW8G/bdkoJDWeS1oYgSD3nkmvXvrVESCrbT +C05OtQOiDXjSpkLim81vNVPtI2XEug+9fEA+jeJakyGwwB+K8xqV3QILKCoWHKGx +yWcMHSR6cP9tdXCk2JHZBm1PLSJ8hIgMH/YwBJLYg90u8lLAs9WtpVBKkLplzzgm +B4Z4VxCC+xI1kt+3ZgYvYC+oUXJXrjyAzy+J1f+aWl2+S/79glWgl/xz2VibWMz6 +nZUE+wLMxOQqyOsBALsoE6z81y/7gfn4R/BziBASi1jq/r/wdboFYowmqd39DACX ++i+V0OplP2TN/F5JajzRgkrlq5cwZHinnw+IFwj9RTfOkdGb3YwhBt/h2PP38969 +ZG+y8muNtaIqih1pXj1fz9HRtsiCABN0j+JYpvV2D2xuLL7P1O0dt5BpJ3KqNCRw +mGgO2GLxbwvlulsLidCPxdK/M8g9Eeb/xwA5LVwvjVchHkzHuUT7durn7AT0RWiK +BT8iDfeBB9RKienAbWyybEqRaR6/Tv+mghFIalsDiBPbfm4rsNzsq3ohfByqECiy +yUvs2O3NDwkoaBDkA3GFyKv8/SVpcuL5OkVxAHNCIMhNzSgotQ3KLcQc0IREfFCa +3CsBAC7CsE2bJZ9IA9sbBa3jimVhWUQVudRWiLFeYHUF/hjhqS8IHyFwprjEOLaV +EG0kBO6ELypD/bOsmN9XZLPYyI3y9DM6Vo0KMomE+yK/By/ZMxVfex8/TZreUdhP +VdCLL95Rc4w9io8qFb2qGtYBij2wm0RWLcM0IhXWAtjI3B17IN+6hmv+JpiZccsM +AMNR5/RVdXIl0hzr8LROD0Xe4sTyZ+fm3mvpczoDPQNRrWpmI/9OT58itnVmZ5jM +7djV5y/NjBk63mlqYYfkfWto97wkhg0MnTnOhzdtzSiZQRzj+vf+ilLfIlLnuRr1 +JRV9Skv6xQltcFArx4JyfZCo7JB1ZXcbdFAvIXXS11RTErO0XVrXNm2RenpW/yZA +9f+ESQ/uUB6XNuyqVUnJDAFJFLdzx8sO3DXo7dhIlgpFqgQobUl+APpbU5LT95sm +89UrV0Lt9vh7k6zQtKOjEUhm+dErmuBnJo8MvchAuXLagHjvb58vYBCUxVxzt1KG +2IePwJ/oXIfawNEGad9Lmdo1FYG1u53AKWZmpYOTouu92O50FG2+7dBh0V2vO253 +aIGFRT1r14B1pkCIun7z7B/JELqOkmwmlRrUnxlADZEcQT3z/S8/4+2P7P6kXO7X +/TAX5xBhSqUbKe3DhJSOvf05/RVL5ULc2U2JFGLAtmBOFmnD/u0qoo5UvWliI+v/ +47QnU3RlZmFuIFByb2RhbiA8c3RlZmFuLnByb2RhbkBnbWFpbC5jb20+iJAEExEI +ADgWIQQHgExUr4FrLdKzpNYyma6w5AhbrwUCX34eAwIbAwULCQgHAgYVCgkICwIE +FgIDAQIeAQIXgAAKCRAyma6w5Ahbrzu/AP9l2YpRaWZr6wSQuEn0gMN8DRzsWJPx +pn0akdY7SRP3ngD9GoKgu41FAItnHAJ2KiHv/fHFyHMndNP3kPGPNW4BF+65Aw0E +X34eAxAMAMdYFCHmVA8TZxSTMBDpKYave8RiDCMMMjk26Gl0EPN9f2Y+s5++DhiQ +hojNH9VmJkFwZX1xppxe1y1aLa/U6fBAqMP/IdNH8270iv+A9YIxdsWLmpm99BDO +3suRfsHcOe9T0x/CwRfDNdGM/enGMhYGTgF4VD58DRDE6WntaBhl4JJa300NG6X0 +GM4Gh59DKWDnez/Shulj8demlWmakP5imCVoY+omOEc2k3nH02U+foqaGG5WxZZ+ +GwEPswm2sBxvn8nwjy9gbQwEtzNI7lWYiz36wCj2VS56Udqt+0eNg8WzocUT0XyI +moe1qm8YJQ6fxIzaC431DYi/mCDzgx4EV9ww33SXX3Yp2NL6PsdWJWw2QnoqSMpM +z5otw2KlMgUHkkXEKs0apmK4Hu2b6KD7/ydoQRFUqR38Gb0IZL1tOL6PnbCRUcig +Aypy016W/WMCjBfQ8qxIGTaj5agX2t28hbiURbxZkCkz+Z3OWkO0Rq3Y2hNAYM5s +eTn94JIGGwADBgv/dbSZ9LrBvdMwg8pAtdlLtQdjPiT1i9w5NZuQd7OuKhOxYTEB +NRDTgy4/DgeNThCeOkMB/UQQPtJ3Et45S2YRtnnuvfxgnlz7xlUn765/grtnRk4t +ONjMmb6tZos1FjIJecB/6h4RsvUd2egvtlpD/Z3YKr6MpNjWg4ji7m27e9pcJfP6 +YpTDrq9GamiHy9FS2F2pZlQxriPpVhjCLVn9tFGBIsXNxxn7SP4so6rJBmyHEAlq +iym9wl933e0FIgAw5C1vvprYu2amk+jmVBsJjjCmInW5q/kWAFnFaHBvk+v+/7tX +hywWUI7BqseikgUlkgJ6eU7E9z1DEyuS08x/cViDoNh2ntVUhpnluDu48pdqBvvY +a4uL/D+KI84THUAJ/vZy+q6G3BEb4hI9pFjgrdJpUKubxyZolmkCFZHjV34uOcTc +LQr28P8xW8vQbg5DpIsivxYLqDGXt3OyiItxvLMtw/ypt6PkoeP9A4KDST4StITE +1hrOrPtJ/VRmS2o0iHgEGBEIACAWIQQHgExUr4FrLdKzpNYyma6w5AhbrwUCX34e +AwIbDAAKCRAyma6w5Ahbr6QWAP9/pl2R6r1nuCnXzewSbnH1OLsXf32hFQAjaQ5o +Oomb3gD/TRf/nAdVED+k81GdLzciYdUGtI71/qI47G0nMBluLRE= +=/4e+ +-----END PGP PUBLIC KEY BLOCK----- +` + + keyRingFingerprintFixture = "3299AEB0E4085BAF" + + malformedKeyRing = ` +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQSuBF9+HgMRDADKT8UBcSzpTi4JXt/ohhVW3x81AGFPrQvs6MYrcnNJfIkPTJD8 +mY5T7j1fkaN5wcf1wnxM9qTcW8BodkWNGEoEYOtVuigLSxPFqIncxK0PHvdU8ths +TEInBrgZv9t6xIVa4QngOEUd2D/aYni7M+75z7ntgj6eU1xLZ60upRFn05862OvJ +rZFUvzjsZXMAO3enCu2VhG/2axCY/5uI8PgWjyiKV2TH4LBJgzlb0v6SyI+fYf5K +Bg2WzDuLKvQBi9tFSwnUbQoFFlOeiGW8G/bdkoJDWeS1oYgSD3nkmvXvrVESCrbT +-----END PGP PUBLIC KEY BLOCK----- +` +) + +func TestCommit_String(t *testing.T) { + tests := []struct { + name string + commit *Commit + want string + }{ + { + name: "Reference and commit", + commit: &Commit{ + Hash: []byte("commit"), + Reference: "refs/heads/main", + }, + want: "main/commit", + }, + { + name: "Reference with slash and commit", + commit: &Commit{ + Hash: []byte("commit"), + Reference: "refs/heads/feature/branch", + }, + want: "feature/branch/commit", + }, + { + name: "No reference", + commit: &Commit{ + Hash: []byte("commit"), + }, + want: "HEAD/commit", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(tt.commit.String()).To(Equal(tt.want)) + }) + } +} + +func TestCommit_Verify(t *testing.T) { + tests := []struct { + name string + commit *Commit + keyRings []string + want string + wantErr string + }{ + { + name: "Valid commit signature", + commit: &Commit{ + Encoded: []byte(encodedCommitFixture), + Signature: signatureCommitFixture, + }, + keyRings: []string{armoredKeyRingFixture}, + want: keyRingFingerprintFixture, + }, + { + name: "Malformed encoded commit", + commit: &Commit{ + Encoded: []byte(malformedEncodedCommitFixture), + Signature: signatureCommitFixture, + }, + keyRings: []string{armoredKeyRingFixture}, + wantErr: "failed to verify commit with any of the given key rings", + }, + { + name: "Malformed key ring", + commit: &Commit{ + Encoded: []byte(encodedCommitFixture), + Signature: signatureCommitFixture, + }, + keyRings: []string{malformedKeyRing}, + wantErr: "failed to read armored key ring: unexpected EOF", + }, + { + name: "Missing signature", + commit: &Commit{ + Encoded: []byte(encodedCommitFixture), + }, + keyRings: []string{armoredKeyRingFixture}, + wantErr: "commit does not have a PGP signature", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := tt.commit.Verify(tt.keyRings...) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(got).To(BeEmpty()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} diff --git a/pkg/git/gogit/checkout.go b/pkg/git/gogit/checkout.go index 5744654e..96818cac 100644 --- a/pkg/git/gogit/checkout.go +++ b/pkg/git/gogit/checkout.go @@ -18,7 +18,9 @@ package gogit import ( "context" + "errors" "fmt" + "io/ioutil" "sort" "time" @@ -27,6 +29,7 @@ import ( "github.com/fluxcd/pkg/version" extgogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/object" sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" "github.com/fluxcd/source-controller/pkg/git" @@ -58,11 +61,12 @@ type CheckoutBranch struct { recurseSubmodules bool } -func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { authMethod, err := transportAuth(opts) if err != nil { - return nil, "", fmt.Errorf("could not construct auth method: %w", err) + return nil, fmt.Errorf("failed to construct auth method with options: %w", err) } + ref := plumbing.NewBranchReferenceName(c.branch) repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{ URL: url, Auth: authMethod, @@ -77,17 +81,17 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g CABundle: caBundle(opts), }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.GoGitError(err)) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.GoGitError(err)) } head, err := repo.Head() if err != nil { - return nil, "", fmt.Errorf("git resolve HEAD error: %w", err) + return nil, fmt.Errorf("failed to resolve HEAD of branch '%s': %w", c.branch, err) } - commit, err := repo.CommitObject(head.Hash()) + cc, err := repo.CommitObject(head.Hash()) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", head.Hash(), err) + return nil, fmt.Errorf("failed to resolve commit object for HEAD '%s': %w", head.Hash(), err) } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.branch, head.Hash().String()), nil + return commitWithRef(cc, ref) } type CheckoutTag struct { @@ -95,11 +99,12 @@ type CheckoutTag struct { recurseSubmodules bool } -func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { authMethod, err := transportAuth(opts) if err != nil { - return nil, "", fmt.Errorf("could not construct auth method: %w", err) + return nil, fmt.Errorf("failed to construct auth method with options: %w", err) } + ref := plumbing.NewTagReferenceName(c.tag) repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{ URL: url, Auth: authMethod, @@ -114,17 +119,17 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git. CABundle: caBundle(opts), }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.GoGitError(err)) } head, err := repo.Head() if err != nil { - return nil, "", fmt.Errorf("git resolve HEAD error: %w", err) + return nil, fmt.Errorf("failed to resolve HEAD of tag '%s': %w", c.tag, err) } - commit, err := repo.CommitObject(head.Hash()) + cc, err := repo.CommitObject(head.Hash()) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", head.Hash(), err) + return nil, fmt.Errorf("failed to resolve commit object for HEAD '%s': %w", head.Hash(), err) } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.tag, head.Hash().String()), nil + return commitWithRef(cc, ref) } type CheckoutCommit struct { @@ -133,16 +138,17 @@ type CheckoutCommit struct { recurseSubmodules bool } -func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { authMethod, err := transportAuth(opts) if err != nil { - return nil, "", fmt.Errorf("could not construct transportAuth method: %w", err) + return nil, fmt.Errorf("failed to construct auth method with options: %w", err) } + ref := plumbing.NewBranchReferenceName(c.branch) repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{ URL: url, Auth: authMethod, RemoteName: git.DefaultOrigin, - ReferenceName: plumbing.NewBranchReferenceName(c.branch), + ReferenceName: ref, SingleBranch: true, NoCheckout: false, RecurseSubmodules: recurseSubmodules(c.recurseSubmodules), @@ -151,24 +157,26 @@ func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *g CABundle: caBundle(opts), }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.GoGitError(err)) } w, err := repo.Worktree() if err != nil { - return nil, "", fmt.Errorf("git worktree error: %w", err) + return nil, fmt.Errorf("failed to open Git worktree: %w", err) } - commit, err := repo.CommitObject(plumbing.NewHash(c.commit)) + f, _ := repo.Head() + f.String() + cc, err := repo.CommitObject(plumbing.NewHash(c.commit)) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", c.commit, err) + return nil, fmt.Errorf("failed to resolve commit object for '%s': %w", c.commit, err) } err = w.Checkout(&extgogit.CheckoutOptions{ - Hash: commit.Hash, + Hash: cc.Hash, Force: true, }) if err != nil { - return nil, "", fmt.Errorf("git checkout error: %w", err) + return nil, fmt.Errorf("failed to checkout commit '%s': %w", c.commit, err) } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.branch, commit.Hash.String()), nil + return commitWithRef(cc, ref) } type CheckoutSemVer struct { @@ -176,15 +184,17 @@ type CheckoutSemVer struct { recurseSubmodules bool } -func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { verConstraint, err := semver.NewConstraint(c.semVer) if err != nil { - return nil, "", fmt.Errorf("semver parse range error: %w", err) + return nil, fmt.Errorf("semver parse error: %w", err) } + authMethod, err := transportAuth(opts) if err != nil { - return nil, "", fmt.Errorf("could not construct transportAuth method: %w", err) + return nil, fmt.Errorf("failed to construct auth method with options: %w", err) } + repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{ URL: url, Auth: authMethod, @@ -197,12 +207,12 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g CABundle: caBundle(opts), }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, err) } repoTags, err := repo.Tags() if err != nil { - return nil, "", fmt.Errorf("git list tags error: %w", err) + return nil, fmt.Errorf("failed to list tags: %w", err) } tags := make(map[string]string) @@ -222,7 +232,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g tags[t.Name().Short()] = t.Strings()[1] return nil }); err != nil { - return nil, "", err + return nil, err } var matchedVersions semver.Collection @@ -237,7 +247,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g matchedVersions = append(matchedVersions, v) } if len(matchedVersions) == 0 { - return nil, "", fmt.Errorf("no match found for semver: %s", c.semVer) + return nil, fmt.Errorf("no match found for semver: %s", c.semVer) } // Sort versions @@ -260,27 +270,61 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g w, err := repo.Worktree() if err != nil { - return nil, "", fmt.Errorf("git worktree error: %w", err) + return nil, fmt.Errorf("failed to open Git worktree: %w", err) } + ref := plumbing.NewTagReferenceName(t) err = w.Checkout(&extgogit.CheckoutOptions{ - Branch: plumbing.NewTagReferenceName(t), + Branch: ref, }) if err != nil { - return nil, "", fmt.Errorf("git checkout error: %w", err) + return nil, fmt.Errorf("failed to checkout tag '%s': %w", t, err) } - head, err := repo.Head() if err != nil { - return nil, "", fmt.Errorf("git resolve HEAD error: %w", err) + return nil, fmt.Errorf("failed to resolve HEAD of tag '%s': %w", t, err) } - - commit, err := repo.CommitObject(head.Hash()) + cc, err := repo.CommitObject(head.Hash()) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", head.Hash(), err) + return nil, fmt.Errorf("failed to resolve commit object for HEAD '%s': %w", head.Hash(), err) + } + return commitWithRef(cc, ref) +} + +func commitWithRef(c *object.Commit, ref plumbing.ReferenceName) (*git.Commit, error) { + if c == nil { + return nil, errors.New("failed to construct commit: no object") } - return &Commit{commit}, fmt.Sprintf("%s/%s", t, head.Hash().String()), nil + // Encode commit components, excluding signature into SignedData.. + encoded := &plumbing.MemoryObject{} + if err := c.EncodeWithoutSignature(encoded); err != nil { + return nil, fmt.Errorf("failed to encode commit '%s': %w", c.Hash, err) + } + reader, err := encoded.Reader() + if err != nil { + return nil, fmt.Errorf("failed to encode commit '%s': %w", c.Hash, err) + } + b, err := ioutil.ReadAll(reader) + if err != nil { + return nil, fmt.Errorf("failed to read encoded commit '%s': %w", c.Hash, err) + } + return &git.Commit{ + Hash: []byte(c.Hash.String()), + Reference: ref.String(), + Author: signature(c.Author), + Committer: signature(c.Committer), + Signature: c.PGPSignature, + Encoded: b, + }, nil +} + +func signature(s object.Signature) git.Signature { + return git.Signature{ + Name: s.Name, + Email: s.Email, + When: s.When, + } } func recurseSubmodules(recurse bool) extgogit.SubmoduleRescursivity { diff --git a/pkg/git/gogit/checkout_test.go b/pkg/git/gogit/checkout_test.go index ce7a5e7e..c82e0d3b 100644 --- a/pkg/git/gogit/checkout_test.go +++ b/pkg/git/gogit/checkout_test.go @@ -88,14 +88,14 @@ func TestCheckoutBranch_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "test") defer os.RemoveAll(tmpDir) - _, ref, err := branch.Checkout(context.TODO(), tmpDir, path, nil) + cc, err := branch.Checkout(context.TODO(), tmpDir, path, nil) if tt.expectedErr != "" { g.Expect(err.Error()).To(ContainSubstring(tt.expectedErr)) - g.Expect(ref).To(BeEmpty()) + g.Expect(cc).To(BeNil()) return } g.Expect(err).To(BeNil()) - g.Expect(ref).To(Equal(tt.branch + "/" + tt.expectedCommit)) + g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) }) } } @@ -157,17 +157,17 @@ func TestCheckoutTag_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "test") defer os.RemoveAll(tmpDir) - _, ref, err := tag.Checkout(context.TODO(), tmpDir, path, nil) + cc, err := tag.Checkout(context.TODO(), tmpDir, path, nil) if tt.expectErr != "" { g.Expect(err.Error()).To(ContainSubstring(tt.expectErr)) - g.Expect(ref).To(BeEmpty()) + g.Expect(cc).To(BeNil()) return } - if tt.expectTag != "" { - g.Expect(ref).To(Equal(tt.expectTag + "/" + h.String())) - g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) - g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag)) - } + + g.Expect(err).To(BeNil()) + g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + h.String())) + g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag)) }) } } @@ -196,9 +196,9 @@ func TestCheckoutCommit_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "git2go") defer os.RemoveAll(tmpDir) - _, ref, err := commit.Checkout(context.TODO(), tmpDir, path, nil) + cc, err := commit.Checkout(context.TODO(), tmpDir, path, nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(ref).To(Equal("master/" + c.String())) + g.Expect(cc.String()).To(Equal("master" + "/" + c.String())) g.Expect(filepath.Join(tmpDir, "commit")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "commit"))).To(BeEquivalentTo("init")) @@ -209,10 +209,10 @@ func TestCheckoutCommit_Checkout(t *testing.T) { tmpDir2, _ := os.MkdirTemp("", "git2go") defer os.RemoveAll(tmpDir) - _, ref, err = commit.Checkout(context.TODO(), tmpDir2, path, nil) + cc, err = commit.Checkout(context.TODO(), tmpDir2, path, nil) g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(HavePrefix("git commit '4dc3185c5fc94eb75048376edeb44571cece25f4' not found:")) - g.Expect(ref).To(BeEmpty()) + g.Expect(err.Error()).To(ContainSubstring("object not found")) + g.Expect(cc).To(BeNil()) } func TestCheckoutTagSemVer_Checkout(t *testing.T) { @@ -305,15 +305,15 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "test") defer os.RemoveAll(tmpDir) - _, ref, err := semVer.Checkout(context.TODO(), tmpDir, path, nil) + cc, err := semVer.Checkout(context.TODO(), tmpDir, path, nil) if tt.expectErr != nil { g.Expect(err).To(Equal(tt.expectErr)) - g.Expect(ref).To(BeEmpty()) + g.Expect(cc).To(BeNil()) return } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(ref).To(Equal(tt.expectTag + "/" + refs[tt.expectTag])) + g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + refs[tt.expectTag])) g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.expectTag)) }) @@ -371,8 +371,8 @@ func commitFile(repo *extgogit.Repository, path, content string, time time.Time) return plumbing.Hash{}, err } return wt.Commit("Adding: "+path, &extgogit.CommitOptions{ - Author: signature(time), - Committer: signature(time), + Author: mockSignature(time), + Committer: mockSignature(time), }) } @@ -380,14 +380,14 @@ func tag(repo *extgogit.Repository, commit plumbing.Hash, annotated bool, tag st var opts *extgogit.CreateTagOptions if annotated { opts = &extgogit.CreateTagOptions{ - Tagger: signature(time), + Tagger: mockSignature(time), Message: "Annotated tag for: " + tag, } } return repo.CreateTag(tag, commit, opts) } -func signature(time time.Time) *object.Signature { +func mockSignature(time time.Time) *object.Signature { return &object.Signature{ Name: "Jane Doe", Email: "jane@example.com", diff --git a/pkg/git/gogit/commit.go b/pkg/git/gogit/commit.go deleted file mode 100644 index 8c14fea4..00000000 --- a/pkg/git/gogit/commit.go +++ /dev/null @@ -1,51 +0,0 @@ -/* -Copyright 2020 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package gogit - -import ( - "fmt" - - "github.com/go-git/go-git/v5/plumbing/object" - corev1 "k8s.io/api/core/v1" -) - -type Commit struct { - commit *object.Commit -} - -func (c *Commit) Hash() string { - return c.commit.Hash.String() -} - -// Verify returns an error if the PGP signature can't be verified -func (c *Commit) Verify(secret corev1.Secret) error { - if c.commit.PGPSignature == "" { - return fmt.Errorf("no PGP signature found for commit: %s", c.commit.Hash) - } - - var verified bool - for _, bytes := range secret.Data { - if _, err := c.commit.Verify(string(bytes)); err == nil { - verified = true - break - } - } - if !verified { - return fmt.Errorf("PGP signature '%s' of '%s' can't be verified", c.commit.PGPSignature, c.commit.Author) - } - return nil -} diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index fd44ded9..0dfbbcd2 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -58,7 +58,7 @@ type CheckoutBranch struct { branch string } -func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: &git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, @@ -67,25 +67,27 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g CheckoutBranch: c.branch, }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) } + defer repo.Free() head, err := repo.Head() if err != nil { - return nil, "", fmt.Errorf("git resolve HEAD error: %w", err) + return nil, fmt.Errorf("git resolve HEAD error: %w", err) } defer head.Free() - commit, err := repo.LookupCommit(head.Target()) + cc, err := repo.LookupCommit(head.Target()) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", head.Target(), err) + return nil, fmt.Errorf("could not find commit '%s' in branch '%s': %w", head.Target(), c.branch, err) } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.branch, head.Target().String()), nil + defer cc.Free() + return commit(cc, "refs/heads/"+c.branch), nil } type CheckoutTag struct { tag string } -func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: &git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, @@ -93,13 +95,15 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git. }, }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) } - commit, err := checkoutDetachedDwim(repo, c.tag) + defer repo.Free() + cc, err := checkoutDetachedDwim(repo, c.tag) if err != nil { - return nil, "", err + return nil, err } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.tag, commit.Id().String()), nil + defer cc.Free() + return commit(cc, "refs/tags/"+c.tag), nil } type CheckoutCommit struct { @@ -107,7 +111,7 @@ type CheckoutCommit struct { commit string } -func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: &git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, @@ -115,28 +119,28 @@ func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *g }, }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) } - + defer repo.Free() oid, err := git2go.NewOid(c.commit) if err != nil { - return nil, "", fmt.Errorf("could not create oid for '%s': %w", c.commit, err) + return nil, fmt.Errorf("could not create oid for '%s': %w", c.commit, err) } - commit, err := checkoutDetachedHEAD(repo, oid) + cc, err := checkoutDetachedHEAD(repo, oid) if err != nil { - return nil, "", fmt.Errorf("git checkout error: %w", err) + return nil, fmt.Errorf("git checkout error: %w", err) } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.branch, commit.Id().String()), nil + return commit(cc, ""), nil } type CheckoutSemVer struct { semVer string } -func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (git.Commit, string, error) { +func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { verConstraint, err := semver.NewConstraint(c.semVer) if err != nil { - return nil, "", fmt.Errorf("semver parse range error: %w", err) + return nil, fmt.Errorf("semver parse error: %w", err) } repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ @@ -146,8 +150,9 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g }, }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) + return nil, fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) } + defer repo.Free() tags := make(map[string]string) tagTimestamps := make(map[string]time.Time) @@ -182,7 +187,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g tags[t.Name()] = name return nil }); err != nil { - return nil, "", err + return nil, err } var matchedVersions semver.Collection @@ -197,7 +202,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g matchedVersions = append(matchedVersions, v) } if len(matchedVersions) == 0 { - return nil, "", fmt.Errorf("no match found for semver: %s", c.semVer) + return nil, fmt.Errorf("no match found for semver: %s", c.semVer) } // Sort versions @@ -218,8 +223,12 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g v := matchedVersions[len(matchedVersions)-1] t := v.Original() - commit, err := checkoutDetachedDwim(repo, t) - return &Commit{commit}, fmt.Sprintf("%s/%s", t, commit.Id().String()), nil + cc, err := checkoutDetachedDwim(repo, t) + if err != nil { + return nil, err + } + defer cc.Free() + return commit(cc, "refs/tags/"+t), nil } // checkoutDetachedDwim attempts to perform a detached HEAD checkout by first DWIMing the short name @@ -235,31 +244,31 @@ func checkoutDetachedDwim(repo *git2go.Repository, name string) (*git2go.Commit, return nil, fmt.Errorf("could not get commit for ref '%s': %w", ref.Name(), err) } defer c.Free() - commit, err := c.AsCommit() + cc, err := c.AsCommit() if err != nil { return nil, fmt.Errorf("could not get commit object for ref '%s': %w", ref.Name(), err) } - defer commit.Free() - return checkoutDetachedHEAD(repo, commit.Id()) + defer cc.Free() + return checkoutDetachedHEAD(repo, cc.Id()) } // checkoutDetachedHEAD attempts to perform a detached HEAD checkout for the given commit. func checkoutDetachedHEAD(repo *git2go.Repository, oid *git2go.Oid) (*git2go.Commit, error) { - commit, err := repo.LookupCommit(oid) + cc, err := repo.LookupCommit(oid) if err != nil { return nil, fmt.Errorf("git commit '%s' not found: %w", oid.String(), err) } - if err = repo.SetHeadDetached(commit.Id()); err != nil { - commit.Free() + if err = repo.SetHeadDetached(cc.Id()); err != nil { + cc.Free() return nil, fmt.Errorf("could not detach HEAD at '%s': %w", oid.String(), err) } if err = repo.CheckoutHead(&git2go.CheckoutOptions{ Strategy: git2go.CheckoutForce, }); err != nil { - commit.Free() + cc.Free() return nil, fmt.Errorf("git checkout error: %w", err) } - return commit, nil + return cc, nil } // headCommit returns the current HEAD of the repository, or an error. @@ -269,11 +278,30 @@ func headCommit(repo *git2go.Repository) (*git2go.Commit, error) { return nil, err } defer head.Free() - - commit, err := repo.LookupCommit(head.Target()) + c, err := repo.LookupCommit(head.Target()) if err != nil { return nil, err } - - return commit, nil + return c, nil +} + +func commit(c *git2go.Commit, ref string) *git.Commit { + sig, msg, _ := c.ExtractSignature() + return &git.Commit{ + Hash: []byte(c.Id().String()), + Reference: ref, + Author: signature(c.Author()), + Committer: signature(c.Committer()), + Signature: sig, + Encoded: []byte(msg), + Message: c.Message(), + } +} + +func signature(s *git2go.Signature) git.Signature { + return git.Signature{ + Name: s.Name, + Email: s.Email, + When: s.When, + } } diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 8c1d31c5..8a077a92 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -82,15 +82,15 @@ func TestCheckoutBranch_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "test") defer os.RemoveAll(tmpDir) - _, ref, err := branch.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + cc, err := branch.Checkout(context.TODO(), tmpDir, repo.Path(), nil) if tt.expectedErr != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tt.expectedErr)) - g.Expect(ref).To(BeEmpty()) + g.Expect(cc).To(BeNil()) return } - g.Expect(ref).To(Equal(tt.branch + "/" + tt.expectedCommit)) - g.Expect(err).To(BeNil()) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) }) } } @@ -153,18 +153,18 @@ func TestCheckoutTag_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "test") defer os.RemoveAll(tmpDir) - _, ref, err := tag.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + cc, err := tag.Checkout(context.TODO(), tmpDir, repo.Path(), nil) if tt.expectErr != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tt.expectErr)) - g.Expect(ref).To(BeEmpty()) + g.Expect(cc).To(BeNil()) return } - if tt.expectTag != "" { - g.Expect(ref).To(Equal(tt.expectTag + "/" + commit.Id().String())) - g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) - g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag)) - } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + commit.Id().String())) + g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag)) }) } } @@ -194,9 +194,10 @@ func TestCheckoutCommit_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "git2go") defer os.RemoveAll(tmpDir) - _, ref, err := commit.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + cc, err := commit.Checkout(context.TODO(), tmpDir, repo.Path(), nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(ref).To(Equal("main/" + c.String())) + g.Expect(cc).ToNot(BeNil()) + g.Expect(cc.String()).To(Equal("HEAD/" + c.String())) g.Expect(filepath.Join(tmpDir, "commit")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "commit"))).To(BeEquivalentTo("init")) @@ -206,10 +207,10 @@ func TestCheckoutCommit_Checkout(t *testing.T) { tmpDir2, _ := os.MkdirTemp("", "git2go") defer os.RemoveAll(tmpDir) - _, ref, err = commit.Checkout(context.TODO(), tmpDir2, repo.Path(), nil) + cc, err = commit.Checkout(context.TODO(), tmpDir2, repo.Path(), nil) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(HavePrefix("git checkout error: git commit '4dc3185c5fc94eb75048376edeb44571cece25f4' not found:")) - g.Expect(ref).To(BeEmpty()) + g.Expect(cc).To(BeNil()) } func TestCheckoutTagSemVer_Checkout(t *testing.T) { @@ -313,15 +314,15 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { tmpDir, _ := os.MkdirTemp("", "test") defer os.RemoveAll(tmpDir) - _, ref, err := semVer.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + cc, err := semVer.Checkout(context.TODO(), tmpDir, repo.Path(), nil) if tt.expectErr != nil { g.Expect(err).To(Equal(tt.expectErr)) - g.Expect(ref).To(BeEmpty()) + g.Expect(cc).To(BeNil()) return } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(ref).To(Equal(tt.expectTag + "/" + refs[tt.expectTag])) + g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + refs[tt.expectTag])) g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.expectTag)) }) @@ -397,11 +398,11 @@ func commitFile(repo *git2go.Repository, path, content string, time time.Time) ( } defer tree.Free() - commit, err := repo.CreateCommit("HEAD", signature(time), signature(time), "Committing "+path, tree, parentC...) + c, err := repo.CreateCommit("HEAD", mockSignature(time), mockSignature(time), "Committing "+path, tree, parentC...) if err != nil { return nil, err } - return commit, nil + return c, nil } func tag(repo *git2go.Repository, cId *git2go.Oid, annotated bool, tag string, time time.Time) (*git2go.Oid, error) { @@ -410,12 +411,12 @@ func tag(repo *git2go.Repository, cId *git2go.Oid, annotated bool, tag string, t return nil, err } if annotated { - return repo.Tags.Create(tag, commit, signature(time), fmt.Sprintf("Annotated tag for %s", tag)) + return repo.Tags.Create(tag, commit, mockSignature(time), fmt.Sprintf("Annotated tag for %s", tag)) } return repo.Tags.CreateLightweight(tag, commit, false) } -func signature(time time.Time) *git2go.Signature { +func mockSignature(time time.Time) *git2go.Signature { return &git2go.Signature{ Name: "Jane Doe", Email: "author@example.com", diff --git a/pkg/git/libgit2/commit.go b/pkg/git/libgit2/commit.go deleted file mode 100644 index 1e459f31..00000000 --- a/pkg/git/libgit2/commit.go +++ /dev/null @@ -1,65 +0,0 @@ -/* -Copyright 2020 The Flux authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package libgit2 - -import ( - "bytes" - "fmt" - "strings" - - "golang.org/x/crypto/openpgp" - - git2go "github.com/libgit2/git2go/v31" - corev1 "k8s.io/api/core/v1" -) - -type Commit struct { - commit *git2go.Commit -} - -func (c *Commit) Hash() string { - return c.commit.Id().String() -} - -// Verify returns an error if the PGP signature can't be verified -func (c *Commit) Verify(secret corev1.Secret) error { - signature, signedData, err := c.commit.ExtractSignature() - if err != nil { - return err - } - - var verified bool - for _, b := range secret.Data { - keyRingReader := strings.NewReader(string(b)) - keyring, err := openpgp.ReadArmoredKeyRing(keyRingReader) - if err != nil { - return err - } - - _, err = openpgp.CheckArmoredDetachedSignature(keyring, strings.NewReader(signedData), bytes.NewBufferString(signature)) - if err == nil { - verified = true - break - } - } - - if !verified { - return fmt.Errorf("PGP signature '%s' of '%s' can't be verified", signature, c.commit.Committer().Email) - } - - return nil -}