Skip to content

Commit

Permalink
controllers: use new git contract
Browse files Browse the repository at this point in the history
This commit makes use of the refactored `git` package, which has
been reworked to increase stability and test coverage, and ensures
implementation details do not leak out into the "main wrapper".

This indirectly seems to resolve a memory leak that happenedd with
the previous wiring, thereby fixing fluxcd#247.

The code changes for this controller itself are minimal, mostly
ensuring the auth and checkout configurations are created in the
"new way".

Signed-off-by: Hidde Beydals <[email protected]>
  • Loading branch information
hiddeco authored and souleb committed Mar 12, 2024
1 parent 65f5368 commit a3897de
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 39 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ CACHE := cache

# Version of the source-controller from which to get the GitRepository CRD.
# Change this if you bump the source-controller/api version in go.mod.
SOURCE_VER ?= v0.16.0
SOURCE_VER ?= v0.17.0

# Version of the image-reflector-controller from which to get the ImagePolicy CRD.
# Change this if you bump the image-reflector-controller/api version in go.mod.
Expand Down
5 changes: 2 additions & 3 deletions controllers/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/go-logr/logr"

"github.com/fluxcd/pkg/gittestserver"
"github.com/fluxcd/source-controller/pkg/git"
)

func populateRepoFromFixture(repo *gogit.Repository, fixture string) error {
Expand Down Expand Up @@ -159,7 +158,7 @@ func TestPushRejected(t *testing.T) {
// This is here to guard against push in general being broken
err = push(context.TODO(), tmp, "main", repoAccess{
url: repoURL,
auth: &git.Auth{},
auth: nil,
})
if err != nil {
t.Fatal(err)
Expand All @@ -174,7 +173,7 @@ func TestPushRejected(t *testing.T) {
// pushed to.
err = push(context.TODO(), tmp, branch, repoAccess{
url: repoURL,
auth: &git.Auth{},
auth: nil,
})
if err == nil {
t.Error("push to a forbidden branch is expected to fail, but succeeded")
Expand Down
34 changes: 16 additions & 18 deletions controllers/imageupdateautomation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import (
"github.com/fluxcd/pkg/runtime/predicates"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
"github.com/fluxcd/source-controller/pkg/git"
gitlibgit2 "github.com/fluxcd/source-controller/pkg/git/libgit2"
gitstrat "github.com/fluxcd/source-controller/pkg/git/strategy"

imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1"
Expand Down Expand Up @@ -444,35 +445,28 @@ func (r *ImageUpdateAutomationReconciler) automationsForImagePolicy(obj client.O
// matter what is used.

type repoAccess struct {
auth *git.Auth
auth *git.AuthOptions
url string
}

func (r *ImageUpdateAutomationReconciler) getRepoAccess(ctx context.Context, repository *sourcev1.GitRepository) (repoAccess, error) {
var access repoAccess
access.auth = &git.Auth{}
access.url = repository.Spec.URL

authStrat, err := gitstrat.AuthSecretStrategyForURL(access.url, git.CheckoutOptions{GitImplementation: sourcev1.LibGit2Implementation})
if err != nil {
return access, err
}

if repository.Spec.SecretRef != nil && authStrat != nil {

if repository.Spec.SecretRef != nil {
name := types.NamespacedName{
Namespace: repository.GetNamespace(),
Name: repository.Spec.SecretRef.Name,
}

var secret corev1.Secret
err = r.Client.Get(ctx, name, &secret)
secret := &corev1.Secret{}
err := r.Client.Get(ctx, name, secret)
if err != nil {
err = fmt.Errorf("auth secret error: %w", err)
return access, err
}

access.auth, err = authStrat.Method(secret)
access.auth, err = git.AuthOptionsFromSecret(access.url, secret)
if err != nil {
err = fmt.Errorf("auth error: %w", err)
return access, err
Expand All @@ -482,19 +476,23 @@ func (r *ImageUpdateAutomationReconciler) getRepoAccess(ctx context.Context, rep
}

func (r repoAccess) remoteCallbacks() libgit2.RemoteCallbacks {
return libgit2.RemoteCallbacks{
CertificateCheckCallback: r.auth.CertCallback,
CredentialsCallback: r.auth.CredCallback,
}
return gitlibgit2.RemoteCallbacks(r.auth)
}

// cloneInto clones the upstream repository at the `ref` given (which
// can be `nil`). It returns a `*gogit.Repository` since that is used
// for committing changes.
func cloneInto(ctx context.Context, access repoAccess, ref *sourcev1.GitRepositoryRef, path string) (*gogit.Repository, error) {
checkoutStrat, err := gitstrat.CheckoutStrategyForRef(ref, git.CheckoutOptions{GitImplementation: sourcev1.LibGit2Implementation})
opts := git.CheckoutOptions{}
if ref != nil {
opts.Tag = ref.Tag
opts.SemVer = ref.SemVer
opts.Tag = ref.Tag
opts.Branch = ref.Branch
}
checkoutStrat, err := gitstrat.CheckoutStrategyForImplementation(ctx, sourcev1.LibGit2Implementation, opts)
if err == nil {
_, _, err = checkoutStrat.Checkout(ctx, path, access.url, access.auth)
_, err = checkoutStrat.Checkout(ctx, path, access.url, access.auth)
}
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ require (
// If you bump this, change REFLECTOR_VER in the Makefile to match
github.com/fluxcd/image-reflector-controller/api v0.12.0
github.com/fluxcd/pkg/apis/meta v0.10.1
github.com/fluxcd/pkg/gittestserver v0.3.1
github.com/fluxcd/pkg/gittestserver v0.4.1
github.com/fluxcd/pkg/runtime v0.12.1
github.com/fluxcd/pkg/ssh v0.1.0
// If you bump this, change SOURCE_VER in the Makefile to match
github.com/fluxcd/source-controller v0.16.0
github.com/fluxcd/source-controller/api v0.16.0
github.com/fluxcd/source-controller v0.17.0
github.com/fluxcd/source-controller/api v0.17.0
github.com/go-git/go-billy/v5 v5.3.1
github.com/go-git/go-git/v5 v5.4.2
github.com/go-logr/logr v0.4.0
Expand Down
Loading

0 comments on commit a3897de

Please sign in to comment.