Skip to content

Commit b6d6b59

Browse files
aryan9600Paulo Gomes
aryan9600
authored and
Paulo Gomes
committed
gitrepo: refactor reconciler to use fluxcd/pkg/git
Signed-off-by: Sanskar Jaiswal <[email protected]>
1 parent a9a85b2 commit b6d6b59

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+93
-7239
lines changed

‎controllers/gitrepository_controller.go

Lines changed: 48 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"net/url"
2324
"os"
2425
"path/filepath"
2526
"strings"
@@ -41,6 +42,9 @@ import (
4142
"sigs.k8s.io/controller-runtime/pkg/ratelimiter"
4243

4344
"github.com/fluxcd/pkg/apis/meta"
45+
"github.com/fluxcd/pkg/git"
46+
"github.com/fluxcd/pkg/git/gogit"
47+
"github.com/fluxcd/pkg/git/libgit2"
4448
"github.com/fluxcd/pkg/runtime/conditions"
4549
helper "github.com/fluxcd/pkg/runtime/controller"
4650
"github.com/fluxcd/pkg/runtime/events"
@@ -54,8 +58,6 @@ import (
5458
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
5559
"github.com/fluxcd/source-controller/internal/reconcile/summarize"
5660
"github.com/fluxcd/source-controller/internal/util"
57-
"github.com/fluxcd/source-controller/pkg/git"
58-
"github.com/fluxcd/source-controller/pkg/git/strategy"
5961
)
6062

6163
// gitRepositoryReadyCondition contains the information required to summarize a
@@ -440,9 +442,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
440442
conditions.Delete(obj, sourcev1.SourceVerifiedCondition)
441443
}
442444

443-
// Configure authentication strategy to access the source
444-
var authOpts *git.AuthOptions
445-
var err error
445+
var data map[string][]byte
446446
if obj.Spec.SecretRef != nil {
447447
// Attempt to retrieve secret
448448
name := types.NamespacedName{
@@ -459,12 +459,29 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
459459
// Return error as the world as observed may change
460460
return sreconcile.ResultEmpty, e
461461
}
462+
data = secret.Data
463+
}
462464

