Skip to content

Commit aa32881

Browse files
Paulo Gomesdarkowlzz
andcommitted
Implement Managed Transport for libgit2
libgit2 network operations are blocking and do not provide timeout nor context capabilities, leading for several reports by users of the controllers hanging indefinitely. By using managed transport, golang primitives such as http.Transport and net.Dial can be used to ensure timeouts are enforced. Co-Authored-by: Sunny <[email protected]> Signed-off-by: Paulo Gomes <[email protected]>
1 parent 9bbcd09 commit aa32881

File tree

8 files changed

+753
-1
lines changed

8 files changed

+753
-1
lines changed

controllers/gitrepository_controller.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import (
5050
"github.com/fluxcd/source-controller/internal/reconcile/summarize"
5151
"github.com/fluxcd/source-controller/internal/util"
5252
"github.com/fluxcd/source-controller/pkg/git"
53+
"github.com/fluxcd/source-controller/pkg/git/libgit2/managed"
5354
"github.com/fluxcd/source-controller/pkg/git/strategy"
5455
"github.com/fluxcd/source-controller/pkg/sourceignore"
5556
)
@@ -369,10 +370,37 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
369370
return sreconcile.ResultEmpty, e
370371
}
371372

373+
repositoryURL := obj.Spec.URL
374+
// managed GIT transport only affects the libgit2 implementation
375+
if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation {
376+
// At present only HTTP connections have the ability to define remote options.
377+
// Although this can be easily extended by ensuring that the fake URL below uses the
378+
// target ssh scheme, and the libgit2/managed/ssh.go pulls that information accordingly.
379+
//
380+
// This is due to the fact the key libgit2 remote callbacks do not take place for HTTP
381+
// whilst most still work for SSH.
382+
if strings.HasPrefix(repositoryURL, "http") {
383+
// Due to the lack of the callback feature, a fake target URL is created to allow
384+
// for the smart sub transport be able to pick the options specific for this
385+
// GitRepository object.
386+
// The URL should use unique information that do not collide in a multi tenant
387+
// deployment.
388+
repositoryURL = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
389+
managed.AddTransportOptions(repositoryURL,
390+
managed.TransportOptions{
391+
TargetUrl: obj.Spec.URL,
392+
CABundle: authOpts.CAFile,
393+
})
394+
395+
// We remove the options from memory, to avoid accumulating unused options over time.
396+
defer managed.RemoveTransportOptions(repositoryURL)
397+
}
398+
}
399+
372400
// Checkout HEAD of reference in object
373401
gitCtx, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
374402
defer cancel()
375-
c, err := checkoutStrategy.Checkout(gitCtx, dir, obj.Spec.URL, authOpts)
403+
c, err := checkoutStrategy.Checkout(gitCtx, dir, repositoryURL, authOpts)
376404
if err != nil {
377405
e := &serror.Event{
378406
Err: fmt.Errorf("failed to checkout and determine revision: %w", err),

main.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
4646
"github.com/fluxcd/source-controller/controllers"
4747
"github.com/fluxcd/source-controller/internal/helm"
48+
"github.com/fluxcd/source-controller/pkg/git/libgit2/managed"
4849
// +kubebuilder:scaffold:imports
4950
)
5051

@@ -226,6 +227,10 @@ func main() {
226227
startFileServer(storage.BasePath, storageAddr, setupLog)
227228
}()
228229

230+
if managed.Enabled() {
231+
managed.InitManagedTransport()
232+
}
233+
229234
setupLog.Info("starting manager")
230235
if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
231236
setupLog.Error(err, "problem running manager")

pkg/git/libgit2/managed/flag.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package managed
2+
3+
import (
4+
"os"
5+
"strings"
6+
)
7+
8+
// Enabled defines whether the use of Managed Transport should be enabled.
9+
// This is only affects git operations that uses libgit2 implementation.
10+
//
11+
// True is returned when the environment variable `EXPERIMENTAL_GIT_TRANSPORT`
12+
// is detected with the value of `true` or `1`.
13+
func Enabled() bool {
14+
if v, ok := os.LookupEnv("EXPERIMENTAL_GIT_TRANSPORT"); ok {
15+
return strings.ToLower(v) == "true" || v == "1"
16+
}
17+
return false
18+
}

0 commit comments

Comments
 (0)