From 4ce31319eed4e09b81d1cf6e8ace06ff5d57425b Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Thu, 9 Jun 2022 10:06:24 +0530 Subject: [PATCH 1/2] libgit2: refactor tests to use managed and unmanaged transport cleanly Refactors libgit2 checkout tests to test managed and unmanaged transport by making sure the tests requiring unmanaged transport are run before, any tests that require managed transport (since disabling managed transport isn't possible). This is done via arranging the tests carefully in alphabetically sorted names, i.e. the tests with unmanaged transport go in `checkout_test.go`, which forces golang to run the tests in that file before any other tests. Signed-off-by: Sanskar Jaiswal --- controllers/suite_test.go | 3 + pkg/git/libgit2/checkout_test.go | 282 +++++++++++++----- pkg/git/libgit2/managed_checkout_test.go | 46 +++ pkg/git/libgit2/managed_test.go | 214 ++----------- pkg/git/strategy/proxy/strategy_proxy_test.go | 5 + 5 files changed, 288 insertions(+), 262 deletions(-) create mode 100644 pkg/git/libgit2/managed_checkout_test.go diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 62674da8e..a8ccb8039 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -37,6 +37,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "github.com/fluxcd/pkg/runtime/controller" + feathelper "github.com/fluxcd/pkg/runtime/features" "github.com/fluxcd/pkg/runtime/testenv" "github.com/fluxcd/pkg/testserver" "github.com/go-logr/logr" @@ -206,6 +207,8 @@ func TestMain(m *testing.M) { panic(fmt.Sprintf("Failed to create a test registry server: %v", err)) } + fg := feathelper.FeatureGates{} + fg.SupportedFeatures(features.FeatureGates()) managed.InitManagedTransport(logr.Discard()) if err := (&GitRepositoryReconciler{ diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 0ff5ee888..d04b6e416 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -25,40 +25,49 @@ import ( "testing" "time" - "github.com/fluxcd/source-controller/pkg/git" + "github.com/fluxcd/pkg/gittestserver" git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" + + "github.com/fluxcd/source-controller/pkg/git" ) -func TestCheckoutBranch_checkoutUnmanaged(t *testing.T) { - repo, err := initBareRepo(t) +func TestCheckoutBranch_unmanaged(t *testing.T) { + checkoutBranch(t, false) +} + +// checkoutBranch is a test helper function which runs the tests for checking out +// via CheckoutBranch. +func checkoutBranch(t *testing.T, managed bool) { + // we use a HTTP Git server instead of a bare repo (for all tests in this + // package), because our managed transports don't support the file protocol, + // so we wouldn't actually be using our custom transports, if we used a bare + // repo. + server, err := gittestserver.NewTempGitServer() if err != nil { t.Fatal(err) } - defer repo.Free() + defer os.RemoveAll(server.Root()) - cfg, err := git2go.OpenDefault() + err = server.StartHTTP() if err != nil { t.Fatal(err) } + defer server.StopHTTP() - // ignores the error here because it can be defaulted - // https://github.blog/2020-07-27-highlights-from-git-2-28/#introducing-init-defaultbranch - defaultBranch := "master" - iter, err := cfg.NewIterator() + repoPath := "test.git" + err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) if err != nil { t.Fatal(err) } - for { - val, e := iter.Next() - if e != nil { - break - } - if val.Name == "init.defaultbranch" { - defaultBranch = val.Value - break - } + + repo, err := git2go.OpenRepository(filepath.Join(server.Root(), repoPath)) + if err != nil { + t.Fatal(err) } + defer repo.Free() + + defaultBranch := "master" firstCommit, err := commitFile(repo, "branch", "init", time.Now()) if err != nil { @@ -75,31 +84,52 @@ func TestCheckoutBranch_checkoutUnmanaged(t *testing.T) { if err != nil { t.Fatal(err) } + repoURL := server.HTTPAddress() + "/" + repoPath tests := []struct { - name string - branch string - filesCreated map[string]string - lastRevision string - expectedCommit string - expectedErr string + name string + branch string + filesCreated map[string]string + lastRevision string + expectedCommit string + expectedConcreteCommit bool + expectedErr string }{ { - name: "Default branch", - branch: defaultBranch, - filesCreated: map[string]string{"branch": "second"}, - expectedCommit: secondCommit.String(), + name: "Default branch", + branch: defaultBranch, + filesCreated: map[string]string{"branch": "second"}, + expectedCommit: secondCommit.String(), + expectedConcreteCommit: true, }, { - name: "Other branch", - branch: "test", - filesCreated: map[string]string{"branch": "init"}, - expectedCommit: firstCommit.String(), + name: "Other branch", + branch: "test", + filesCreated: map[string]string{"branch": "init"}, + expectedCommit: firstCommit.String(), + expectedConcreteCommit: true, }, { - name: "Non existing branch", - branch: "invalid", - expectedErr: "reference 'refs/remotes/origin/invalid' not found", + name: "Non existing branch", + branch: "invalid", + expectedErr: "reference 'refs/remotes/origin/invalid' not found", + expectedConcreteCommit: true, + }, + { + name: "skip clone - lastRevision hasn't changed", + branch: defaultBranch, + filesCreated: map[string]string{"branch": "second"}, + lastRevision: fmt.Sprintf("%s/%s", defaultBranch, secondCommit.String()), + expectedCommit: secondCommit.String(), + expectedConcreteCommit: false, + }, + { + name: "lastRevision is different", + branch: defaultBranch, + filesCreated: map[string]string{"branch": "second"}, + lastRevision: fmt.Sprintf("%s/%s", defaultBranch, firstCommit.String()), + expectedCommit: secondCommit.String(), + expectedConcreteCommit: true, }, } @@ -111,9 +141,13 @@ func TestCheckoutBranch_checkoutUnmanaged(t *testing.T) { Branch: tt.branch, LastRevision: tt.lastRevision, } + tmpDir := t.TempDir() + authOpts := git.AuthOptions{ + TransportOptionsURL: getTransportOptionsURL(git.HTTP), + } - cc, err := branch.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + cc, err := branch.Checkout(context.TODO(), tmpDir, repoURL, &authOpts) if tt.expectedErr != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tt.expectedErr)) @@ -122,31 +156,51 @@ func TestCheckoutBranch_checkoutUnmanaged(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) + if managed { + g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit)) + } + + if tt.expectedConcreteCommit { + for k, v := range tt.filesCreated { + g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v)) + } + } }) } } -func TestCheckoutTag_checkoutUnmanaged(t *testing.T) { +func TestCheckoutTag_unmanaged(t *testing.T) { + checkoutTag(t, false) +} + +// checkoutTag is a test helper function which runs the tests for checking out +// via CheckoutTag. +func checkoutTag(t *testing.T, managed bool) { type testTag struct { name string annotated bool } tests := []struct { - name string - tagsInRepo []testTag - checkoutTag string - expectErr string + name string + tagsInRepo []testTag + checkoutTag string + lastRevTag string + expectErr string + expectConcreteCommit bool }{ { - name: "Tag", - tagsInRepo: []testTag{{"tag-1", false}}, - checkoutTag: "tag-1", + name: "Tag", + tagsInRepo: []testTag{{"tag-1", false}}, + checkoutTag: "tag-1", + expectConcreteCommit: true, }, { - name: "Annotated", - tagsInRepo: []testTag{{"annotated", true}}, - checkoutTag: "annotated", + name: "Annotated", + tagsInRepo: []testTag{{"annotated", true}}, + checkoutTag: "annotated", + expectConcreteCommit: true, }, { name: "Non existing tag", @@ -154,29 +208,46 @@ func TestCheckoutTag_checkoutUnmanaged(t *testing.T) { expectErr: "unable to find 'invalid': no reference found for shorthand 'invalid'", }, { - name: "Skip clone - last revision unchanged", - tagsInRepo: []testTag{{"tag-1", false}}, - checkoutTag: "tag-1", + name: "Skip clone - last revision unchanged", + tagsInRepo: []testTag{{"tag-1", false}}, + checkoutTag: "tag-1", + lastRevTag: "tag-1", + expectConcreteCommit: false, }, { - name: "Last revision changed", - tagsInRepo: []testTag{{"tag-1", false}, {"tag-2", false}}, - checkoutTag: "tag-2", + name: "Last revision changed", + tagsInRepo: []testTag{{"tag-1", false}, {"tag-2", false}}, + checkoutTag: "tag-2", + lastRevTag: "tag-1", + expectConcreteCommit: true, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - repo, err := initBareRepo(t) - if err != nil { - t.Fatal(err) - } + server, err := gittestserver.NewTempGitServer() + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(server.Root()) + + err = server.StartHTTP() + g.Expect(err).ToNot(HaveOccurred()) + defer server.StopHTTP() + + repoPath := "test.git" + err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) + g.Expect(err).ToNot(HaveOccurred()) + + repo, err := git2go.OpenRepository(filepath.Join(server.Root(), repoPath)) + g.Expect(err).ToNot(HaveOccurred()) defer repo.Free() // Collect tags and their associated commit for later reference. tagCommits := map[string]*git2go.Commit{} + repoURL := server.HTTPAddress() + "/" + repoPath + // Populate the repo with commits and tags. if tt.tagsInRepo != nil { for _, tr := range tt.tagsInRepo { @@ -199,9 +270,18 @@ func TestCheckoutTag_checkoutUnmanaged(t *testing.T) { checkoutTag := CheckoutTag{ Tag: tt.checkoutTag, } + // If last revision is provided, configure it. + if tt.lastRevTag != "" { + lc := tagCommits[tt.lastRevTag] + checkoutTag.LastRevision = fmt.Sprintf("%s/%s", tt.lastRevTag, lc.Id().String()) + } + tmpDir := t.TempDir() - cc, err := checkoutTag.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + authOpts := git.AuthOptions{ + TransportOptionsURL: getTransportOptionsURL(git.HTTP), + } + cc, err := checkoutTag.Checkout(context.TODO(), tmpDir, repoURL, &authOpts) if tt.expectErr != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tt.expectErr)) @@ -213,17 +293,48 @@ func TestCheckoutTag_checkoutUnmanaged(t *testing.T) { targetTagCommit := tagCommits[tt.checkoutTag] g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc.String()).To(Equal(tt.checkoutTag + "/" + targetTagCommit.Id().String())) + if managed { + g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit)) - g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) - g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.checkoutTag)) + } + + // Check file content only when there's an actual checkout. + if tt.lastRevTag != tt.checkoutTag { + g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.checkoutTag)) + } }) } } -func TestCheckoutCommit_Checkout(t *testing.T) { +func TestCheckoutCommit_unmanaged(t *testing.T) { + checkoutCommit(t, false) +} + +// checkoutCommit is a test helper function which runs the tests for checking out +// via CheckoutCommit. +func checkoutCommit(t *testing.T, managed bool) { g := NewWithT(t) - repo, err := initBareRepo(t) + server, err := gittestserver.NewTempGitServer() + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(server.Root()) + + err = server.StartHTTP() + if err != nil { + t.Fatal(err) + } + defer server.StopHTTP() + + repoPath := "test.git" + err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) + if err != nil { + t.Fatal(err) + } + + repo, err := git2go.OpenRepository(filepath.Join(server.Root(), repoPath)) if err != nil { t.Fatal(err) } @@ -236,13 +347,17 @@ func TestCheckoutCommit_Checkout(t *testing.T) { if _, err = commitFile(repo, "commit", "second", time.Now()); err != nil { t.Fatal(err) } + tmpDir := t.TempDir() + authOpts := git.AuthOptions{ + TransportOptionsURL: getTransportOptionsURL(git.HTTP), + } + repoURL := server.HTTPAddress() + "/" + repoPath commit := CheckoutCommit{ Commit: c.String(), } - tmpDir := t.TempDir() - cc, err := commit.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + cc, err := commit.Checkout(context.TODO(), tmpDir, repoURL, &authOpts) g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc).ToNot(BeNil()) g.Expect(cc.String()).To(Equal("HEAD/" + c.String())) @@ -254,13 +369,19 @@ func TestCheckoutCommit_Checkout(t *testing.T) { } tmpDir2 := t.TempDir() - cc, err = commit.Checkout(context.TODO(), tmpDir2, repo.Path(), nil) + cc, err = commit.Checkout(context.TODO(), tmpDir2, repoURL, &authOpts) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(HavePrefix("git checkout error: git commit '4dc3185c5fc94eb75048376edeb44571cece25f4' not found:")) g.Expect(cc).To(BeNil()) } -func TestCheckoutTagSemVer_Checkout(t *testing.T) { +func TestCheckoutTagSemVer_unmanaged(t *testing.T) { + checkoutSemVer(t, false) +} + +// checkoutSemVer is a test helper function which runs the tests for checking out +// via CheckoutSemVer. +func checkoutSemVer(t *testing.T, managed bool) { g := NewWithT(t) now := time.Now() @@ -322,11 +443,30 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { }, } - repo, err := initBareRepo(t) + server, err := gittestserver.NewTempGitServer() + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(server.Root()) + + err = server.StartHTTP() + if err != nil { + t.Fatal(err) + } + defer server.StopHTTP() + + repoPath := "test.git" + err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) + if err != nil { + t.Fatal(err) + } + + repo, err := git2go.OpenRepository(filepath.Join(server.Root(), repoPath)) if err != nil { t.Fatal(err) } defer repo.Free() + repoURL := server.HTTPAddress() + "/" + repoPath refs := make(map[string]string, len(tags)) for _, tt := range tags { @@ -357,9 +497,13 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { semVer := CheckoutSemVer{ SemVer: tt.constraint, } + tmpDir := t.TempDir() + authOpts := git.AuthOptions{ + TransportOptionsURL: getTransportOptionsURL(git.HTTP), + } - cc, err := semVer.Checkout(context.TODO(), tmpDir, repo.Path(), nil) + cc, err := semVer.Checkout(context.TODO(), tmpDir, repoURL, &authOpts) if tt.expectErr != nil { g.Expect(err).To(Equal(tt.expectErr)) g.Expect(cc).To(BeNil()) @@ -376,7 +520,7 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { func initBareRepo(t *testing.T) (*git2go.Repository, error) { tmpDir := t.TempDir() - repo, err := git2go.InitRepository(tmpDir, false) + repo, err := git2go.InitRepository(tmpDir, true) if err != nil { return nil, err } diff --git a/pkg/git/libgit2/managed_checkout_test.go b/pkg/git/libgit2/managed_checkout_test.go new file mode 100644 index 000000000..4c3bb42d0 --- /dev/null +++ b/pkg/git/libgit2/managed_checkout_test.go @@ -0,0 +1,46 @@ +/* +Copyright 2022 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. +*/ + +// This file is named `managed_checkout_test.go` on purpose to make sure that +// tests needing to use unmanaged transports run before the tests that use managed +// transports do, since the the former are present in `checkout_test.go`. `checkout_test.go` +// comes first in this package (alphabetically speaking), which makes golang run the tests +// in that file first. +package libgit2 + +import ( + "testing" +) + +func TestCheckoutBranch_CheckoutManaged(t *testing.T) { + enableManagedTransport() + checkoutBranch(t, true) +} + +func TestCheckoutTag_CheckoutManaged(t *testing.T) { + enableManagedTransport() + checkoutTag(t, true) +} + +func TestCheckoutCommit_CheckoutManaged(t *testing.T) { + enableManagedTransport() + checkoutCommit(t, true) +} + +func TestCheckoutTagSemVer_CheckoutManaged(t *testing.T) { + enableManagedTransport() + checkoutSemVer(t, true) +} diff --git a/pkg/git/libgit2/managed_test.go b/pkg/git/libgit2/managed_test.go index cf5aabc56..deda75618 100644 --- a/pkg/git/libgit2/managed_test.go +++ b/pkg/git/libgit2/managed_test.go @@ -30,22 +30,23 @@ import ( "github.com/fluxcd/gitkit" "github.com/fluxcd/pkg/gittestserver" "github.com/fluxcd/pkg/ssh" - "github.com/fluxcd/source-controller/pkg/git" - "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" - "github.com/go-logr/logr" - . "github.com/onsi/gomega" - git2go "github.com/libgit2/git2go/v33" + feathelper "github.com/fluxcd/pkg/runtime/features" + . "github.com/onsi/gomega" cryptossh "golang.org/x/crypto/ssh" + + "github.com/fluxcd/source-controller/internal/features" + "github.com/fluxcd/source-controller/pkg/git" + "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" ) const testRepositoryPath = "../testdata/git/repo" -// Test_ManagedSSH_KeyTypes assures support for the different +// Test_managedSSH_KeyTypes assures support for the different // types of keys for SSH Authentication supported by Flux. -func Test_ManagedSSH_KeyTypes(t *testing.T) { - managed.InitManagedTransport(logr.Discard()) +func Test_managedSSH_KeyTypes(t *testing.T) { + enableManagedTransport() tests := []struct { name string @@ -171,10 +172,10 @@ func Test_ManagedSSH_KeyTypes(t *testing.T) { } } -// Test_ManagedSSH_KeyExchangeAlgos assures support for the different +// Test_managedSSH_KeyExchangeAlgos assures support for the different // types of SSH key exchange algorithms supported by Flux. -func Test_ManagedSSH_KeyExchangeAlgos(t *testing.T) { - managed.InitManagedTransport(logr.Discard()) +func Test_managedSSH_KeyExchangeAlgos(t *testing.T) { + enableManagedTransport() tests := []struct { name string @@ -294,10 +295,10 @@ func Test_ManagedSSH_KeyExchangeAlgos(t *testing.T) { } } -// Test_ManagedSSH_HostKeyAlgos assures support for the different +// Test_managedSSH_HostKeyAlgos assures support for the different // types of SSH Host Key algorithms supported by Flux. -func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { - managed.InitManagedTransport(logr.Discard()) +func Test_managedSSH_HostKeyAlgos(t *testing.T) { + enableManagedTransport() tests := []struct { name string @@ -458,185 +459,6 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { } } -func Test_ManagedHTTPCheckout(t *testing.T) { - managed.InitManagedTransport(logr.Discard()) - g := NewWithT(t) - - timeout := 5 * time.Second - server, err := gittestserver.NewTempGitServer() - g.Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(server.Root()) - - user := "test-user" - pwd := "test-pswd" - server.Auth(user, pwd) - - err = server.StartHTTP() - g.Expect(err).ToNot(HaveOccurred()) - defer server.StopHTTP() - - repoPath := "test.git" - err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) - g.Expect(err).ToNot(HaveOccurred()) - - authOpts := &git.AuthOptions{ - Username: "test-user", - Password: "test-pswd", - } - authOpts.TransportOptionsURL = getTransportOptionsURL(git.HTTP) - - // Prepare for checkout. - branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch} - tmpDir := t.TempDir() - - ctx, cancel := context.WithTimeout(context.TODO(), timeout) - defer cancel() - - repoURL := server.HTTPAddress() + "/" + repoPath - // Checkout the repo. - _, err = branchCheckoutStrat.Checkout(ctx, tmpDir, repoURL, authOpts) - g.Expect(err).Error().ShouldNot(HaveOccurred()) -} - -func TestManagedCheckoutBranch_Checkout(t *testing.T) { - managed.InitManagedTransport(logr.Discard()) - g := NewWithT(t) - - timeout := 5 * time.Second - server, err := gittestserver.NewTempGitServer() - g.Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(server.Root()) - - err = server.StartHTTP() - g.Expect(err).ToNot(HaveOccurred()) - defer server.StopHTTP() - - repoPath := "test.git" - err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) - g.Expect(err).ToNot(HaveOccurred()) - - repo, err := git2go.OpenRepository(filepath.Join(server.Root(), repoPath)) - g.Expect(err).ToNot(HaveOccurred()) - defer repo.Free() - - branchRef, err := repo.References.Lookup(fmt.Sprintf("refs/heads/%s", git.DefaultBranch)) - g.Expect(err).ToNot(HaveOccurred()) - defer branchRef.Free() - - commit, err := repo.LookupCommit(branchRef.Target()) - g.Expect(err).ToNot(HaveOccurred()) - defer commit.Free() - - authOpts := &git.AuthOptions{ - TransportOptionsURL: getTransportOptionsURL(git.HTTP), - } - - tmpDir := t.TempDir() - - ctx, cancel := context.WithTimeout(context.TODO(), timeout) - defer cancel() - - repoURL := server.HTTPAddress() + "/" + repoPath - branch := CheckoutBranch{ - Branch: git.DefaultBranch, - // Set last revision to HEAD commit, to force a no-op clone. - LastRevision: fmt.Sprintf("%s/%s", git.DefaultBranch, commit.Id().String()), - } - - cc, err := branch.Checkout(ctx, tmpDir, repoURL, authOpts) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(git.DefaultBranch + "/" + commit.Id().String())) - g.Expect(git.IsConcreteCommit(*cc)).To(Equal(false)) - - // Set last revision to a fake commit to force a full clone. - branch.LastRevision = fmt.Sprintf("%s/non-existent-commit", git.DefaultBranch) - cc, err = branch.Checkout(ctx, tmpDir, repoURL, authOpts) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(git.DefaultBranch + "/" + commit.Id().String())) - g.Expect(git.IsConcreteCommit(*cc)).To(Equal(true)) - - // Create a new branch and push it. - err = createBranch(repo, "test", nil) - g.Expect(err).ToNot(HaveOccurred()) - transportOptsURL := getTransportOptionsURL(git.HTTP) - managed.AddTransportOptions(transportOptsURL, managed.TransportOptions{ - TargetURL: repoURL, - }) - defer managed.RemoveTransportOptions(transportOptsURL) - origin, err := repo.Remotes.Create("origin", transportOptsURL) - defer origin.Free() - g.Expect(err).ToNot(HaveOccurred()) - err = origin.Push([]string{"refs/heads/test:refs/heads/test"}, &git2go.PushOptions{}) - g.Expect(err).ToNot(HaveOccurred()) - - branch.Branch = "test" - tmpDir2 := t.TempDir() - cc, err = branch.Checkout(ctx, tmpDir2, repoURL, authOpts) - g.Expect(err).ToNot(HaveOccurred()) - - // Check if the repo HEAD points to the branch. - repo, err = git2go.OpenRepository(tmpDir2) - g.Expect(err).ToNot(HaveOccurred()) - head, err := repo.Head() - defer head.Free() - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(head.Branch().Name()).To(Equal("test")) -} - -func TestManagedCheckoutTag_Checkout(t *testing.T) { - managed.InitManagedTransport(logr.Discard()) - g := NewWithT(t) - - timeout := 5 * time.Second - server, err := gittestserver.NewTempGitServer() - g.Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(server.Root()) - - err = server.StartHTTP() - g.Expect(err).ToNot(HaveOccurred()) - defer server.StopHTTP() - - repoPath := "test.git" - err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) - g.Expect(err).ToNot(HaveOccurred()) - - repo, err := git2go.OpenRepository(filepath.Join(server.Root(), repoPath)) - g.Expect(err).ToNot(HaveOccurred()) - defer repo.Free() - - branchRef, err := repo.References.Lookup(fmt.Sprintf("refs/heads/%s", git.DefaultBranch)) - g.Expect(err).ToNot(HaveOccurred()) - defer branchRef.Free() - - commit, err := repo.LookupCommit(branchRef.Target()) - g.Expect(err).ToNot(HaveOccurred()) - defer commit.Free() - _, err = tag(repo, commit.Id(), false, "tag-1", time.Now()) - - checkoutTag := CheckoutTag{ - Tag: "tag-1", - } - authOpts := &git.AuthOptions{ - TransportOptionsURL: getTransportOptionsURL(git.HTTP), - } - repoURL := server.HTTPAddress() + "/" + repoPath - tmpDir := t.TempDir() - - ctx, cancel := context.WithTimeout(context.TODO(), timeout) - defer cancel() - - cc, err := checkoutTag.Checkout(ctx, tmpDir, repoURL, authOpts) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal("tag-1" + "/" + commit.Id().String())) - g.Expect(git.IsConcreteCommit(*cc)).To(Equal(true)) - - checkoutTag.LastRevision = "tag-1" + "/" + commit.Id().String() - cc, err = checkoutTag.Checkout(ctx, tmpDir, repoURL, authOpts) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal("tag-1" + "/" + commit.Id().String())) - g.Expect(git.IsConcreteCommit(*cc)).To(Equal(false)) -} - func getTransportOptionsURL(transport git.TransportType) string { letterRunes := []rune("abcdefghijklmnopqrstuvwxyz1234567890") b := make([]rune, 10) @@ -645,3 +467,9 @@ func getTransportOptionsURL(transport git.TransportType) string { } return string(transport) + "://" + string(b) } + +func enableManagedTransport() { + fg := feathelper.FeatureGates{} + fg.SupportedFeatures(features.FeatureGates()) + managed.InitManagedTransport(logr.Discard()) +} diff --git a/pkg/git/strategy/proxy/strategy_proxy_test.go b/pkg/git/strategy/proxy/strategy_proxy_test.go index 5f9573793..2e83c6602 100644 --- a/pkg/git/strategy/proxy/strategy_proxy_test.go +++ b/pkg/git/strategy/proxy/strategy_proxy_test.go @@ -29,9 +29,11 @@ import ( "github.com/elazarl/goproxy" "github.com/fluxcd/pkg/gittestserver" + feathelper "github.com/fluxcd/pkg/runtime/features" "github.com/go-logr/logr" . "github.com/onsi/gomega" + "github.com/fluxcd/source-controller/internal/features" "github.com/fluxcd/source-controller/pkg/git" "github.com/fluxcd/source-controller/pkg/git/gogit" "github.com/fluxcd/source-controller/pkg/git/libgit2" @@ -45,6 +47,9 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { // for libgit2 we are only testing for managed transport, // as unmanaged is sunsetting. // Unmanaged transport does not support HTTP_PROXY. + fg := feathelper.FeatureGates{} + fg.SupportedFeatures(features.FeatureGates()) + managed.InitManagedTransport(logr.Discard()) type cleanupFunc func() From 45ee564e27f3016d6be27d7aee70c4312a8ce0d7 Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Thu, 9 Jun 2022 21:50:46 +0530 Subject: [PATCH 2/2] assert state of managed transport acc for each test Signed-off-by: Sanskar Jaiswal --- pkg/git/libgit2/checkout_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index d04b6e416..07bc46b25 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -30,6 +30,8 @@ import ( . "github.com/onsi/gomega" "github.com/fluxcd/source-controller/pkg/git" + + mt "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" ) func TestCheckoutBranch_unmanaged(t *testing.T) { @@ -136,6 +138,7 @@ func checkoutBranch(t *testing.T, managed bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + g.Expect(mt.Enabled()).To(Equal(managed)) branch := CheckoutBranch{ Branch: tt.branch, @@ -226,6 +229,7 @@ func checkoutTag(t *testing.T, managed bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + g.Expect(mt.Enabled()).To(Equal(managed)) server, err := gittestserver.NewTempGitServer() g.Expect(err).ToNot(HaveOccurred()) @@ -315,6 +319,7 @@ func TestCheckoutCommit_unmanaged(t *testing.T) { // via CheckoutCommit. func checkoutCommit(t *testing.T, managed bool) { g := NewWithT(t) + g.Expect(mt.Enabled()).To(Equal(managed)) server, err := gittestserver.NewTempGitServer() if err != nil { @@ -493,6 +498,7 @@ func checkoutSemVer(t *testing.T, managed bool) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + g.Expect(mt.Enabled()).To(Equal(managed)) semVer := CheckoutSemVer{ SemVer: tt.constraint, @@ -611,6 +617,8 @@ func mockSignature(time time.Time) *git2go.Signature { func TestInitializeRepoWithRemote(t *testing.T) { g := NewWithT(t) + + g.Expect(mt.Enabled()).To(BeFalse()) tmp := t.TempDir() ctx := context.TODO() testRepoURL := "https://example.com/foo/bar"