Skip to content

Commit 116aca7

Browse files
committed
separate managed and unmanaged code
Signed-off-by: Sanskar Jaiswal <[email protected]>
1 parent 54e69d7 commit 116aca7

15 files changed

+285
-519
lines changed

controllers/gitrepository_controller.go

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -455,26 +455,8 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
455455
return sreconcile.ResultEmpty, e
456456
}
457457

458-
repositoryURL := obj.Spec.URL
459-
// managed GIT transport only affects the libgit2 implementation
460-
if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation {
461-
// We set the TransportAuthID of this set of authentication options here by constructing
462-
// a unique ID that won't clash in a multi tenant environment. This unique ID is used by
463-
// libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in
464-
// libgit2, which is inflexible and unstable.
465-
if strings.HasPrefix(repositoryURL, "http") {
466-
authOpts.TransportAuthID = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
467-
}
468-
if strings.HasPrefix(repositoryURL, "ssh") {
469-
authOpts.TransportAuthID = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
470-
}
471-
}
472-
473458
// Fetch the included artifact metadata.
474459
artifacts, err := r.fetchIncludes(ctx, obj)
475-
if err != nil {
476-
return sreconcile.ResultEmpty, err
477-
}
478460

479461
// Observe if the artifacts still match the previous included ones
480462
if artifacts.Diff(obj.Status.IncludedArtifacts) {
@@ -491,7 +473,27 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
491473
optimizedClone = true
492474
}
493475

494-
c, err := r.gitCheckout(ctx, obj, repositoryURL, authOpts, dir, optimizedClone)
476+
// managed GIT transport only affects the libgit2 implementation
477+
if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation {
478+
// We set the TransportAuthID of this set of authentication options here by constructing
479+
// a unique ID that won't clash in a multi tenant environment. This unique ID is used by
480+
// libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in
481+
// libgit2, which is inflexible and unstable.
482+
if strings.HasPrefix(obj.Spec.URL, "http") {
483+
authOpts.TransportAuthID = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
484+
} else if strings.HasPrefix(obj.Spec.URL, "ssh") {
485+
authOpts.TransportAuthID = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
486+
} else {
487+
e := &serror.Stalling{
488+
Err: fmt.Errorf("git repository URL has invalid transport type: '%s'", obj.Spec.URL),
489+
Reason: sourcev1.GitOperationFailedReason,
490+
}
491+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
492+
return sreconcile.ResultEmpty, e
493+
}
494+
}
495+
496+
c, err := r.gitCheckout(ctx, obj, obj.Spec.URL, authOpts, dir, optimizedClone)
495497
if err != nil {
496498
e := serror.NewGeneric(
497499
fmt.Errorf("failed to checkout and determine revision: %w", err),
@@ -530,7 +532,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
530532

531533
// If we can't skip the reconciliation, checkout again without any
532534
// optimization.
533-
c, err := r.gitCheckout(ctx, obj, repositoryURL, authOpts, dir, false)
535+
c, err := r.gitCheckout(ctx, obj, obj.Spec.URL, authOpts, dir, false)
534536
if err != nil {
535537
e := serror.NewGeneric(
536538
fmt.Errorf("failed to checkout and determine revision: %w", err),

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ require (
2828
github.com/fluxcd/pkg/helmtestserver v0.5.0
2929
github.com/fluxcd/pkg/lockedfile v0.1.0
3030
github.com/fluxcd/pkg/runtime v0.16.0
31-
github.com/fluxcd/pkg/ssh v0.3.4
31+
github.com/fluxcd/pkg/ssh v0.4.0
3232
github.com/fluxcd/pkg/testserver v0.2.0
3333
github.com/fluxcd/pkg/untar v0.1.0
3434
github.com/fluxcd/pkg/version v0.1.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,8 @@ github.com/fluxcd/pkg/lockedfile v0.1.0 h1:YsYFAkd6wawMCcD74ikadAKXA4s2sukdxrn7w
350350
github.com/fluxcd/pkg/lockedfile v0.1.0/go.mod h1:EJLan8t9MiOcgTs8+puDjbE6I/KAfHbdvIy9VUgIjm8=
351351
github.com/fluxcd/pkg/runtime v0.16.0 h1:ynzvkOedFFZHlsa47EE7XtxZe8qs8edhtmjVZBEWi1Y=
352352
github.com/fluxcd/pkg/runtime v0.16.0/go.mod h1:Iklg+r/Jnqc9cNf2NK+iaosvw49CxX07Pyn0r3zSg/o=
353-
github.com/fluxcd/pkg/ssh v0.3.4 h1:Ko+MUNiiQG3evyoMO19iRk7d4X0VJ6w6+GEeVQ1jLC0=
354-
github.com/fluxcd/pkg/ssh v0.3.4/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE=
353+
github.com/fluxcd/pkg/ssh v0.4.0 h1:2HY88irZ5BCSMlzZExR6cnhRkjxCDsK/lTHHQqCJDJQ=
354+
github.com/fluxcd/pkg/ssh v0.4.0/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE=
355355
github.com/fluxcd/pkg/testserver v0.2.0 h1:Mj0TapmKaywI6Fi5wvt1LAZpakUHmtzWQpJNKQ0Krt4=
356356
github.com/fluxcd/pkg/testserver v0.2.0/go.mod h1:bgjjydkXsZTeFzjz9Cr4heGANr41uTB1Aj1Q5qzuYVk=
357357
github.com/fluxcd/pkg/untar v0.1.0 h1:k97V/xV5hFrAkIkVPuv5AVhyxh1ZzzAKba/lbDfGo6o=

pkg/git/libgit2/checkout.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ type CheckoutBranch struct {
6969
func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
7070
defer recoverPanic(&err)
7171

72+
remoteCallBacks := RemoteCallbacks(ctx, opts)
73+
7274
if managed.Enabled() {
7375
// We store the target url and auth options mapped to a unique ID. We overwrite the target url
7476
// with the TransportAuthID, because managed transports don't provide a way for any kind of
@@ -77,15 +79,18 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
7779
// Performing all fetch operations with the TransportAuthID as the url, lets the managed
7880
// transport action use it to fetch the registered transport options which contains the
7981
// _actual_ target url and the correct credentials to use.
82+
if opts.TransportAuthID == "" {
83+
return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.")
84+
}
8085
managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
8186
TargetURL: url,
8287
AuthOpts: opts,
8388
})
8489
url = opts.TransportAuthID
90+
remoteCallBacks = managed.RemoteCallbacks()
8591
defer managed.RemoveTransportOptions(opts.TransportAuthID)
8692
}
8793

88-
remoteCallBacks := RemoteCallbacks(ctx, opts)
8994
proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}
9095

9196
repo, remote, err := initializeRepoWithRemote(ctx, path, url, opts)
@@ -186,16 +191,21 @@ type CheckoutTag struct {
186191
func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
187192
defer recoverPanic(&err)
188193

194+
remoteCallBacks := RemoteCallbacks(ctx, opts)
195+
189196
if managed.Enabled() {
197+
if opts.TransportAuthID == "" {
198+
return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.")
199+
}
190200
managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
191201
TargetURL: url,
192202
AuthOpts: opts,
193203
})
194204
url = opts.TransportAuthID
205+
remoteCallBacks = managed.RemoteCallbacks()
195206
defer managed.RemoveTransportOptions(opts.TransportAuthID)
196207
}
197208

198-
remoteCallBacks := RemoteCallbacks(ctx, opts)
199209
proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}
200210

201211
repo, remote, err := initializeRepoWithRemote(ctx, path, url, opts)
@@ -274,19 +284,25 @@ type CheckoutCommit struct {
274284
func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
275285
defer recoverPanic(&err)
276286

287+
remoteCallBacks := RemoteCallbacks(ctx, opts)
288+
277289
if managed.Enabled() {
290+
if opts.TransportAuthID == "" {
291+
return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.")
292+
}
278293
managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
279294
TargetURL: url,
280295
AuthOpts: opts,
281296
})
282297
url = opts.TransportAuthID
298+
remoteCallBacks = managed.RemoteCallbacks()
283299
defer managed.RemoveTransportOptions(opts.TransportAuthID)
284300
}
285301

286302
repo, err := git2go.Clone(url, path, &git2go.CloneOptions{
287303
FetchOptions: git2go.FetchOptions{
288304
DownloadTags: git2go.DownloadTagsNone,
289-
RemoteCallbacks: RemoteCallbacks(ctx, opts),
305+
RemoteCallbacks: remoteCallBacks,
290306
ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
291307
},
292308
})
@@ -312,12 +328,18 @@ type CheckoutSemVer struct {
312328
func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
313329
defer recoverPanic(&err)
314330

331+
remoteCallBacks := RemoteCallbacks(ctx, opts)
332+
315333
if managed.Enabled() {
334+
if opts.TransportAuthID == "" {
335+
return nil, fmt.Errorf("can't use managed transport without a valid transport auth id.")
336+
}
316337
managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
317338
TargetURL: url,
318339
AuthOpts: opts,
319340
})
320341
url = opts.TransportAuthID
342+
remoteCallBacks = managed.RemoteCallbacks()
321343
defer managed.RemoveTransportOptions(opts.TransportAuthID)
322344
}
323345

