From 4d8ebe69a0ec7d46bb72ba6abd0d0686826b8af7 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Tue, 8 Mar 2022 15:42:53 +0000 Subject: [PATCH 1/2] Ensure libgit2 resources are released Signed-off-by: Paulo Gomes --- controllers/imageupdateautomation_controller.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index 7ecea8be..5957b390 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -544,7 +544,8 @@ func switchBranch(repo *libgit2.Repository, pushBranch string) error { } defer head.Free() - _, err = repo.CreateBranch(pushBranch, head, false) + branch, err := repo.CreateBranch(pushBranch, head, false) + defer branch.Free() return err } @@ -652,6 +653,7 @@ func commitChangedManifests(tracelog logr.Logger, repo *libgit2.Repository, absR if err != nil { return "", err } + defer commit.Free() signedCommitID, err := commit.WithSignatureUsing(func(commitContent string) (string, string, error) { cipherText := new(bytes.Buffer) @@ -677,7 +679,7 @@ func commitChangedManifests(tracelog logr.Logger, repo *libgit2.Repository, absR } defer newHead.Free() - _, err = repo.References.Create( + ref, err := repo.References.Create( newHead.Name(), signedCommit.Id(), true, @@ -686,6 +688,7 @@ func commitChangedManifests(tracelog logr.Logger, repo *libgit2.Repository, absR if err != nil { return "", err } + defer ref.Free() return signedCommitID.String(), nil } From 5b15bb7f944ffa131b3df4b053f04e72eeb9bf79 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Mon, 21 Mar 2022 15:19:00 +0000 Subject: [PATCH 2/2] Implement Managed Transport for libgit2 libgit2 network operations are blocking and do not provide timeout nor context capabilities, leading to several reports 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. Signed-off-by: Paulo Gomes --- .../imageupdateautomation_controller.go | 35 +++++++++++++++---- main.go | 6 ++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index 5957b390..16f76a93 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -62,6 +62,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/pkg/git" gitlibgit2 "github.com/fluxcd/source-controller/pkg/git/libgit2" + "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" gitstrat "github.com/fluxcd/source-controller/pkg/git/strategy" imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" @@ -247,6 +248,34 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr return failWithError(err) } + repositoryURL := origin.Spec.URL + if managed.Enabled() { + // At present only HTTP connections have the ability to define remote options. + // Although this can be easily extended by ensuring that the fake URL below uses the + // target ssh scheme, and the libgit2/managed/ssh.go pulls that information accordingly. + // + // This is due to the fact the key libgit2 remote callbacks do not take place for HTTP + // whilst most still work for SSH. + if strings.HasPrefix(repositoryURL, "http") { + if access.auth != nil && len(access.auth.CAFile) > 0 { + // Due to the lack of the callback feature, a fake target URL is created to allow + // for the smart sub transport be able to pick the options specific for this + // GitRepository object. + // The URL should use unique information that do not collide in a multi tenant + // deployment. + repositoryURL = fmt.Sprintf("http://%s/%s/%d", auto.Name, auto.UID, auto.Generation) + managed.AddTransportOptions(repositoryURL, + managed.TransportOptions{ + TargetURL: repositoryURL, + CABundle: access.auth.CAFile, + }) + + // We remove the options from memory, to avoid accumulating unused options over time. + defer managed.RemoveTransportOptions(repositoryURL) + } + } + } + // Use the git operations timeout for the repo. cloneCtx, cancel := context.WithTimeout(ctx, origin.Spec.Timeout.Duration) defer cancel() @@ -470,12 +499,6 @@ func (r *ImageUpdateAutomationReconciler) automationsForImagePolicy(obj client.O return reqs } -// --- git ops - -// Note: libgit2 is always used for network operations; for cloning, -// it will do a non-shallow clone, and for anything else, it doesn't -// matter what is used. - type repoAccess struct { auth *git.AuthOptions url string diff --git a/main.go b/main.go index 8eb129c1..8a41e142 100644 --- a/main.go +++ b/main.go @@ -40,6 +40,8 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1" + "github.com/fluxcd/source-controller/pkg/git/libgit2/managed" + // +kubebuilder:scaffold:imports "github.com/fluxcd/image-automation-controller/controllers" ) @@ -137,6 +139,10 @@ func main() { } // +kubebuilder:scaffold:builder + if managed.Enabled() { + managed.InitManagedTransport() + } + setupLog.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager")