diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index ae922ff3..04e40445 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -455,21 +455,6 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, return sreconcile.ResultEmpty, e } - repositoryURL := obj.Spec.URL - // managed GIT transport only affects the libgit2 implementation - if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation { - // We set the TransportAuthID of this set of authentication options here by constructing - // a unique ID that won't clash in a multi tenant environment. This unique ID is used by - // libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in - // libgit2, which is inflexible and unstable. - if strings.HasPrefix(repositoryURL, "http") { - authOpts.TransportAuthID = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation) - } - if strings.HasPrefix(repositoryURL, "ssh") { - authOpts.TransportAuthID = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation) - } - } - // Fetch the included artifact metadata. artifacts, err := r.fetchIncludes(ctx, obj) if err != nil { @@ -491,7 +476,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, optimizedClone = true } - c, err := r.gitCheckout(ctx, obj, repositoryURL, authOpts, dir, optimizedClone) + c, err := r.gitCheckout(ctx, obj, authOpts, dir, optimizedClone) if err != nil { return sreconcile.ResultEmpty, err } @@ -525,7 +510,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, // If we can't skip the reconciliation, checkout again without any // optimization. - c, err := r.gitCheckout(ctx, obj, repositoryURL, authOpts, dir, false) + c, err := r.gitCheckout(ctx, obj, authOpts, dir, false) if err != nil { return sreconcile.ResultEmpty, err } @@ -717,7 +702,7 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, // gitCheckout builds checkout options with the given configurations and // performs a git checkout. func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, - obj *sourcev1.GitRepository, repoURL string, authOpts *git.AuthOptions, dir string, optimized bool) (*git.Commit, error) { + obj *sourcev1.GitRepository, authOpts *git.AuthOptions, dir string, optimized bool) (*git.Commit, error) { // Configure checkout strategy. checkoutOpts := git.CheckoutOptions{RecurseSubmodules: obj.Spec.RecurseSubmodules} if ref := obj.Spec.Reference; ref != nil { @@ -743,15 +728,34 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, Err: fmt.Errorf("failed to configure checkout strategy for Git implementation '%s': %w", obj.Spec.GitImplementation, err), Reason: sourcev1.GitOperationFailedReason, } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) // Do not return err as recovery without changes is impossible. return nil, e } + // managed GIT transport only affects the libgit2 implementation + if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation { + // We set the TransportOptionsURL of this set of authentication options here by constructing + // a unique ID that won't clash in a multi tenant environment. This unique ID is used by + // libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in + // libgit2, which is inflexible and unstable. + if strings.HasPrefix(obj.Spec.URL, "http") { + authOpts.TransportOptionsURL = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation) + } else if strings.HasPrefix(obj.Spec.URL, "ssh") { + authOpts.TransportOptionsURL = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation) + } else { + e := &serror.Stalling{ + Err: fmt.Errorf("git repository URL has invalid transport type: '%s'", obj.Spec.URL), + Reason: sourcev1.GitOperationFailedReason, + } + return nil, e + } + } + // Checkout HEAD of reference in object gitCtx, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration) defer cancel() - commit, err := checkoutStrategy.Checkout(gitCtx, dir, repoURL, authOpts) + + commit, err := checkoutStrategy.Checkout(gitCtx, dir, obj.Spec.URL, authOpts) if err != nil { e := serror.NewGeneric( fmt.Errorf("failed to checkout and determine revision: %w", err), diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 50a9463f..a30f608b 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -362,7 +362,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "failed to checkout and determine revision: unable to fetch-connect to remote '': PEM CA bundle could not be appended to x509 certificate pool"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "failed to checkout and determine revision: unable to clone '': PEM CA bundle could not be appended to x509 certificate pool"), }, }, { @@ -645,10 +645,11 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) } conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo") }, - want: sreconcile.ResultEmpty, - wantErr: true, - wantRevision: "staging/", - wantArtifactOutdated: false, + want: sreconcile.ResultEmpty, + wantErr: true, + wantRevision: "staging/", + wantArtifactOutdated: false, + skipForImplementation: "libgit2", }, { name: "Optimized clone different ignore", @@ -669,9 +670,10 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) } conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo") }, - want: sreconcile.ResultSuccess, - wantRevision: "staging/", - wantArtifactOutdated: false, + want: sreconcile.ResultSuccess, + wantRevision: "staging/", + wantArtifactOutdated: false, + skipForImplementation: "libgit2", }, } diff --git a/go.mod b/go.mod index 4f00f6f4..52dc835b 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( github.com/fluxcd/pkg/helmtestserver v0.7.2 github.com/fluxcd/pkg/lockedfile v0.1.0 github.com/fluxcd/pkg/runtime v0.16.1 - github.com/fluxcd/pkg/ssh v0.3.4 + github.com/fluxcd/pkg/ssh v0.4.0 github.com/fluxcd/pkg/testserver v0.2.0 github.com/fluxcd/pkg/untar v0.1.0 github.com/fluxcd/pkg/version v0.1.0 diff --git a/go.sum b/go.sum index c59b7956..4dfcd259 100644 --- a/go.sum +++ b/go.sum @@ -282,8 +282,8 @@ github.com/fluxcd/pkg/lockedfile v0.1.0 h1:YsYFAkd6wawMCcD74ikadAKXA4s2sukdxrn7w github.com/fluxcd/pkg/lockedfile v0.1.0/go.mod h1:EJLan8t9MiOcgTs8+puDjbE6I/KAfHbdvIy9VUgIjm8= github.com/fluxcd/pkg/runtime v0.16.1 h1:WU1vNZz4TAzmATQ/tl2zB/FX6GIUTgYeBn/G5RuTA2c= github.com/fluxcd/pkg/runtime v0.16.1/go.mod h1:cgVJkOXCg9OmrIUGklf/0UtV28MNzkuoBJhaEQICT6E= -github.com/fluxcd/pkg/ssh v0.3.4 h1:Ko+MUNiiQG3evyoMO19iRk7d4X0VJ6w6+GEeVQ1jLC0= -github.com/fluxcd/pkg/ssh v0.3.4/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE= +github.com/fluxcd/pkg/ssh v0.4.0 h1:2HY88irZ5BCSMlzZExR6cnhRkjxCDsK/lTHHQqCJDJQ= +github.com/fluxcd/pkg/ssh v0.4.0/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE= github.com/fluxcd/pkg/testserver v0.2.0 h1:Mj0TapmKaywI6Fi5wvt1LAZpakUHmtzWQpJNKQ0Krt4= github.com/fluxcd/pkg/testserver v0.2.0/go.mod h1:bgjjydkXsZTeFzjz9Cr4heGANr41uTB1Aj1Q5qzuYVk= github.com/fluxcd/pkg/untar v0.1.0 h1:k97V/xV5hFrAkIkVPuv5AVhyxh1ZzzAKba/lbDfGo6o= diff --git a/main.go b/main.go index 50a6bc55..fb54cb74 100644 --- a/main.go +++ b/main.go @@ -312,6 +312,14 @@ func main() { if enabled, _ := features.Enabled(features.GitManagedTransport); enabled { managed.InitManagedTransport(ctrl.Log.WithName("managed-transport")) + } else { + if optimize, _ := feathelper.Enabled(features.OptimizedGitClones); optimize { + setupLog.Error( + fmt.Errorf("OptimizedGitClones=true but GitManagedTransport=false"), + "git clones can only be optimized when using managed transort", + ) + os.Exit(1) + } } setupLog.Info("starting manager") diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 56752951..83c602f8 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -69,113 +69,148 @@ type CheckoutBranch struct { func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + // This branching is temporary, to address the transient panics observed when using unmanaged transport. + // The panics probably happen because we perform multiple fetch ops (introduced as a part of optimizing git clones). + // The branching lets us establish a clear code path to help us be certain of the expected behaviour. + // When we get rid of unmanaged transports, we can get rid of this branching as well. if managed.Enabled() { - // We store the target url and auth options mapped to a unique ID. We overwrite the target url - // with the TransportAuthID, because managed transports don't provide a way for any kind of + // We store the target URL and auth options mapped to a unique ID. We overwrite the target URL + // with the TransportOptionsURL, because managed transports don't provide a way for any kind of // dependency injection. This lets us have a way of doing interop between application level code // and transport level code. - // Performing all fetch operations with the TransportAuthID as the url, lets the managed + // Performing all fetch operations with the TransportOptionsURL as the URL, lets the managed // transport action use it to fetch the registered transport options which contains the - // _actual_ target url and the correct credentials to use. - managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ - TargetURL: url, - AuthOpts: opts, - }) - url = opts.TransportAuthID - defer managed.RemoveTransportOptions(opts.TransportAuthID) - } - - remoteCallBacks := RemoteCallbacks(ctx, opts) - proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto} - - repo, remote, err := initializeRepoWithRemote(ctx, path, url, opts) - if err != nil { - return nil, err - } - // Open remote connection. - err = remote.ConnectFetch(&remoteCallBacks, proxyOpts, nil) - if err != nil { - remote.Free() - repo.Free() - return nil, fmt.Errorf("unable to fetch-connect to remote '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) - } - defer func() { - remote.Disconnect() - remote.Free() - repo.Free() - }() - - // When the last observed revision is set, check whether it is still the - // same at the remote branch. If so, short-circuit the clone operation here. - if c.LastRevision != "" { - heads, err := remote.Ls(c.Branch) - if err != nil { - return nil, fmt.Errorf("unable to remote ls for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + // _actual_ target URL and the correct credentials to use. + if opts == nil { + return nil, fmt.Errorf("can't use managed transport with an empty set of auth options") } - if len(heads) > 0 { - hash := heads[0].Id.String() - currentRevision := fmt.Sprintf("%s/%s", c.Branch, hash) - if currentRevision == c.LastRevision { - // Construct a partial commit with the existing information. - c := &git.Commit{ - Hash: git.Hash(hash), - Reference: "refs/heads/" + c.Branch, + if opts.TransportOptionsURL == "" { + return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.") + } + managed.AddTransportOptions(opts.TransportOptionsURL, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + ProxyOptions: &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, + }) + url = opts.TransportOptionsURL + remoteCallBacks := managed.RemoteCallbacks() + defer managed.RemoveTransportOptions(opts.TransportOptionsURL) + + repo, remote, err := initializeRepoWithRemote(ctx, path, url, opts) + if err != nil { + return nil, err + } + // Open remote connection. + err = remote.ConnectFetch(&remoteCallBacks, nil, nil) + if err != nil { + remote.Free() + repo.Free() + return nil, fmt.Errorf("unable to fetch-connect to remote '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + defer func() { + remote.Disconnect() + remote.Free() + repo.Free() + }() + + // When the last observed revision is set, check whether it is still the + // same at the remote branch. If so, short-circuit the clone operation here. + if c.LastRevision != "" { + heads, err := remote.Ls(c.Branch) + if err != nil { + return nil, fmt.Errorf("unable to remote ls for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + if len(heads) > 0 { + hash := heads[0].Id.String() + currentRevision := fmt.Sprintf("%s/%s", c.Branch, hash) + if currentRevision == c.LastRevision { + // Construct a partial commit with the existing information. + c := &git.Commit{ + Hash: git.Hash(hash), + Reference: "refs/heads/" + c.Branch, + } + return c, nil } - return c, nil } } - } - // Limit the fetch operation to the specific branch, to decrease network usage. - err = remote.Fetch([]string{c.Branch}, - &git2go.FetchOptions{ - DownloadTags: git2go.DownloadTagsNone, - RemoteCallbacks: remoteCallBacks, - ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, - }, - "") - if err != nil { - return nil, fmt.Errorf("unable to fetch remote '%s': %w", - managed.EffectiveURL(url), gitutil.LibGit2Error(err)) - } + // Limit the fetch operation to the specific branch, to decrease network usage. + err = remote.Fetch([]string{c.Branch}, + &git2go.FetchOptions{ + DownloadTags: git2go.DownloadTagsNone, + RemoteCallbacks: remoteCallBacks, + }, + "") + if err != nil { + return nil, fmt.Errorf("unable to fetch remote '%s': %w", + managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } - branch, err := repo.References.Lookup(fmt.Sprintf("refs/remotes/origin/%s", c.Branch)) - if err != nil { - return nil, fmt.Errorf("unable to lookup branch '%s' for '%s': %w", - c.Branch, managed.EffectiveURL(url), gitutil.LibGit2Error(err)) - } - defer branch.Free() + branch, err := repo.References.Lookup(fmt.Sprintf("refs/remotes/origin/%s", c.Branch)) + if err != nil { + return nil, fmt.Errorf("unable to lookup branch '%s' for '%s': %w", + c.Branch, managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + defer branch.Free() - upstreamCommit, err := repo.LookupCommit(branch.Target()) - if err != nil { - return nil, fmt.Errorf("unable to lookup commit '%s' for '%s': %w", - c.Branch, managed.EffectiveURL(url), gitutil.LibGit2Error(err)) - } - defer upstreamCommit.Free() + upstreamCommit, err := repo.LookupCommit(branch.Target()) + if err != nil { + return nil, fmt.Errorf("unable to lookup commit '%s' for '%s': %w", + c.Branch, managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + defer upstreamCommit.Free() - // Once the index has been updated with Fetch, and we know the tip commit, - // a hard reset can be used to align the local worktree with the remote branch's. - err = repo.ResetToCommit(upstreamCommit, git2go.ResetHard, &git2go.CheckoutOptions{ - Strategy: git2go.CheckoutForce, - }) - if err != nil { - return nil, fmt.Errorf("unable to hard reset to commit for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) - } + // Once the index has been updated with Fetch, and we know the tip commit, + // a hard reset can be used to align the local worktree with the remote branch's. + err = repo.ResetToCommit(upstreamCommit, git2go.ResetHard, &git2go.CheckoutOptions{ + Strategy: git2go.CheckoutForce, + }) + if err != nil { + return nil, fmt.Errorf("unable to hard reset to commit for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } - // Use the current worktree's head as reference for the commit to be returned. - head, err := repo.Head() - if err != nil { - return nil, fmt.Errorf("git resolve HEAD error: %w", err) - } - defer head.Free() + // Use the current worktree's head as reference for the commit to be returned. + head, err := repo.Head() + if err != nil { + return nil, fmt.Errorf("git resolve HEAD error: %w", err) + } + defer head.Free() - cc, err := repo.LookupCommit(head.Target()) - if err != nil { - return nil, fmt.Errorf("failed to lookup HEAD commit '%s' for branch '%s': %w", head.Target(), c.Branch, err) - } - defer cc.Free() + cc, err := repo.LookupCommit(head.Target()) + if err != nil { + return nil, fmt.Errorf("failed to lookup HEAD commit '%s' for branch '%s': %w", head.Target(), c.Branch, err) + } + defer cc.Free() - return buildCommit(cc, "refs/heads/"+c.Branch), nil + return buildCommit(cc, "refs/heads/"+c.Branch), nil + } else { + repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ + FetchOptions: git2go.FetchOptions{ + DownloadTags: git2go.DownloadTagsNone, + RemoteCallbacks: RemoteCallbacks(ctx, opts), + ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, + }, + CheckoutOptions: git2go.CheckoutOptions{ + Strategy: git2go.CheckoutForce, + }, + CheckoutBranch: c.Branch, + }) + if err != nil { + return nil, fmt.Errorf("unable to clone '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + defer repo.Free() + head, err := repo.Head() + if err != nil { + return nil, fmt.Errorf("git resolve HEAD error: %w", err) + } + defer head.Free() + cc, err := repo.LookupCommit(head.Target()) + if err != nil { + return nil, fmt.Errorf("failed to lookup HEAD commit '%s' for branch '%s': %w", head.Target(), c.Branch, err) + } + defer cc.Free() + return buildCommit(cc, "refs/heads/"+c.Branch), nil + } } type CheckoutTag struct { @@ -186,85 +221,108 @@ type CheckoutTag struct { func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + // This branching is temporary, to address the transient panics observed when using unmanaged transport. + // The panics probably happen because we perform multiple fetch ops (introduced as a part of optimizing git clones). + // The branching lets us establish a clear code path to help us be certain of the expected behaviour. + // When we get rid of unmanaged transports, we can get rid of this branching as well. if managed.Enabled() { - managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ - TargetURL: url, - AuthOpts: opts, + if opts.TransportOptionsURL == "" { + return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.") + } + managed.AddTransportOptions(opts.TransportOptionsURL, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + ProxyOptions: &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, }) - url = opts.TransportAuthID - defer managed.RemoveTransportOptions(opts.TransportAuthID) - } + url = opts.TransportOptionsURL + remoteCallBacks := managed.RemoteCallbacks() + defer managed.RemoveTransportOptions(opts.TransportOptionsURL) - remoteCallBacks := RemoteCallbacks(ctx, opts) - proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto} - - repo, remote, err := initializeRepoWithRemote(ctx, path, url, opts) - if err != nil { - return nil, err - } - // Open remote connection. - err = remote.ConnectFetch(&remoteCallBacks, proxyOpts, nil) - if err != nil { - remote.Free() - repo.Free() - return nil, fmt.Errorf("unable to fetch-connect to remote '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) - } - defer func() { - remote.Disconnect() - remote.Free() - repo.Free() - }() - - // When the last observed revision is set, check whether it is still the - // same at the remote branch. If so, short-circuit the clone operation here. - if c.LastRevision != "" { - heads, err := remote.Ls(c.Tag) + repo, remote, err := initializeRepoWithRemote(ctx, path, url, opts) if err != nil { - return nil, fmt.Errorf("unable to remote ls for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + return nil, err } - if len(heads) > 0 { - hash := heads[0].Id.String() - currentRevision := fmt.Sprintf("%s/%s", c.Tag, hash) - var same bool - if currentRevision == c.LastRevision { - same = true - } else if len(heads) > 1 { - hash = heads[1].Id.String() - currentAnnotatedRevision := fmt.Sprintf("%s/%s", c.Tag, hash) - if currentAnnotatedRevision == c.LastRevision { + // Open remote connection. + err = remote.ConnectFetch(&remoteCallBacks, nil, nil) + if err != nil { + remote.Free() + repo.Free() + return nil, fmt.Errorf("unable to fetch-connect to remote '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + defer func() { + remote.Disconnect() + remote.Free() + repo.Free() + }() + + // When the last observed revision is set, check whether it is still the + // same at the remote branch. If so, short-circuit the clone operation here. + if c.LastRevision != "" { + heads, err := remote.Ls(c.Tag) + if err != nil { + return nil, fmt.Errorf("unable to remote ls for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + if len(heads) > 0 { + hash := heads[0].Id.String() + currentRevision := fmt.Sprintf("%s/%s", c.Tag, hash) + var same bool + if currentRevision == c.LastRevision { same = true + } else if len(heads) > 1 { + hash = heads[1].Id.String() + currentAnnotatedRevision := fmt.Sprintf("%s/%s", c.Tag, hash) + if currentAnnotatedRevision == c.LastRevision { + same = true + } } - } - if same { - // Construct a partial commit with the existing information. - c := &git.Commit{ - Hash: git.Hash(hash), - Reference: "refs/tags/" + c.Tag, + if same { + // Construct a partial commit with the existing information. + c := &git.Commit{ + Hash: git.Hash(hash), + Reference: "refs/tags/" + c.Tag, + } + return c, nil } - return c, nil } } - } - err = remote.Fetch([]string{c.Tag}, - &git2go.FetchOptions{ - DownloadTags: git2go.DownloadTagsAuto, - RemoteCallbacks: remoteCallBacks, - ProxyOptions: *proxyOpts, - }, - "") + err = remote.Fetch([]string{c.Tag}, + &git2go.FetchOptions{ + DownloadTags: git2go.DownloadTagsAuto, + RemoteCallbacks: remoteCallBacks, + }, + "") - if err != nil { - return nil, fmt.Errorf("unable to fetch remote '%s': %w", - managed.EffectiveURL(url), gitutil.LibGit2Error(err)) - } + if err != nil { + return nil, fmt.Errorf("unable to fetch remote '%s': %w", + managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } - cc, err := checkoutDetachedDwim(repo, c.Tag) - if err != nil { - return nil, err + cc, err := checkoutDetachedDwim(repo, c.Tag) + if err != nil { + return nil, err + } + defer cc.Free() + return buildCommit(cc, "refs/tags/"+c.Tag), nil + } else { + repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ + FetchOptions: git2go.FetchOptions{ + DownloadTags: git2go.DownloadTagsAll, + RemoteCallbacks: RemoteCallbacks(ctx, opts), + ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, + }, + }) + if err != nil { + return nil, fmt.Errorf("unable to clone '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err)) + } + defer repo.Free() + cc, err := checkoutDetachedDwim(repo, c.Tag) + if err != nil { + return nil, err + } + defer cc.Free() + return buildCommit(cc, "refs/tags/"+c.Tag), nil } - defer cc.Free() - return buildCommit(cc, "refs/tags/"+c.Tag), nil } type CheckoutCommit struct { @@ -274,20 +332,26 @@ type CheckoutCommit struct { func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + remoteCallBacks := RemoteCallbacks(ctx, opts) + if managed.Enabled() { - managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ - TargetURL: url, - AuthOpts: opts, + if opts.TransportOptionsURL == "" { + return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.") + } + managed.AddTransportOptions(opts.TransportOptionsURL, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + ProxyOptions: &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, }) - url = opts.TransportAuthID - defer managed.RemoveTransportOptions(opts.TransportAuthID) + url = opts.TransportOptionsURL + remoteCallBacks = managed.RemoteCallbacks() + defer managed.RemoveTransportOptions(opts.TransportOptionsURL) } repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, - RemoteCallbacks: RemoteCallbacks(ctx, opts), - ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, + RemoteCallbacks: remoteCallBacks, }, }) if err != nil { @@ -312,13 +376,20 @@ type CheckoutSemVer struct { func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { defer recoverPanic(&err) + remoteCallBacks := RemoteCallbacks(ctx, opts) + if managed.Enabled() { - managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{ - TargetURL: url, - AuthOpts: opts, + if opts.TransportOptionsURL == "" { + return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.") + } + managed.AddTransportOptions(opts.TransportOptionsURL, managed.TransportOptions{ + TargetURL: url, + AuthOpts: opts, + ProxyOptions: &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, }) - url = opts.TransportAuthID - defer managed.RemoveTransportOptions(opts.TransportAuthID) + url = opts.TransportOptionsURL + remoteCallBacks = managed.RemoteCallbacks() + defer managed.RemoveTransportOptions(opts.TransportOptionsURL) } verConstraint, err := semver.NewConstraint(c.SemVer) @@ -329,8 +400,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, - RemoteCallbacks: RemoteCallbacks(ctx, opts), - ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, + RemoteCallbacks: remoteCallBacks, }, }) if err != nil { diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index b4f6c11d..c2fe7a12 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -77,49 +77,29 @@ func TestCheckoutBranch_Checkout(t *testing.T) { } tests := []struct { - name string - branch string - filesCreated map[string]string - lastRevision string - expectedCommit string - expectedConcreteCommit bool - expectedErr string + name string + branch string + filesCreated map[string]string + lastRevision string + expectedCommit string + expectedErr string }{ { - name: "Default branch", - branch: defaultBranch, - filesCreated: map[string]string{"branch": "second"}, - expectedCommit: secondCommit.String(), - expectedConcreteCommit: true, + name: "Default branch", + branch: defaultBranch, + filesCreated: map[string]string{"branch": "second"}, + expectedCommit: secondCommit.String(), }, { - name: "Other branch", - branch: "test", - filesCreated: map[string]string{"branch": "init"}, - expectedCommit: firstCommit.String(), - expectedConcreteCommit: true, + name: "Other branch", + branch: "test", + filesCreated: map[string]string{"branch": "init"}, + expectedCommit: firstCommit.String(), }, { - 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, + name: "Non existing branch", + branch: "invalid", + expectedErr: "reference 'refs/remotes/origin/invalid' not found", }, } @@ -142,14 +122,6 @@ func TestCheckoutBranch_Checkout(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) - 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)) - } - } }) } } @@ -161,24 +133,20 @@ func TestCheckoutTag_Checkout(t *testing.T) { } tests := []struct { - name string - tagsInRepo []testTag - checkoutTag string - lastRevTag string - expectErr string - expectConcreteCommit bool + name string + tagsInRepo []testTag + checkoutTag string + expectErr string }{ { - name: "Tag", - tagsInRepo: []testTag{{"tag-1", false}}, - checkoutTag: "tag-1", - expectConcreteCommit: true, + name: "Tag", + tagsInRepo: []testTag{{"tag-1", false}}, + checkoutTag: "tag-1", }, { - name: "Annotated", - tagsInRepo: []testTag{{"annotated", true}}, - checkoutTag: "annotated", - expectConcreteCommit: true, + name: "Annotated", + tagsInRepo: []testTag{{"annotated", true}}, + checkoutTag: "annotated", }, { name: "Non existing tag", @@ -186,18 +154,14 @@ func TestCheckoutTag_Checkout(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", - lastRevTag: "tag-1", - expectConcreteCommit: false, + name: "Skip clone - last revision unchanged", + tagsInRepo: []testTag{{"tag-1", false}}, + checkoutTag: "tag-1", }, { - name: "Last revision changed", - tagsInRepo: []testTag{{"tag-1", false}, {"tag-2", false}}, - checkoutTag: "tag-2", - lastRevTag: "tag-1", - expectConcreteCommit: true, + name: "Last revision changed", + tagsInRepo: []testTag{{"tag-1", false}, {"tag-2", false}}, + checkoutTag: "tag-2", }, } for _, tt := range tests { @@ -235,12 +199,6 @@ func TestCheckoutTag_Checkout(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) @@ -252,16 +210,12 @@ func TestCheckoutTag_Checkout(t *testing.T) { } // Check successful checkout results. - g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit)) targetTagCommit := tagCommits[tt.checkoutTag] g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc.String()).To(Equal(tt.checkoutTag + "/" + targetTagCommit.Id().String())) - // 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)) - } + g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.checkoutTag)) }) } } diff --git a/pkg/git/libgit2/managed/http.go b/pkg/git/libgit2/managed/http.go index ffccb1f6..1533e6bd 100644 --- a/pkg/git/libgit2/managed/http.go +++ b/pkg/git/libgit2/managed/http.go @@ -47,6 +47,7 @@ import ( "bytes" "crypto/tls" "crypto/x509" + "encoding/base64" "errors" "fmt" "io" @@ -55,6 +56,7 @@ import ( "sync" pool "github.com/fluxcd/source-controller/internal/transport" + "github.com/fluxcd/source-controller/pkg/git" git2go "github.com/libgit2/git2go/v33" ) @@ -86,30 +88,45 @@ type httpSmartSubtransport struct { httpTransport *http.Transport } -func (t *httpSmartSubtransport) Action(transportAuthID string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { +func (t *httpSmartSubtransport) Action(transportOptionsURL string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { + opts, found := getTransportOptions(transportOptionsURL) + + if !found { + return nil, fmt.Errorf("failed to create client: could not find transport options for the object: %s", transportOptionsURL) + } + targetURL := opts.TargetURL + + if targetURL == "" { + return nil, fmt.Errorf("repository URL cannot be empty") + } + + if len(targetURL) > URLMaxLength { + return nil, fmt.Errorf("URL exceeds the max length (%d)", URLMaxLength) + } + var proxyFn func(*http.Request) (*url.URL, error) - proxyOpts, err := t.transport.SmartProxyOptions() - if err != nil { - return nil, err - } - switch proxyOpts.Type { - case git2go.ProxyTypeNone: - proxyFn = nil - case git2go.ProxyTypeAuto: - proxyFn = http.ProxyFromEnvironment - case git2go.ProxyTypeSpecified: - parsedUrl, err := url.Parse(proxyOpts.Url) - if err != nil { - return nil, err + proxyOpts := opts.ProxyOptions + if proxyOpts != nil { + switch proxyOpts.Type { + case git2go.ProxyTypeNone: + proxyFn = nil + case git2go.ProxyTypeAuto: + proxyFn = http.ProxyFromEnvironment + case git2go.ProxyTypeSpecified: + parsedUrl, err := url.Parse(proxyOpts.Url) + if err != nil { + return nil, err + } + proxyFn = http.ProxyURL(parsedUrl) } - - proxyFn = http.ProxyURL(parsedUrl) + t.httpTransport.Proxy = proxyFn + t.httpTransport.ProxyConnectHeader = map[string][]string{} + } else { + t.httpTransport.Proxy = nil } - - t.httpTransport.Proxy = proxyFn t.httpTransport.DisableCompression = false - client, req, err := createClientRequest(transportAuthID, action, t.httpTransport) + client, req, err := createClientRequest(targetURL, action, t.httpTransport, opts.AuthOpts) if err != nil { return nil, err } @@ -142,7 +159,8 @@ func (t *httpSmartSubtransport) Action(transportAuthID string, action git2go.Sma return stream, nil } -func createClientRequest(transportAuthID string, action git2go.SmartServiceAction, t *http.Transport) (*http.Client, *http.Request, error) { +func createClientRequest(targetURL string, action git2go.SmartServiceAction, + t *http.Transport, authOpts *git.AuthOptions) (*http.Client, *http.Request, error) { var req *http.Request var err error @@ -150,17 +168,6 @@ func createClientRequest(transportAuthID string, action git2go.SmartServiceActio return nil, nil, fmt.Errorf("failed to create client: transport cannot be nil") } - opts, found := getTransportOptions(transportAuthID) - - if !found { - return nil, nil, fmt.Errorf("failed to create client: could not find transport options for the object: %s", transportAuthID) - } - targetURL := opts.TargetURL - - if len(targetURL) > URLMaxLength { - return nil, nil, fmt.Errorf("URL exceeds the max length (%d)", URLMaxLength) - } - client := &http.Client{ Transport: t, Timeout: fullHttpClientTimeOut, @@ -176,6 +183,9 @@ func createClientRequest(transportAuthID string, action git2go.SmartServiceActio break } req.Header.Set("Content-Type", "application/x-git-upload-pack-request") + if t.Proxy != nil { + t.ProxyConnectHeader.Set("Content-Type", "application/x-git-upload-pack-request") + } case git2go.SmartServiceActionReceivepackLs: req, err = http.NewRequest("GET", targetURL+"/info/refs?service=git-receive-pack", nil) @@ -186,6 +196,9 @@ func createClientRequest(transportAuthID string, action git2go.SmartServiceActio break } req.Header.Set("Content-Type", "application/x-git-receive-pack-request") + if t.Proxy != nil { + t.ProxyConnectHeader.Set("Content-Type", "application/x-git-receive-pack-request") + } default: err = errors.New("unknown action") @@ -195,12 +208,20 @@ func createClientRequest(transportAuthID string, action git2go.SmartServiceActio return nil, nil, err } - // Add any provided certificate to the http transport. - if opts.AuthOpts != nil { - req.SetBasicAuth(opts.AuthOpts.Username, opts.AuthOpts.Password) - if len(opts.AuthOpts.CAFile) > 0 { + // Apply authentication and TLS settings to the HTTP transport. + if authOpts != nil { + if len(authOpts.Username) > 0 { + req.SetBasicAuth(authOpts.Username, authOpts.Password) + if t.Proxy != nil { + t.ProxyConnectHeader.Set( + "Authorization", + "Basic "+basicAuth(authOpts.Username, authOpts.Password), + ) + } + } + if len(authOpts.CAFile) > 0 { certPool := x509.NewCertPool() - if ok := certPool.AppendCertsFromPEM(opts.AuthOpts.CAFile); !ok { + if ok := certPool.AppendCertsFromPEM(authOpts.CAFile); !ok { return nil, nil, fmt.Errorf("failed to use certificate from PEM") } t.TLSClientConfig = &tls.Config{ @@ -210,6 +231,9 @@ func createClientRequest(transportAuthID string, action git2go.SmartServiceActio } req.Header.Set("User-Agent", "git/2.0 (flux-libgit2)") + if t.Proxy != nil { + t.ProxyConnectHeader.Set("User-Agent", "git/2.0 (flux-libgit2)") + } return client, req, nil } @@ -389,3 +413,9 @@ func (self *httpSmartSubtransportStream) sendRequest() error { self.sentRequest = true return nil } + +// From: https://github.com/golang/go/blob/go1.18/src/net/http/client.go#L418 +func basicAuth(username, password string) string { + auth := username + ":" + password + return base64.StdEncoding.EncodeToString([]byte(auth)) +} diff --git a/pkg/git/libgit2/managed/http_test.go b/pkg/git/libgit2/managed/http_test.go index bf54de59..32b2137a 100644 --- a/pkg/git/libgit2/managed/http_test.go +++ b/pkg/git/libgit2/managed/http_test.go @@ -32,76 +32,88 @@ import ( ) func TestHttpAction_CreateClientRequest(t *testing.T) { - opts := &TransportOptions{ - TargetURL: "https://final-target/abc", + authOpts := git.AuthOptions{ + Username: "user", + Password: "pwd", } - - optsWithAuth := &TransportOptions{ - TargetURL: "https://final-target/abc", - AuthOpts: &git.AuthOptions{ - Username: "user", - Password: "pwd", - }, - } - id := "https://obj-id" + url := "https://final-target/abc" tests := []struct { name string assertFunc func(g *WithT, req *http.Request, client *http.Client) action git2go.SmartServiceAction - opts *TransportOptions + authOpts git.AuthOptions transport *http.Transport wantedErr error }{ { - name: "Uploadpack: URL and method are correctly set", - action: git2go.SmartServiceActionUploadpack, - transport: &http.Transport{}, + name: "Uploadpack: URL, method and headers are correctly set", + action: git2go.SmartServiceActionUploadpack, + transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + ProxyConnectHeader: map[string][]string{}, + }, assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { g.Expect(req.URL.String()).To(Equal("https://final-target/abc/git-upload-pack")) g.Expect(req.Method).To(Equal("POST")) + g.Expect(req.Header).To(BeEquivalentTo(map[string][]string{ + "User-Agent": []string{"git/2.0 (flux-libgit2)"}, + "Content-Type": []string{"application/x-git-upload-pack-request"}, + })) }, - opts: opts, wantedErr: nil, }, { - name: "UploadpackLs: URL and method are correctly set", + name: "UploadpackLs: URL, method and headers are correctly set", action: git2go.SmartServiceActionUploadpackLs, transport: &http.Transport{}, assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { g.Expect(req.URL.String()).To(Equal("https://final-target/abc/info/refs?service=git-upload-pack")) g.Expect(req.Method).To(Equal("GET")) + g.Expect(req.Header).To(BeEquivalentTo(map[string][]string{ + "User-Agent": []string{"git/2.0 (flux-libgit2)"}, + })) }, - opts: opts, wantedErr: nil, }, { - name: "Receivepack: URL and method are correctly set", - action: git2go.SmartServiceActionReceivepack, - transport: &http.Transport{}, + name: "Receivepack: URL, method and headers are correctly set", + action: git2go.SmartServiceActionReceivepack, + transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + ProxyConnectHeader: map[string][]string{}, + }, assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { g.Expect(req.URL.String()).To(Equal("https://final-target/abc/git-receive-pack")) g.Expect(req.Method).To(Equal("POST")) + g.Expect(req.Header).To(BeEquivalentTo(map[string][]string{ + "Content-Type": []string{"application/x-git-receive-pack-request"}, + "User-Agent": []string{"git/2.0 (flux-libgit2)"}, + })) }, - opts: opts, wantedErr: nil, }, { - name: "ReceivepackLs: URL and method are correctly set", + name: "ReceivepackLs: URL, method and headars are correctly set", action: git2go.SmartServiceActionReceivepackLs, transport: &http.Transport{}, assertFunc: func(g *WithT, req *http.Request, _ *http.Client) { g.Expect(req.URL.String()).To(Equal("https://final-target/abc/info/refs?service=git-receive-pack")) g.Expect(req.Method).To(Equal("GET")) + g.Expect(req.Header).To(BeEquivalentTo(map[string][]string{ + "User-Agent": []string{"git/2.0 (flux-libgit2)"}, + })) }, - opts: opts, wantedErr: nil, }, { - name: "credentials are correctly configured", - action: git2go.SmartServiceActionUploadpack, - transport: &http.Transport{}, - opts: optsWithAuth, + name: "credentials are correctly configured", + action: git2go.SmartServiceActionUploadpack, + transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + ProxyConnectHeader: map[string][]string{}, + }, + authOpts: authOpts, assertFunc: func(g *WithT, req *http.Request, client *http.Client) { g.Expect(req.URL.String()).To(Equal("https://final-target/abc/git-upload-pack")) g.Expect(req.Method).To(Equal("POST")) @@ -119,26 +131,15 @@ func TestHttpAction_CreateClientRequest(t *testing.T) { name: "error when no http.transport provided", action: git2go.SmartServiceActionUploadpack, transport: nil, - opts: opts, wantedErr: fmt.Errorf("failed to create client: transport cannot be nil"), }, - { - name: "error when no transport options are registered", - action: git2go.SmartServiceActionUploadpack, - transport: &http.Transport{}, - opts: nil, - wantedErr: fmt.Errorf("failed to create client: could not find transport options for the object: https://obj-id"), - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - if tt.opts != nil { - AddTransportOptions(id, *tt.opts) - } - client, req, err := createClientRequest(id, tt.action, tt.transport) + client, req, err := createClientRequest(url, tt.action, tt.transport, &tt.authOpts) if err != nil { t.Log(err) } @@ -148,9 +149,6 @@ func TestHttpAction_CreateClientRequest(t *testing.T) { tt.assertFunc(g, req, client) } - if tt.opts != nil { - RemoveTransportOptions(id) - } }) } } @@ -167,18 +165,10 @@ func TestHTTPManagedTransport_E2E(t *testing.T) { server.Auth(user, pwd) server.KeyDir(filepath.Join(server.Root(), "keys")) - err = server.ListenSSH() - g.Expect(err).ToNot(HaveOccurred()) - err = server.StartHTTP() g.Expect(err).ToNot(HaveOccurred()) defer server.StopHTTP() - go func() { - server.StartSSH() - }() - defer server.StopSSH() - // Force managed transport to be enabled InitManagedTransport(logr.Discard()) @@ -188,7 +178,7 @@ func TestHTTPManagedTransport_E2E(t *testing.T) { tmpDir := t.TempDir() - // Register the auth options and target url mapped to a unique id. + // Register the auth options and target url mapped to a unique url. id := "http://obj-id" AddTransportOptions(id, TransportOptions{ TargetURL: server.HTTPAddress() + "/" + repoPath, @@ -198,9 +188,9 @@ func TestHTTPManagedTransport_E2E(t *testing.T) { }, }) - // We call Clone with id instead of the actual url, as the transport action - // will fetch the actual url and the required credentials using the id as - // a identifier. + // We call git2go.Clone with transportOptsURL instead of the actual URL, + // as the transport action will fetch the actual URL and the required + // credentials using the it as an identifier. repo, err := git2go.Clone(id, tmpDir, &git2go.CloneOptions{ CheckoutOptions: git2go.CheckoutOptions{ Strategy: git2go.CheckoutForce, diff --git a/pkg/git/libgit2/managed/managed_test.go b/pkg/git/libgit2/managed/managed_test.go deleted file mode 100644 index beda7fc2..00000000 --- a/pkg/git/libgit2/managed/managed_test.go +++ /dev/null @@ -1,48 +0,0 @@ -/* -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. -*/ - -package managed - -import ( - "os" - "testing" -) - -func TestFlagStatus(t *testing.T) { - if Enabled() { - t.Errorf("experimental transport should not be enabled by default") - } - - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") - if !Enabled() { - t.Errorf("experimental transport should be enabled when env EXPERIMENTAL_GIT_TRANSPORT=true") - } - - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "1") - if !Enabled() { - t.Errorf("experimental transport should be enabled when env EXPERIMENTAL_GIT_TRANSPORT=1") - } - - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "somethingelse") - if Enabled() { - t.Errorf("experimental transport should be enabled only when env EXPERIMENTAL_GIT_TRANSPORT is 1 or true but was enabled for 'somethingelse'") - } - - os.Unsetenv("EXPERIMENTAL_GIT_TRANSPORT") - if Enabled() { - t.Errorf("experimental transport should not be enabled when env EXPERIMENTAL_GIT_TRANSPORT is not present") - } -} diff --git a/pkg/git/libgit2/managed/options.go b/pkg/git/libgit2/managed/options.go index 58a04da7..900d593c 100644 --- a/pkg/git/libgit2/managed/options.go +++ b/pkg/git/libgit2/managed/options.go @@ -20,36 +20,45 @@ import ( "sync" "github.com/fluxcd/source-controller/pkg/git" + git2go "github.com/libgit2/git2go/v33" ) // TransportOptions represents options to be applied at transport-level // at request time. type TransportOptions struct { - TargetURL string - AuthOpts *git.AuthOptions + TargetURL string + AuthOpts *git.AuthOptions + ProxyOptions *git2go.ProxyOptions } var ( - // transportOpts maps a unique id to a set of transport options. + // transportOpts maps a unique url to a set of transport options. transportOpts = make(map[string]TransportOptions, 0) m sync.RWMutex ) -func AddTransportOptions(id string, opts TransportOptions) { +// AddTransportOptions registers a TransportOptions object mapped to the +// provided transportOptsURL, which must be a valid URL, i.e. prefixed with "http://" +// or "ssh://", as it is used as a dummy URL for all git operations and the managed +// transports will only be invoked for the protocols that they have been +// registered for. +func AddTransportOptions(transportOptsURL string, opts TransportOptions) { m.Lock() - transportOpts[id] = opts + transportOpts[transportOptsURL] = opts m.Unlock() } -func RemoveTransportOptions(id string) { +// RemoveTransportOptions removes the registerd TransportOptions object +// mapped to the provided id. +func RemoveTransportOptions(transportOptsURL string) { m.Lock() - delete(transportOpts, id) + delete(transportOpts, transportOptsURL) m.Unlock() } -func getTransportOptions(id string) (*TransportOptions, bool) { +func getTransportOptions(transportOptsURL string) (*TransportOptions, bool) { m.RLock() - opts, found := transportOpts[id] + opts, found := transportOpts[transportOptsURL] m.RUnlock() if found { @@ -63,16 +72,16 @@ func getTransportOptions(id string) (*TransportOptions, bool) { // Given that TransportOptions can allow for the target URL to be overriden // this returns the same input if Managed Transport is disabled or if no TargetURL // is set on TransportOptions. -func EffectiveURL(id string) string { +func EffectiveURL(transporOptsURL string) string { if !Enabled() { - return id + return transporOptsURL } - if opts, found := getTransportOptions(id); found { + if opts, found := getTransportOptions(transporOptsURL); found { if opts.TargetURL != "" { return opts.TargetURL } } - return id + return transporOptsURL } diff --git a/pkg/git/libgit2/managed/ssh.go b/pkg/git/libgit2/managed/ssh.go index 3895fbe4..dddcadc0 100644 --- a/pkg/git/libgit2/managed/ssh.go +++ b/pkg/git/libgit2/managed/ssh.go @@ -45,8 +45,6 @@ package managed import ( "context" - "crypto/md5" - "crypto/sha1" "crypto/sha256" "fmt" "io" @@ -96,13 +94,13 @@ type sshSmartSubtransport struct { connected bool } -func (t *sshSmartSubtransport) Action(credentialsID string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { +func (t *sshSmartSubtransport) Action(transportOptionsURL string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) { runtime.LockOSThread() defer runtime.UnlockOSThread() - opts, found := getTransportOptions(credentialsID) + opts, found := getTransportOptions(transportOptionsURL) if !found { - return nil, fmt.Errorf("could not find transport options for object: %s", credentialsID) + return nil, fmt.Errorf("could not find transport options for object: %s", transportOptionsURL) } u, err := url.Parse(opts.TargetURL) @@ -167,16 +165,17 @@ func (t *sshSmartSubtransport) Action(credentialsID string, action git2go.SmartS cert := &git2go.Certificate{ Kind: git2go.CertificateHostkey, Hostkey: git2go.HostkeyCertificate{ - Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5 | git2go.HostkeySHA256 | git2go.HostkeyRaw, - HashMD5: md5.Sum(marshaledKey), - HashSHA1: sha1.Sum(marshaledKey), + Kind: git2go.HostkeySHA256, HashSHA256: sha256.Sum256(marshaledKey), Hostkey: marshaledKey, SSHPublicKey: key, }, } - return t.transport.SmartCertificateCheck(cert, true, hostname) + if len(opts.AuthOpts.KnownHosts) > 0 { + return KnownHostsCallback(hostname, opts.AuthOpts.KnownHosts)(cert, true, hostname) + } + return nil } err = t.createConn(t.addr, sshConfig) diff --git a/pkg/git/libgit2/managed/ssh_test.go b/pkg/git/libgit2/managed/ssh_test.go index 4d5a7b37..81b83f3c 100644 --- a/pkg/git/libgit2/managed/ssh_test.go +++ b/pkg/git/libgit2/managed/ssh_test.go @@ -98,9 +98,9 @@ func TestSSHManagedTransport_E2E(t *testing.T) { err = server.InitRepo("../../testdata/git/repo", git.DefaultBranch, repoPath) g.Expect(err).ToNot(HaveOccurred()) - transportID := "ssh://git@fake-url" + transportOptsURL := "ssh://git@fake-url" sshAddress := server.SSHAddress() + "/" + repoPath - AddTransportOptions(transportID, TransportOptions{ + AddTransportOptions(transportOptsURL, TransportOptions{ TargetURL: sshAddress, AuthOpts: &git.AuthOptions{ Username: "user", @@ -110,10 +110,12 @@ func TestSSHManagedTransport_E2E(t *testing.T) { tmpDir := t.TempDir() - // We call git2go.Clone with transportID, so that the managed ssh transport can + // We call git2go.Clone with transportOptsURL, so that the managed ssh transport can // fetch the correct set of credentials and the actual target url as well. - repo, err := git2go.Clone(transportID, tmpDir, &git2go.CloneOptions{ - FetchOptions: git2go.FetchOptions{}, + repo, err := git2go.Clone(transportOptsURL, tmpDir, &git2go.CloneOptions{ + FetchOptions: git2go.FetchOptions{ + RemoteCallbacks: RemoteCallbacks(), + }, CheckoutOptions: git2go.CheckoutOptions{ Strategy: git2go.CheckoutForce, }, diff --git a/pkg/git/libgit2/managed/transport.go b/pkg/git/libgit2/managed/transport.go new file mode 100644 index 00000000..763e6f8b --- /dev/null +++ b/pkg/git/libgit2/managed/transport.go @@ -0,0 +1,103 @@ +package managed + +import ( + "crypto/md5" + "crypto/sha1" + "crypto/sha256" + "fmt" + "hash" + "net" + + pkgkh "github.com/fluxcd/pkg/ssh/knownhosts" + git2go "github.com/libgit2/git2go/v33" + "golang.org/x/crypto/ssh/knownhosts" +) + +// knownHostCallback returns a CertificateCheckCallback that verifies +// the key of Git server against the given host and known_hosts for +// git.SSH Transports. +func KnownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckCallback { + return func(cert *git2go.Certificate, valid bool, hostname string) error { + kh, err := pkgkh.ParseKnownHosts(string(knownHosts)) + if err != nil { + return fmt.Errorf("failed to parse known_hosts: %w", err) + } + + // First, attempt to split the configured host and port to validate + // the port-less hostname given to the callback. + hostWithoutPort, _, err := net.SplitHostPort(host) + if err != nil { + // SplitHostPort returns an error if the host is missing + // a port, assume the host has no port. + hostWithoutPort = host + } + + // Different versions of libgit handle this differently. + // This fixes the case in which ports may be sent back. + hostnameWithoutPort, _, err := net.SplitHostPort(hostname) + if err != nil { + hostnameWithoutPort = hostname + } + + if hostnameWithoutPort != hostWithoutPort { + return fmt.Errorf("host mismatch: %q %q", hostWithoutPort, hostnameWithoutPort) + } + + var fingerprint []byte + var hasher hash.Hash + switch { + case cert.Hostkey.Kind&git2go.HostkeySHA256 > 0: + fingerprint = cert.Hostkey.HashSHA256[:] + hasher = sha256.New() + // SHA1 and MD5 are present here, because they're used for unmanaged transport. + // TODO: get rid of this, when unmanaged transport is completely removed. + case cert.Hostkey.Kind&git2go.HostkeySHA1 > 0: + fingerprint = cert.Hostkey.HashSHA1[:] + hasher = sha1.New() + case cert.Hostkey.Kind&git2go.HostkeyMD5 > 0: + fingerprint = cert.Hostkey.HashMD5[:] + hasher = md5.New() + default: + return fmt.Errorf("invalid host key kind, expected to be one of SHA256, SHA1, MD5") + } + + // We are now certain that the configured host and the hostname + // given to the callback match. Use the configured host (that + // includes the port), and normalize it, so we can check if there + // is an entry for the hostname _and_ port. + h := knownhosts.Normalize(host) + for _, k := range kh { + if k.Matches(h, fingerprint, hasher) { + return nil + } + } + return fmt.Errorf("hostkey could not be verified") + } +} + +// RemoteCallbacks constructs git2go.RemoteCallbacks with dummy callbacks. +func RemoteCallbacks() git2go.RemoteCallbacks { + // This may not be fully removed as without some of the callbacks git2go + // gets anxious and panics. + return git2go.RemoteCallbacks{ + CredentialsCallback: credentialsCallback(), + CertificateCheckCallback: certificateCallback(), + } +} + +// credentialsCallback constructs a dummy CredentialsCallback. +func credentialsCallback() git2go.CredentialsCallback { + return func(url string, username string, allowedTypes git2go.CredentialType) (*git2go.Credential, error) { + // If credential is nil, panic will ensue. We fake it as managed transport does not + // require it. + return git2go.NewCredentialUserpassPlaintext("", "") + } +} + +// certificateCallback constructs a dummy CertificateCallback. +func certificateCallback() git2go.CertificateCheckCallback { + // returning a nil func can cause git2go to panic. + return func(cert *git2go.Certificate, valid bool, hostname string) error { + return nil + } +} diff --git a/pkg/git/libgit2/managed/transport_test.go b/pkg/git/libgit2/managed/transport_test.go new file mode 100644 index 00000000..2428d599 --- /dev/null +++ b/pkg/git/libgit2/managed/transport_test.go @@ -0,0 +1,108 @@ +package managed + +import ( + "crypto/x509" + "encoding/base64" + "encoding/pem" + "errors" + "fmt" + "testing" + + git2go "github.com/libgit2/git2go/v33" + . "github.com/onsi/gomega" +) + +// knownHostsFixture is known_hosts fixture in the expected +// format. +var knownHostsFixture = `github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==` + +func TestKnownHostsCallback(t *testing.T) { + tests := []struct { + name string + host string + expectedHost string + knownHosts []byte + hostkey git2go.HostkeyCertificate + want error + }{ + { + name: "Match", + host: "github.com", + knownHosts: []byte(knownHostsFixture), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, + expectedHost: "github.com", + want: nil, + }, + { + name: "Match with port", + host: "github.com", + knownHosts: []byte(knownHostsFixture), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, + expectedHost: "github.com:22", + want: nil, + }, + { + name: "Hostname mismatch", + host: "github.com", + knownHosts: []byte(knownHostsFixture), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, + expectedHost: "example.com", + want: fmt.Errorf("host mismatch: %q %q", "example.com", "github.com"), + }, + { + name: "Hostkey mismatch", + host: "github.com", + knownHosts: []byte(knownHostsFixture), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\xb6\x03\x0e\x39\x97\x9e\xd0\xe7\x24\xce\xa3\x77\x3e\x01\x42\x09")}, + expectedHost: "github.com", + want: fmt.Errorf("hostkey could not be verified"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + cert := &git2go.Certificate{Hostkey: tt.hostkey} + callback := KnownHostsCallback(tt.expectedHost, tt.knownHosts) + result := g.Expect(callback(cert, false, tt.host)) + if tt.want == nil { + result.To(BeNil()) + } else { + result.To(Equal(tt.want)) + } + }) + } +} +func md5Fingerprint(in string) [16]byte { + var out [16]byte + copy(out[:], in) + return out +} + +func sha1Fingerprint(in string) [20]byte { + d, err := base64.RawStdEncoding.DecodeString(in) + if err != nil { + panic(err) + } + var out [20]byte + copy(out[:], d) + return out +} + +func sha256Fingerprint(in string) [32]byte { + d, err := base64.RawStdEncoding.DecodeString(in) + if err != nil { + panic(err) + } + var out [32]byte + copy(out[:], d) + return out +} + +func certificateFromPEM(pemBytes string) (*x509.Certificate, error) { + block, _ := pem.Decode([]byte(pemBytes)) + if block == nil { + return nil, errors.New("failed to decode PEM") + } + return x509.ParseCertificate(block.Bytes) +} diff --git a/pkg/git/libgit2/managed_test.go b/pkg/git/libgit2/managed_test.go index 728c61fe..8cb07016 100644 --- a/pkg/git/libgit2/managed_test.go +++ b/pkg/git/libgit2/managed_test.go @@ -36,6 +36,7 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/gomega" + git2go "github.com/libgit2/git2go/v33" cryptossh "golang.org/x/crypto/ssh" ) @@ -123,7 +124,6 @@ func Test_ManagedSSH_KeyTypes(t *testing.T) { knownHosts, err := ssh.ScanHostKey(u.Host, timeout, git.HostKeyAlgos, false) g.Expect(err).ToNot(HaveOccurred()) - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") managed.InitManagedTransport(logr.Discard()) for _, tt := range tests { @@ -139,21 +139,11 @@ func Test_ManagedSSH_KeyTypes(t *testing.T) { authorizedPublicKey = string(kp.PublicKey) } - // secret := corev1.Secret{ - // Data: map[string][]byte{ - // "identity": kp.PrivateKey, - // "known_hosts": knownHosts, - // }, - // } - // - // authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) - // g.Expect(err).ToNot(HaveOccurred()) - authOpts := &git.AuthOptions{ Identity: kp.PrivateKey, KnownHosts: knownHosts, } - authOpts.TransportAuthID = "ssh://" + getTransportAuthID() + authOpts.TransportOptionsURL = getTransportOptionsURL(git.SSH) // Prepare for checkout. branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch} @@ -233,7 +223,6 @@ func Test_ManagedSSH_KeyExchangeAlgos(t *testing.T) { }, } - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") managed.InitManagedTransport(logr.Discard()) for _, tt := range tests { @@ -280,20 +269,11 @@ func Test_ManagedSSH_KeyExchangeAlgos(t *testing.T) { kp, err := ssh.GenerateKeyPair(ssh.ED25519) g.Expect(err).ToNot(HaveOccurred()) - // secret := corev1.Secret{ - // Data: map[string][]byte{ - // "identity": kp.PrivateKey, - // "known_hosts": knownHosts, - // }, - // } - // - // authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) - // g.Expect(err).ToNot(HaveOccurred()) authOpts := &git.AuthOptions{ Identity: kp.PrivateKey, KnownHosts: knownHosts, } - authOpts.TransportAuthID = "ssh://" + getTransportAuthID() + authOpts.TransportOptionsURL = getTransportOptionsURL(git.SSH) // Prepare for checkout. branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch} @@ -402,7 +382,6 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { }, } - os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true") managed.InitManagedTransport(logr.Discard()) for _, tt := range tests { @@ -459,20 +438,11 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { kp, err := ssh.GenerateKeyPair(ssh.ED25519) g.Expect(err).ToNot(HaveOccurred()) - // secret := corev1.Secret{ - // Data: map[string][]byte{ - // "identity": kp.PrivateKey, - // "known_hosts": knownHosts, - // }, - // } - // - // authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret) - // g.Expect(err).ToNot(HaveOccurred()) authOpts := &git.AuthOptions{ Identity: kp.PrivateKey, KnownHosts: knownHosts, } - authOpts.TransportAuthID = "ssh://" + getTransportAuthID() + authOpts.TransportOptionsURL = getTransportOptionsURL(git.SSH) // Prepare for checkout. branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch} @@ -488,11 +458,161 @@ func Test_ManagedSSH_HostKeyAlgos(t *testing.T) { } } -func getTransportAuthID() string { +func Test_ManagedHTTPCheckout(t *testing.T) { + 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() + + // Force managed transport to be enabled + managed.InitManagedTransport(logr.Discard()) + + 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()) + + 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()) + + 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)) +} + +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()) + + 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()) + _, 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) for i := range b { b[i] = letterRunes[rand.Intn(len(letterRunes))] } - return string(b) + return string(transport) + "://" + string(b) } diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index e7c9671c..f9aeefe2 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -17,27 +17,16 @@ limitations under the License. package libgit2 import ( - "bufio" - "bytes" "context" - "crypto/hmac" - "crypto/md5" - "crypto/sha1" - "crypto/sha256" "crypto/x509" - "encoding/base64" "fmt" - "hash" - "io" - "net" - "strings" "time" git2go "github.com/libgit2/git2go/v33" "golang.org/x/crypto/ssh" - "golang.org/x/crypto/ssh/knownhosts" "github.com/fluxcd/source-controller/pkg/git" + "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" ) var ( @@ -148,7 +137,7 @@ func certificateCallback(opts *git.AuthOptions) git2go.CertificateCheckCallback } case git.SSH: if len(opts.KnownHosts) > 0 && opts.Host != "" { - return knownHostsCallback(opts.Host, opts.KnownHosts) + return managed.KnownHostsCallback(opts.Host, opts.KnownHosts) } } return nil @@ -174,157 +163,3 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { return nil } } - -// knownHostCallback returns a CertificateCheckCallback that verifies -// the key of Git server against the given host and known_hosts for -// git.SSH Transports. -func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckCallback { - return func(cert *git2go.Certificate, valid bool, hostname string) error { - kh, err := parseKnownHosts(string(knownHosts)) - if err != nil { - return fmt.Errorf("failed to parse known_hosts: %w", err) - } - - // First, attempt to split the configured host and port to validate - // the port-less hostname given to the callback. - hostWithoutPort, _, err := net.SplitHostPort(host) - if err != nil { - // SplitHostPort returns an error if the host is missing - // a port, assume the host has no port. - hostWithoutPort = host - } - - // Different versions of libgit handle this differently. - // This fixes the case in which ports may be sent back. - hostnameWithoutPort, _, err := net.SplitHostPort(hostname) - if err != nil { - hostnameWithoutPort = hostname - } - - if hostnameWithoutPort != hostWithoutPort { - return fmt.Errorf("host mismatch: %q %q", hostWithoutPort, hostnameWithoutPort) - } - - // We are now certain that the configured host and the hostname - // given to the callback match. Use the configured host (that - // includes the port), and normalize it, so we can check if there - // is an entry for the hostname _and_ port. - h := knownhosts.Normalize(host) - for _, k := range kh { - if k.matches(h, cert.Hostkey) { - return nil - } - } - return fmt.Errorf("hostkey could not be verified") - } -} - -type knownKey struct { - hosts []string - key ssh.PublicKey -} - -func parseKnownHosts(s string) ([]knownKey, error) { - var knownHosts []knownKey - scanner := bufio.NewScanner(strings.NewReader(s)) - for scanner.Scan() { - _, hosts, pubKey, _, _, err := ssh.ParseKnownHosts(scanner.Bytes()) - if err != nil { - // Lines that aren't host public key result in EOF, like a comment - // line. Continue parsing the other lines. - if err == io.EOF { - continue - } - return []knownKey{}, err - } - - knownHost := knownKey{ - hosts: hosts, - key: pubKey, - } - knownHosts = append(knownHosts, knownHost) - } - - if err := scanner.Err(); err != nil { - return []knownKey{}, err - } - - return knownHosts, nil -} - -func (k knownKey) matches(host string, hostkey git2go.HostkeyCertificate) bool { - if !containsHost(k.hosts, host) { - return false - } - - var fingerprint []byte - var hasher hash.Hash - switch { - case hostkey.Kind&git2go.HostkeySHA256 > 0: - fingerprint = hostkey.HashSHA256[:] - hasher = sha256.New() - case hostkey.Kind&git2go.HostkeySHA1 > 0: - fingerprint = hostkey.HashSHA1[:] - hasher = sha1.New() - case hostkey.Kind&git2go.HostkeyMD5 > 0: - fingerprint = hostkey.HashMD5[:] - hasher = md5.New() - default: - return false - } - hasher.Write(k.key.Marshal()) - return bytes.Equal(hasher.Sum(nil), fingerprint) -} - -func containsHost(hosts []string, host string) bool { - for _, kh := range hosts { - // hashed host must start with a pipe - if kh[0] == '|' { - match, _ := MatchHashedHost(kh, host) - if match { - return true - } - - } else if kh == host { // unhashed host check - return true - } - } - return false -} - -// MatchHashedHost tries to match a hashed known host (kh) to -// host. -// -// Note that host is not hashed, but it is rather hashed during -// the matching process using the same salt used when hashing -// the known host. -func MatchHashedHost(kh, host string) (bool, error) { - if kh == "" || kh[0] != '|' { - return false, fmt.Errorf("hashed known host must begin with '|': '%s'", kh) - } - - components := strings.Split(kh, "|") - if len(components) != 4 { - return false, fmt.Errorf("invalid format for hashed known host: '%s'", kh) - } - - if components[1] != "1" { - return false, fmt.Errorf("unsupported hash type '%s'", components[1]) - } - - hkSalt, err := base64.StdEncoding.DecodeString(components[2]) - if err != nil { - return false, fmt.Errorf("cannot decode hashed known host: '%w'", err) - } - - hkHash, err := base64.StdEncoding.DecodeString(components[3]) - if err != nil { - return false, fmt.Errorf("cannot decode hashed known host: '%w'", err) - } - - mac := hmac.New(sha1.New, hkSalt) - mac.Write([]byte(host)) - hostHash := mac.Sum(nil) - - return bytes.Equal(hostHash, hkHash), nil -} diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index f645807f..2e0c57d1 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "crypto/x509" - "encoding/base64" "encoding/pem" "errors" "fmt" @@ -205,159 +204,6 @@ func Test_x509Callback(t *testing.T) { } } -func Test_knownHostsCallback(t *testing.T) { - tests := []struct { - name string - host string - expectedHost string - knownHosts []byte - hostkey git2go.HostkeyCertificate - want error - }{ - { - name: "Match", - host: "github.com", - knownHosts: []byte(knownHostsFixture), - hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, - expectedHost: "github.com", - want: nil, - }, - { - name: "Match with port", - host: "github.com", - knownHosts: []byte(knownHostsFixture), - hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, - expectedHost: "github.com:22", - want: nil, - }, - { - name: "Hostname mismatch", - host: "github.com", - knownHosts: []byte(knownHostsFixture), - hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, - expectedHost: "example.com", - want: fmt.Errorf("host mismatch: %q %q", "example.com", "github.com"), - }, - { - name: "Hostkey mismatch", - host: "github.com", - knownHosts: []byte(knownHostsFixture), - hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\xb6\x03\x0e\x39\x97\x9e\xd0\xe7\x24\xce\xa3\x77\x3e\x01\x42\x09")}, - expectedHost: "github.com", - want: fmt.Errorf("hostkey could not be verified"), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - cert := &git2go.Certificate{Hostkey: tt.hostkey} - callback := knownHostsCallback(tt.expectedHost, tt.knownHosts) - result := g.Expect(callback(cert, false, tt.host)) - if tt.want == nil { - result.To(BeNil()) - } else { - result.To(Equal(tt.want)) - } - }) - } -} - -func Test_parseKnownHosts_matches(t *testing.T) { - tests := []struct { - name string - hostkey git2go.HostkeyCertificate - wantMatches bool - }{ - {"good sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256 | git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA256: sha256Fingerprint("nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8")}, true}, - {"bad sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256 | git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA256: sha256Fingerprint("ROQFvPThGrW4RuWLoL9tq9I9zJ42fK4XywyRtbOz/EQ")}, false}, - {"good sha1 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, true}, - {"bad sha1 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("tfpLlQhDDFP3yGdewTvHNxWmAdk")}, false}, - {"good md5 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\x16\x27\xac\xa5\x76\x28\x2d\x36\x63\x1b\x56\x4d\xeb\xdf\xa6\x48")}, true}, - {"bad md5 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\xb6\x03\x0e\x39\x97\x9e\xd0\xe7\x24\xce\xa3\x77\x3e\x01\x42\x09")}, false}, - {"invalid hostkey", git2go.HostkeyCertificate{}, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - knownKeys, err := parseKnownHosts(knownHostsFixture) - if err != nil { - t.Error(err) - return - } - matches := knownKeys[0].matches("github.com", tt.hostkey) - g.Expect(matches).To(Equal(tt.wantMatches)) - }) - } -} - -func Test_parseKnownHosts(t *testing.T) { - tests := []struct { - name string - fixture string - wantErr bool - }{ - { - name: "empty file", - fixture: "", - wantErr: false, - }, - { - name: "single host", - fixture: `github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==`, - wantErr: false, - }, - { - name: "single host with comment", - fixture: `# github.com -github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==`, - wantErr: false, - }, - { - name: "multiple hosts with comments", - fixture: `# github.com -github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ== -# gitlab.com -gitlab.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjquxvt6CM6tdG4SLp1Btn/nOeHHE5UOzRdf`, - }, - { - name: "no host key, only comments", - fixture: `# example.com -#github.com -# gitlab.com`, - wantErr: false, - }, - { - name: "invalid host entry", - fixture: `github.com ssh-rsa`, - wantErr: true, - }, - { - name: "invalid content", - fixture: `some random text`, - wantErr: true, - }, - { - name: "invalid line with valid host key", - fixture: `some random text -gitlab.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjquxvt6CM6tdG4SLp1Btn/nOeHHE5UOzRdf`, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - _, err := parseKnownHosts(tt.fixture) - if tt.wantErr { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).ToNot(HaveOccurred()) - } - }) - } -} - func Test_transferProgressCallback(t *testing.T) { tests := []struct { name string @@ -522,94 +368,6 @@ func Test_pushTransferProgressCallback(t *testing.T) { } } -func TestMatchHashedHost(t *testing.T) { - tests := []struct { - name string - knownHost string - host string - match bool - wantErr string - }{ - { - name: "match valid known host", - knownHost: "|1|vApZG0Ybr4rHfTb69+cjjFIGIv0=|M5sSXen14encOvQAy0gseRahnJw=", - host: "[127.0.0.1]:44167", - match: true, - }, - { - name: "empty known host errors", - wantErr: "hashed known host must begin with '|'", - }, - { - name: "unhashed known host errors", - knownHost: "[127.0.0.1]:44167", - wantErr: "hashed known host must begin with '|'", - }, - { - name: "invalid known host format errors", - knownHost: "|1M5sSXen14encOvQAy0gseRahnJw=", - wantErr: "invalid format for hashed known host", - }, - { - name: "invalid hash type errors", - knownHost: "|2|vApZG0Ybr4rHfTb69+cjjFIGIv0=|M5sSXen14encOvQAy0gseRahnJw=", - wantErr: "unsupported hash type", - }, - { - name: "invalid base64 component[2] errors", - knownHost: "|1|azz|M5sSXen14encOvQAy0gseRahnJw=", - wantErr: "cannot decode hashed known host", - }, - { - name: "invalid base64 component[3] errors", - knownHost: "|1|M5sSXen14encOvQAy0gseRahnJw=|azz", - wantErr: "cannot decode hashed known host", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - matched, err := MatchHashedHost(tt.knownHost, tt.host) - - if tt.wantErr == "" { - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(matched).To(Equal(tt.match)) - } else { - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) - } - }) - } -} - -func md5Fingerprint(in string) [16]byte { - var out [16]byte - copy(out[:], in) - return out -} - -func sha1Fingerprint(in string) [20]byte { - d, err := base64.RawStdEncoding.DecodeString(in) - if err != nil { - panic(err) - } - var out [20]byte - copy(out[:], d) - return out -} - -func sha256Fingerprint(in string) [32]byte { - d, err := base64.RawStdEncoding.DecodeString(in) - if err != nil { - panic(err) - } - var out [32]byte - copy(out[:], d) - return out -} - func certificateFromPEM(pemBytes string) (*x509.Certificate, error) { block, _ := pem.Decode([]byte(pemBytes)) if block == nil { diff --git a/pkg/git/options.go b/pkg/git/options.go index 81bbd6ce..a9169a59 100644 --- a/pkg/git/options.go +++ b/pkg/git/options.go @@ -72,11 +72,16 @@ type AuthOptions struct { Identity []byte KnownHosts []byte CAFile []byte - // TransportAuthID is a unique identifier for this set of authentication + // TransportOptionsURL is a unique identifier for this set of authentication // options. It's used by managed libgit2 transports to uniquely identify - // which credentials to use for a particular git operation, and avoid misuse - // of credentials in a multi tenant environment. - TransportAuthID string + // which credentials to use for a particular Git operation, and avoid misuse + // of credentials in a multi-tenant environment. + // It must be prefixed with a valid transport protocol ("ssh:// "or "http://") because + // of the way managed transports are registered and invoked. + // It's a field of AuthOptions despite not providing any kind of authentication + // info, as it's the only way to sneak it into git.Checkout, without polluting + // it's args and keeping it generic. + TransportOptionsURL string } // KexAlgos hosts the key exchange algorithms to be used for SSH connections.