@@ -329,7 +351,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g
329351
repo, err := git2go.Clone(url, path, &git2go.CloneOptions{
330352
FetchOptions: git2go.FetchOptions{
331353
DownloadTags: git2go.DownloadTagsAll,
332-
RemoteCallbacks: RemoteCallbacks(ctx, opts),
354+
RemoteCallbacks: remoteCallBacks,
333355
ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
334356
},
335357
})

pkg/git/libgit2/managed/http.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ func createClientRequest(transportAuthID string, action git2go.SmartServiceActio
157157
}
158158
targetURL := opts.TargetURL
159159

160+
if targetURL == "" {
161+
return nil, nil, fmt.Errorf("repository URL cannot be empty")
162+
}
163+
160164
if len(targetURL) > URLMaxLength {
161165
return nil, nil, fmt.Errorf("URL exceeds the max length (%d)", URLMaxLength)
162166
}
@@ -197,7 +201,9 @@ func createClientRequest(transportAuthID string, action git2go.SmartServiceActio
197201

198202
// Add any provided certificate to the http transport.
199203
if opts.AuthOpts != nil {
200-
req.SetBasicAuth(opts.AuthOpts.Username, opts.AuthOpts.Password)
204+
if len(opts.AuthOpts.Username) > 0 {
205+
req.SetBasicAuth(opts.AuthOpts.Username, opts.AuthOpts.Password)
206+
}
201207
if len(opts.AuthOpts.CAFile) > 0 {
202208
certPool := x509.NewCertPool()
203209
if ok := certPool.AppendCertsFromPEM(opts.AuthOpts.CAFile); !ok {

pkg/git/libgit2/managed/managed_test.go

Lines changed: 0 additions & 48 deletions
This file was deleted.

pkg/git/libgit2/managed/options.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,19 @@ var (
3535
m sync.RWMutex
3636
)
3737

38+
// AddTransportOptions registers a TransportOptions object mapped to the
39+
// provided id. The id must be a valid url, i.e. prefixed with http or ssh,
40+
// as the id is used as a dummy url for all git operations and the managed
41+
// transports will only be invoked for the protocols that they have been
42+
// registered for.
3843
func AddTransportOptions(id string, opts TransportOptions) {
3944
m.Lock()
4045
transportOpts[id] = opts
4146
m.Unlock()
4247
}
4348

49+
// RemoveTransportOptions removes the registerd TransportOptions object
50+
// mapped to the provided id.
4451
func RemoveTransportOptions(id string) {
4552
m.Lock()
4653
delete(transportOpts, id)

pkg/git/libgit2/managed/ssh.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func (t *sshSmartSubtransport) Action(credentialsID string, action git2go.SmartS
167167
cert := &git2go.Certificate{
168168
Kind: git2go.CertificateHostkey,
169169
Hostkey: git2go.HostkeyCertificate{
170-
Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5 | git2go.HostkeySHA256 | git2go.HostkeyRaw,
170+
Kind: git2go.HostkeySHA256,
171171
HashMD5: md5.Sum(marshaledKey),
172172
HashSHA1: sha1.Sum(marshaledKey),
173173
HashSHA256: sha256.Sum256(marshaledKey),
@@ -176,7 +176,10 @@ func (t *sshSmartSubtransport) Action(credentialsID string, action git2go.SmartS
176176
},
177177
}
178178

179-
return t.transport.SmartCertificateCheck(cert, true, hostname)
179+
if len(opts.AuthOpts.KnownHosts) > 0 {
180+
return KnownHostsCallback(hostname, opts.AuthOpts.KnownHosts)(cert, true, hostname)
181+
}
182+
return nil
180183
}
181184

182185
err = t.createConn(t.addr, sshConfig)

pkg/git/libgit2/managed/ssh_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,9 @@ func TestSSHManagedTransport_E2E(t *testing.T) {
113113
// We call git2go.Clone with transportID, so that the managed ssh transport can
114114
// fetch the correct set of credentials and the actual target url as well.
115115
repo, err := git2go.Clone(transportID, tmpDir, &git2go.CloneOptions{
116-
FetchOptions: git2go.FetchOptions{},
116+
FetchOptions: git2go.FetchOptions{
117+
RemoteCallbacks: RemoteCallbacks(),
118+
},
117119
CheckoutOptions: git2go.CheckoutOptions{
118120
Strategy: git2go.CheckoutForce,
119121
},

0 commit comments

Comments
 (0)