From d32470ee95b4e421b14bfd4cb34b3ce54023230e Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Thu, 23 Feb 2023 13:08:14 +0530 Subject: [PATCH 1/2] gitrepo: use absolute refs when ref name is provided Use `commit.AbsoluteReference()` to show the full reference when `.spec.ref.name` is provided. For eg: `refs/heads/main@sha1:`. Signed-off-by: Sanskar Jaiswal --- controllers/gitrepository_controller.go | 21 ++++++++++++++------- go.mod | 8 ++++---- go.sum | 20 +++++++++++--------- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index b93c4ee71..f3fadfa49 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -342,7 +342,7 @@ func (r *GitRepositoryReconciler) notify(ctx context.Context, oldObj, newObj *so if git.IsConcreteCommit(commit) { message = fmt.Sprintf("stored artifact for commit '%s'", commit.ShortMessage()) } else { - message = fmt.Sprintf("stored artifact for commit '%s'", commit.String()) + message = fmt.Sprintf("stored artifact for commit '%s'", commitReference(newObj, &commit)) } // Notify on new artifact and failure recovery. @@ -558,7 +558,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch if !gitContentConfigChanged(obj, includes) { ge := serror.NewGeneric( fmt.Errorf("no changes since last reconcilation: observed revision '%s'", - commit.String()), sourcev1.GitOperationSucceedReason, + commitReference(obj, commit)), sourcev1.GitOperationSucceedReason, ) ge.Notification = false ge.Ignore = true @@ -570,7 +570,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch // reconciliation reconcileArtifact() ensures that it's set at the // very end. conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, - "stored artifact for revision '%s'", commit.String()) + "stored artifact for revision '%s'", commitReference(obj, commit)) // TODO: Find out if such condition setting is needed when commit // signature verification is enabled. return sreconcile.ResultEmpty, ge @@ -584,7 +584,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch } *commit = *c } - ctrl.LoggerFrom(ctx).V(logger.DebugLevel).Info("git repository checked out", "url", obj.Spec.URL, "revision", commit.String()) + ctrl.LoggerFrom(ctx).V(logger.DebugLevel).Info("git repository checked out", "url", obj.Spec.URL, "revision", commitReference(obj, commit)) conditions.Delete(obj, sourcev1.FetchFailedCondition) // Verify commit signature @@ -593,8 +593,8 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch } // Mark observations about the revision on the object - if !obj.GetArtifact().HasRevision(commit.String()) { - message := fmt.Sprintf("new upstream revision '%s'", commit.String()) + if !obj.GetArtifact().HasRevision(commitReference(obj, commit)) { + message := fmt.Sprintf("new upstream revision '%s'", commitReference(obj, commit)) if obj.GetArtifact() != nil { conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message) } @@ -622,7 +622,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) { // Create potential new artifact with current available metadata - artifact := r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), commit.String(), fmt.Sprintf("%s.tar.gz", commit.Hash.String())) + artifact := r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), commitReference(obj, commit), fmt.Sprintf("%s.tar.gz", commit.Hash.String())) // Set the ArtifactInStorageCondition if there's no drift. defer func() { @@ -1048,3 +1048,10 @@ func gitRepositoryIncludeEqual(a, b sourcev1.GitRepositoryInclude) bool { } return true } + +func commitReference(obj *sourcev1.GitRepository, commit *git.Commit) string { + if obj.Spec.Reference != nil && obj.Spec.Reference.Name != "" { + return commit.AbsoluteReference() + } + return commit.String() +} diff --git a/go.mod b/go.mod index a1a71f44a..aaae48d81 100644 --- a/go.mod +++ b/go.mod @@ -25,8 +25,8 @@ require ( github.com/fluxcd/go-git/v5 v5.0.0-20221219190809-2e5c9d01cfc4 github.com/fluxcd/pkg/apis/event v0.4.0 github.com/fluxcd/pkg/apis/meta v0.19.0 - github.com/fluxcd/pkg/git v0.10.0 - github.com/fluxcd/pkg/git/gogit v0.7.1 + github.com/fluxcd/pkg/git v0.11.0 + github.com/fluxcd/pkg/git/gogit v0.8.0 github.com/fluxcd/pkg/gittestserver v0.8.1 github.com/fluxcd/pkg/helmtestserver v0.11.1 github.com/fluxcd/pkg/lockedfile v0.1.0 @@ -45,7 +45,7 @@ require ( github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20230217043738-4a0e0af4bf95 github.com/google/uuid v1.3.0 github.com/minio/minio-go/v7 v7.0.49 - github.com/onsi/gomega v1.27.1 + github.com/onsi/gomega v1.27.2 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/go-digest/blake3 v0.0.0-20220411205349-bde1400a84be github.com/ory/dockertest/v3 v3.9.1 @@ -96,7 +96,7 @@ require ( github.com/Masterminds/squirrel v1.5.3 // indirect github.com/Microsoft/go-winio v0.6.0 // indirect github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 // indirect - github.com/ProtonMail/go-crypto v0.0.0-20230214155104-81033d7f4442 // indirect + github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 // indirect github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d // indirect github.com/ThalesIgnite/crypto11 v1.2.5 // indirect github.com/acomagu/bufpipe v1.0.3 // indirect diff --git a/go.sum b/go.sum index 23678f445..cc7a4ab7e 100644 --- a/go.sum +++ b/go.sum @@ -172,8 +172,8 @@ github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEV github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5/go.mod h1:lmUJ/7eu/Q8D7ML55dXQrVaamCz2vxCfdQBasLZfHKk= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/ProtonMail/go-crypto v0.0.0-20221026131551-cf6655e29de4/go.mod h1:UBYPn8k0D56RtnR8RFQMjmh4KrZzWJ5o7Z9SYjossQ8= -github.com/ProtonMail/go-crypto v0.0.0-20230214155104-81033d7f4442 h1:OUJ54Fkd+AQXYmr9eOUxZfWNzpK3/e/KD40qa2rKHS4= -github.com/ProtonMail/go-crypto v0.0.0-20230214155104-81033d7f4442/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g= +github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 h1:wPbRQzjjwFc0ih8puEVAOFGELsn1zoIIYdxvML7mDxA= +github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g= github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d h1:UrqY+r/OJnIp5u0s1SbQ8dVfLCZJsnvazdBP5hS4iRs= @@ -537,10 +537,10 @@ github.com/fluxcd/pkg/apis/event v0.4.0 h1:UPCC269KjgKgkmtiCiBq/DNue/EpXy8Tq1zFx github.com/fluxcd/pkg/apis/event v0.4.0/go.mod h1:xYOOlf+9gCBSYcs93N2XAbJvSVwuVBDBUzqhR+cAo7M= github.com/fluxcd/pkg/apis/meta v0.19.0 h1:CX75e/eaRWZDTzNdMSWomY1InlssLKcS8GQDSg/aopI= github.com/fluxcd/pkg/apis/meta v0.19.0/go.mod h1:7b6prDPsViyAzoY7eRfSPS0/MbXpGGsOMvRq2QrTKa4= -github.com/fluxcd/pkg/git v0.10.0 h1:tO04FyUV3kmyJOpAKjMFZWClqr1JNGxS8RxI7znq6is= -github.com/fluxcd/pkg/git v0.10.0/go.mod h1:zn3pJ4mRItezf6J0okHZbZ+3YNAGsjnhrS+Kbo+56Jw= -github.com/fluxcd/pkg/git/gogit v0.7.1 h1:9QQtx8olL9CE0RaDUIPGBvkuh1IYZ5i5iFLQbcSvcyU= -github.com/fluxcd/pkg/git/gogit v0.7.1/go.mod h1:QrYVKE25QpLTvM83Toec6KtVJ3WCnvvGTybL+2Zabxs= +github.com/fluxcd/pkg/git v0.11.0 h1:GvB+3QOB8xbF5WNjVrkskseOnsZBuqSOzW3VxfsHuX4= +github.com/fluxcd/pkg/git v0.11.0/go.mod h1:VHRVlrZMHNoWBlaSAWxlGH6Vwlb9VRazUhPUykviHwY= +github.com/fluxcd/pkg/git/gogit v0.8.0 h1:rSOiTnNOLCyJbVYu2P0uqXtYEg4oRwyQB1RPNG9/wts= +github.com/fluxcd/pkg/git/gogit v0.8.0/go.mod h1:wN5GrntOSQDHNSjse/qf387x+dcQjmabqBHRgA0Qfr4= github.com/fluxcd/pkg/gittestserver v0.8.1 h1:FMqnZBuS/11+9NhtLv9UAg+wm/v0Nf+hHeUOi2wJR3Q= github.com/fluxcd/pkg/gittestserver v0.8.1/go.mod h1:Ar0epRFZ7ZKZZldSjytWkkMiCWfxgpZ4jZZvJEKhTE0= github.com/fluxcd/pkg/helmtestserver v0.11.1 h1:seotZ19JtzPfuzru5zHCEX/0Ff96PVPI41OLaHh4rC0= @@ -681,6 +681,7 @@ github.com/go-sql-driver/mysql v1.6.0 h1:BCTh4TKNUYmOmMUcQ3IipzF5prigylS7XXjEkfC github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-stack/stack v1.8.1/go.mod h1:dcoOX6HbPZSZptuspn9bctJ+N/CnF5gGygcUP3XYfe4= +github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 h1:p104kn46Q8WdvHunIJ9dAyjPVtrBPhSr3KT2yUst43I= github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= github.com/go-test/deep v1.1.0 h1:WOcxcdHcvdgThNXjw0t76K42FXTU7HpNQWHpA2HHNlg= github.com/gobuffalo/attrs v0.0.0-20190224210810-a9411de4debd/go.mod h1:4duuawTqi2wkkpB4ePgWMaai6/Kc6WEz83bhFwpHzj0= @@ -852,6 +853,7 @@ github.com/google/pprof v0.0.0-20210226084205-cbba55b83ad5/go.mod h1:kpwsk12EmLe github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/pprof v0.0.0-20210601050228-01bbb1931b22/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/pprof v0.0.0-20210609004039-a478d1d731e9/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= +github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 h1:K6RDEckDVWvDI9JAJYCmNdQXq6neHJOYx3V6jnqNEec= github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/rpmpack v0.0.0-20191226140753-aa36bfddb3a0/go.mod h1:RaTPr0KUf2K7fnZYLNDrr8rxAamWs3iNywJLtQ2AzBg= @@ -1244,15 +1246,15 @@ github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vv github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c= -github.com/onsi/ginkgo/v2 v2.8.1 h1:xFTEVwOFa1D/Ty24Ws1npBWkDYEV9BqZrsDxVrVkrrU= +github.com/onsi/ginkgo/v2 v2.8.4 h1:gf5mIQ8cLFieruNLAdgijHF1PYfLphKm2dxxcUtcqK0= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= -github.com/onsi/gomega v1.27.1 h1:rfztXRbg6nv/5f+Raen9RcGoSecHIFgBBLQK3Wdj754= -github.com/onsi/gomega v1.27.1/go.mod h1:aHX5xOykVYzWOV4WqQy0sy8BQptgukenXpCXfadcIAw= +github.com/onsi/gomega v1.27.2 h1:SKU0CXeKE/WVgIV1T61kSa3+IRE8Ekrv9rdXDwwTqnY= +github.com/onsi/gomega v1.27.2/go.mod h1:5mR3phAHpkAVIDkHEUBY6HGVsU+cpcEscrGPB4oPlZI= github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk= github.com/opencontainers/go-digest v1.0.1-0.20220411205349-bde1400a84be h1:f2PlhC9pm5sqpBZFvnAoKj+KzXRzbjFMA+TqXfJdgho= github.com/opencontainers/go-digest v1.0.1-0.20220411205349-bde1400a84be/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= From 4cbacd030873b9a2b69f80421a6cbe26b8311c09 Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Fri, 24 Feb 2023 07:17:16 +0000 Subject: [PATCH 2/2] gitrepo: add tests for reference name checkout strategy Signed-off-by: Sanskar Jaiswal --- controllers/gitrepository_controller_test.go | 39 +++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 64da73cca..de02eedad 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -600,6 +600,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) branches := []string{"staging"} tags := []string{"non-semver-tag", "v0.1.0", "0.2.0", "v0.2.1", "v1.0.0-alpha", "v1.1.0", "v2.0.0"} + refs := []string{"refs/pull/420/head"} tests := []struct { name string @@ -645,6 +646,24 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) wantRevision: "staging@sha1:", wantReconciling: true, }, + { + name: "Ref Name pointing to a branch", + reference: &sourcev1.GitRepositoryRef{ + Name: "refs/heads/staging", + }, + want: sreconcile.ResultSuccess, + wantRevision: "refs/heads/staging@sha1:", + wantReconciling: true, + }, + { + name: "Ref Name pointing to a PR", + reference: &sourcev1.GitRepositoryRef{ + Name: "refs/pull/420/head", + }, + want: sreconcile.ResultSuccess, + wantRevision: "refs/pull/420/head@sha1:", + wantReconciling: true, + }, { name: "SemVer", reference: &sourcev1.GitRepositoryRef{ @@ -801,6 +820,10 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) g.Expect(remoteTagForHead(localRepo, headRef, tag)).To(Succeed()) } + for _, ref := range refs { + g.Expect(remoteRefForHead(localRepo, headRef, ref)).To(Succeed()) + } + r := &GitRepositoryReconciler{ Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(), EventRecorder: record.NewFakeRecorder(32), @@ -854,7 +877,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) g.Expect(got).To(Equal(tt.want)) if tt.wantRevision != "" && !tt.wantErr { revision := strings.ReplaceAll(tt.wantRevision, "", headRef.Hash().String()) - g.Expect(commit.String()).To(Equal(revision)) + g.Expect(commitReference(obj, &commit)).To(Equal(revision)) g.Expect(conditions.IsTrue(obj, sourcev1.ArtifactOutdatedCondition)).To(Equal(tt.wantArtifactOutdated)) g.Expect(conditions.IsTrue(obj, meta.ReconcilingCondition)).To(Equal(tt.wantReconciling)) } @@ -1888,6 +1911,20 @@ func remoteTagForHead(repo *gogit.Repository, head *plumbing.Reference, tag stri }) } +func remoteRefForHead(repo *gogit.Repository, head *plumbing.Reference, reference string) error { + if err := repo.Storer.SetReference(plumbing.NewHashReference(plumbing.ReferenceName(reference), head.Hash())); err != nil { + return err + } + if err := repo.Push(&gogit.PushOptions{ + RefSpecs: []config.RefSpec{ + config.RefSpec("+" + reference + ":" + reference), + }, + }); err != nil { + return err + } + return nil +} + func TestGitRepositoryReconciler_statusConditions(t *testing.T) { tests := []struct { name string