463-
// Configure strategy with secret
464-
authOpts, err = git.AuthOptionsFromSecret(obj.Spec.URL, &secret)
465-
} else {
466-
// Set the minimal auth options for valid transport.
467-
authOpts, err = git.AuthOptionsWithoutSecret(obj.Spec.URL)
465+
u, err := url.Parse(obj.Spec.URL)
466+
if err != nil {
467+
e := serror.NewStalling(
468+
fmt.Errorf("failed to parse url '%s': %w", obj.Spec.URL, err),
469+
sourcev1.URLInvalidReason,
470+
)
471+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
472+
return sreconcile.ResultEmpty, e
473+
}
474+
475+
// Configure authentication strategy to access the source
476+
authOpts, err := git.NewAuthOptions(*u, data)
477+
478+
if err != nil {
479+
e := serror.NewGeneric(
480+
fmt.Errorf("failed to configure authentication options: %w", err),
481+
sourcev1.AuthenticationFailedReason,
482+
)
483+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
484+
return sreconcile.ResultEmpty, e
468485
}
469486
if err != nil {
470487
e := serror.NewGeneric(
@@ -725,59 +742,49 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
725742
func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context,
726743
obj *sourcev1.GitRepository, authOpts *git.AuthOptions, dir string, optimized bool) (*git.Commit, error) {
727744
// Configure checkout strategy.
728-
checkoutOpts := git.CheckoutOptions{RecurseSubmodules: obj.Spec.RecurseSubmodules}
745+
cloneOpts := git.CloneOptions{
746+
RecurseSubmodules: obj.Spec.RecurseSubmodules,
747+
ShallowClone: true,
748+
}
729749
if ref := obj.Spec.Reference; ref != nil {
730-
checkoutOpts.Branch = ref.Branch
731-
checkoutOpts.Commit = ref.Commit
732-
checkoutOpts.Tag = ref.Tag
733-
checkoutOpts.SemVer = ref.SemVer
750+
cloneOpts.Branch = ref.Branch
751+
cloneOpts.Commit = ref.Commit
752+
cloneOpts.Tag = ref.Tag
753+
cloneOpts.SemVer = ref.SemVer
734754
}
735755

736756
// Only if the object has an existing artifact in storage, attempt to
737757
// short-circuit clone operation. reconcileStorage has already verified
738758
// that the artifact exists.
739759
if optimized && conditions.IsTrue(obj, sourcev1.ArtifactInStorageCondition) {
740760
if artifact := obj.GetArtifact(); artifact != nil {
741-
checkoutOpts.LastRevision = artifact.Revision
761+
cloneOpts.LastObservedCommit = artifact.Revision
742762
}
743763
}
744764

745765
gitCtx, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
746766
defer cancel()
747767

748-
checkoutStrategy, err := strategy.CheckoutStrategyForImplementation(gitCtx,
749-
git.Implementation(obj.Spec.GitImplementation), checkoutOpts)
768+
var gitReader git.RepositoryReader
769+
var err error
770+
771+
if obj.Spec.GitImplementation == libgit2.ClientName {
772+
gitReader, err = libgit2.NewClient(dir, authOpts)
773+
} else {
774+
gitReader, err = gogit.NewClient(dir, authOpts)
775+
}
750776
if err != nil {
751777
// Do not return err as recovery without changes is impossible.
752778
e := &serror.Stalling{
753-
Err: fmt.Errorf("failed to configure checkout strategy for Git implementation '%s': %w", obj.Spec.GitImplementation, err),
779+
Err: fmt.Errorf("failed to create Git client for implementation '%s': %w", obj.Spec.GitImplementation, err),
754780
Reason: sourcev1.GitOperationFailedReason,
755781
}
756782
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
757783
return nil, e
758784
}
785+
defer gitReader.Close()
759786

760-
// this is needed only for libgit2, due to managed transport.
761-
if obj.Spec.GitImplementation == sourcev1.LibGit2Implementation {
762-
// We set the TransportOptionsURL of this set of authentication options here by constructing
763-
// a unique URL that won't clash in a multi tenant environment. This unique URL is used by
764-
// libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in
765-
// libgit2, which is inflexible and unstable.
766-
if strings.HasPrefix(obj.Spec.URL, "http") {
767-
authOpts.TransportOptionsURL = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
768-
} else if strings.HasPrefix(obj.Spec.URL, "ssh") {
769-
authOpts.TransportOptionsURL = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
770-
} else {
771-
e := &serror.Stalling{
772-
Err: fmt.Errorf("git repository URL '%s' has invalid transport type, supported types are: http, https, ssh", obj.Spec.URL),
773-
Reason: sourcev1.URLInvalidReason,
774-
}
775-
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
776-
return nil, e
777-
}
778-
}
779-
780-
commit, err := checkoutStrategy.Checkout(gitCtx, dir, obj.Spec.URL, authOpts)
787+
commit, err := gitReader.Clone(gitCtx, obj.Spec.URL, cloneOpts)
781788
if err != nil {
782789
e := serror.NewGeneric(
783790
fmt.Errorf("failed to checkout and determine revision: %w", err),
@@ -786,6 +793,7 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context,
786793
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
787794
return nil, e
788795
}
796+
789797
return commit, nil
790798
}
791799

‎controllers/gitrepository_controller_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ import (
5656
"github.com/fluxcd/pkg/ssh"
5757
"github.com/fluxcd/pkg/testserver"
5858

59+
"github.com/fluxcd/pkg/git"
60+
"github.com/fluxcd/pkg/git/libgit2/transport"
5961
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
6062
serror "github.com/fluxcd/source-controller/internal/error"
6163
"github.com/fluxcd/source-controller/internal/features"
6264
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
6365
"github.com/fluxcd/source-controller/internal/reconcile/summarize"
64-
"github.com/fluxcd/source-controller/pkg/git"
65-
"github.com/fluxcd/source-controller/pkg/git/libgit2/managed"
6666
)
6767

6868
const (
@@ -502,7 +502,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
502502
EventRecorder: record.NewFakeRecorder(32),
503503
Storage: testStorage,
504504
features: features.FeatureGates(),
505-
Libgit2TransportInitialized: managed.Enabled,
505+
Libgit2TransportInitialized: transport.Enabled,
506506
}
507507

508508
for _, i := range testGitImplementations {
@@ -731,7 +731,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
731731
EventRecorder: record.NewFakeRecorder(32),
732732
Storage: testStorage,
733733
features: features.FeatureGates(),
734-
Libgit2TransportInitialized: managed.Enabled,
734+
Libgit2TransportInitialized: transport.Enabled,
735735
}
736736

737737
for _, tt := range tests {
@@ -1404,7 +1404,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
14041404
},
14051405
wantErr: true,
14061406
assertConditions: []metav1.Condition{
1407-
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: failed to verify commit with any of the given key rings"),
1407+
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: unable to verify commit with any of the given key rings"),
14081408
},
14091409
},
14101410
{
@@ -1599,7 +1599,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) {
15991599
EventRecorder: record.NewFakeRecorder(32),
16001600
Storage: testStorage,
16011601
features: features.FeatureGates(),
1602-
Libgit2TransportInitialized: managed.Enabled,
1602+
Libgit2TransportInitialized: transport.Enabled,
16031603
}
16041604

16051605
key := client.ObjectKeyFromObject(obj)

‎controllers/ocirepository_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import (
5757
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
5858

5959
"github.com/fluxcd/pkg/apis/meta"
60+
"github.com/fluxcd/pkg/git"
6061
"github.com/fluxcd/pkg/oci"
6162
"github.com/fluxcd/pkg/runtime/conditions"
6263
conditionscheck "github.com/fluxcd/pkg/runtime/conditions/check"
@@ -66,7 +67,6 @@ import (
6667
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
6768
serror "github.com/fluxcd/source-controller/internal/error"
6869
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
69-
"github.com/fluxcd/source-controller/pkg/git"
7070
)
7171

7272
func TestOCIRepository_Reconcile(t *testing.T) {

‎controllers/suite_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
ctrl "sigs.k8s.io/controller-runtime"
3838

3939
dcontext "github.com/distribution/distribution/v3/context"
40+
"github.com/fluxcd/pkg/git/libgit2/transport"
4041
"github.com/fluxcd/pkg/runtime/controller"
4142
"github.com/fluxcd/pkg/runtime/testenv"
4243
"github.com/fluxcd/pkg/testserver"
@@ -53,7 +54,6 @@ import (
5354
"github.com/fluxcd/source-controller/internal/cache"
5455
"github.com/fluxcd/source-controller/internal/features"
5556
"github.com/fluxcd/source-controller/internal/helm/registry"
56-
"github.com/fluxcd/source-controller/pkg/git/libgit2/managed"
5757
// +kubebuilder:scaffold:imports
5858
)
5959

@@ -237,7 +237,7 @@ func TestMain(m *testing.M) {
237237
panic(fmt.Sprintf("Failed to create a test registry server: %v", err))
238238
}
239239

240-
if err = managed.InitManagedTransport(); err != nil {
240+
if err = transport.InitManagedTransport(); err != nil {
241241
panic(fmt.Sprintf("Failed to initialize libgit2 managed transport: %v", err))
242242
}
243243

@@ -247,7 +247,7 @@ func TestMain(m *testing.M) {
247247
Metrics: testMetricsH,
248248
Storage: testStorage,
249249
features: features.FeatureGates(),
250-
Libgit2TransportInitialized: managed.Enabled,
250+
Libgit2TransportInitialized: transport.Enabled,
251251
}).SetupWithManager(testEnv); err != nil {
252252
panic(fmt.Sprintf("Failed to start GitRepositoryReconciler: %v", err))
253253
}

‎go.mod

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ replace github.com/fluxcd/source-controller/api => ./api
1212
// - libgit2/git2go#918.
1313
replace github.com/libgit2/git2go/v33 => github.com/fluxcd/git2go/v33 v33.0.9-flux
1414

15+
// Fix CVE-2022-1996 (for v2, Go Modules incompatible)
16+
replace github.com/emicklei/go-restful => github.com/emicklei/go-restful v2.16.0+incompatible
17+
1518
require (
1619
cloud.google.com/go/storage v1.27.0
1720
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.1.3
@@ -22,16 +25,18 @@ require (
2225
// maintained by the ProtonMail team to continue to support the openpgp
2326
// module, after the Go team decided to no longer maintain it.
2427
// When in doubt (and not using openpgp), use /x/crypto.
25-
github.com/ProtonMail/go-crypto v0.0.0-20220930113650-c6815a8c17ad
28+
github.com/ProtonMail/go-crypto v0.0.0-20220930113650-c6815a8c17ad // indirect
2629
github.com/cyphar/filepath-securejoin v0.2.3
2730
github.com/distribution/distribution/v3 v3.0.0-20221019080424-fb2188868d77
2831
github.com/docker/cli v20.10.20+incompatible
2932
github.com/docker/go-units v0.5.0
30-
github.com/elazarl/goproxy v0.0.0-20221015165544-a0805db90819
31-
github.com/fluxcd/gitkit v0.6.0
33+
github.com/elazarl/goproxy v0.0.0-20221015165544-a0805db90819 // indirect
34+
github.com/fluxcd/gitkit v0.6.0 // indirect
3235
github.com/fluxcd/pkg/apis/meta v0.17.0
33-
github.com/fluxcd/pkg/gittestserver v0.7.0
34-
github.com/fluxcd/pkg/gitutil v0.2.0
36+
github.com/fluxcd/pkg/git v0.6.1
37+
github.com/fluxcd/pkg/git/gogit v0.1.1-0.20220902101857-4d204a4a6fa4
38+
github.com/fluxcd/pkg/git/libgit2 v0.1.1-0.20220927151444-1d5a7b25a55f
39+
github.com/fluxcd/pkg/gitutil v0.2.0 // indirect
3540
github.com/fluxcd/pkg/helmtestserver v0.9.0
3641
github.com/fluxcd/pkg/lockedfile v0.1.0
3742
github.com/fluxcd/pkg/masktoken v0.2.0
@@ -60,7 +65,7 @@ require (
6065
github.com/sirupsen/logrus v1.9.0
6166
github.com/spf13/pflag v1.0.5
6267
golang.org/x/crypto v0.1.0
63-
golang.org/x/net v0.1.0
68+
golang.org/x/net v0.1.0 // indirect
6469
golang.org/x/sync v0.1.0
6570
google.golang.org/api v0.100.0
6671
gotest.tools v2.2.0+incompatible
@@ -74,11 +79,7 @@ require (
7479
sigs.k8s.io/yaml v1.3.0
7580
)
7681

77-
// Fix CVE-2022-32149
78-
replace golang.org/x/text => golang.org/x/text v0.4.0
79-
80-
// Fix CVE-2022-1996 (for v2, Go Modules incompatible)
81-
replace github.com/emicklei/go-restful => github.com/emicklei/go-restful v2.16.0+incompatible
82+
require github.com/fluxcd/pkg/gittestserver v0.7.0
8283

8384
require (
8485
bitbucket.org/creachadair/shell v0.0.7 // indirect
@@ -179,6 +180,7 @@ require (
179180
github.com/fatih/color v1.13.0 // indirect
180181
github.com/felixge/httpsnoop v1.0.3 // indirect
181182
github.com/fluxcd/pkg/apis/acl v0.1.0 // indirect
183+
github.com/fluxcd/pkg/http/transport v0.0.1 // indirect
182184
github.com/fsnotify/fsnotify v1.5.4 // indirect
183185
github.com/fullstorydev/grpcurl v1.8.7 // indirect
184186
github.com/go-chi/chi v4.1.2+incompatible // indirect

0 commit comments

Comments
 (0)