Fix problem with pushing further commits to a "push branch"

controllers/imageupdateautomation_controller.go

@@ -33,6 +33,7 @@ import (
  libgit2 "github.com/libgit2/git2go/v31"
 
  securejoin "github.com/cyphar/filepath-securejoin"
+ "github.com/go-git/go-git/v5/config"
  "github.com/go-git/go-git/v5/plumbing"
  "github.com/go-git/go-git/v5/plumbing/object"
  "github.com/go-logr/logr"
@@ -44,6 +45,7 @@ import (
  kuberecorder "k8s.io/client-go/tools/record"
  "k8s.io/client-go/tools/reference"
  ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/builder"
  "sigs.k8s.io/controller-runtime/pkg/client"
  "sigs.k8s.io/controller-runtime/pkg/handler"
  "sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -70,7 +72,6 @@ const originRemote = "origin"
 const defaultMessageTemplate = `Update from image update automation`
 
 const repoRefKey = ".spec.gitRepository"
-const imagePolicyKey = ".spec.update.imagePolicy"
 
 const signingSecretKey = "git.asc"
 
@@ -188,7 +189,11 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
  // When there's a push spec, the pushed-to branch is where commits
  // shall be made
  if auto.Spec.Push != nil {
- if err := switchBranch(repo, auto.Spec.Push.Branch); err != nil {
+ pushBranch := auto.Spec.Push.Branch
+ if err := fetch(ctx, tmp, repo, pushBranch, access, origin.Spec.GitImplementation); err != nil && err != errRemoteBranchMissing {
+ return failWithError(err)
+ }
+ if err = switchBranch(repo, pushBranch); err != nil {
  return failWithError(err)
  }
  }
@@ -294,9 +299,10 @@ func (r *ImageUpdateAutomationReconciler) SetupWithManager(mgr ctrl.Manager) err
  }
 
  return ctrl.NewControllerManagedBy(mgr).
- For(&imagev1.ImageUpdateAutomation{}).
- WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})).
+ For(&imagev1.ImageUpdateAutomation{}, builder.WithPredicates(
+  predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}))).
  Watches(&source.Kind{Type: &sourcev1.GitRepository{}}, handler.EnqueueRequestsFromMapFunc(r.automationsForGitRepo)).
+ Watches(&source.Kind{Type: &imagev1_reflect.ImagePolicy{}}, handler.EnqueueRequestsFromMapFunc(r.automationsForImagePolicy)).
  Complete(r)
 }
 
@@ -351,6 +357,24 @@ func (r *ImageUpdateAutomationReconciler) automationsForGitRepo(obj client.Objec
  return reqs
 }
 
+// automationsForImagePolicy fetches all the automation objects that
+// might depend on a image policy object. Since the link is via
+// markers in the git repo, _any_ automation object in the same
+// namespace could be affected.
+func (r *ImageUpdateAutomationReconciler) automationsForImagePolicy(obj client.Object) []reconcile.Request {
+ ctx := context.Background()
+ var autoList imagev1.ImageUpdateAutomationList
+ if err := r.List(ctx, &autoList, client.InNamespace(obj.GetNamespace())); err != nil {
+ return nil
+ }
+ reqs := make([]reconcile.Request, len(autoList.Items), len(autoList.Items))
+ for i := range autoList.Items {
+ reqs[i].NamespacedName.Name = autoList.Items[i].GetName()
+ reqs[i].NamespacedName.Namespace = autoList.Items[i].GetNamespace()
+ }
+ return reqs
+}
+
 // --- git ops
 
 type repoAccess struct {
@@ -389,6 +413,13 @@ func (r *ImageUpdateAutomationReconciler) getRepoAccess(ctx context.Context, rep
  return access, nil
 }
 
+func (r repoAccess) remoteCallbacks() libgit2.RemoteCallbacks {
+ return libgit2.RemoteCallbacks{
+ CertificateCheckCallback: r.auth.CertCallback,
+ CredentialsCallback: r.auth.CredCallback,
+ }
+}
+
 // cloneInto clones the upstream repository at the `branch` given,
 // using the git library indicated by `impl`. It returns a
 // `*gogit.Repository` regardless of the git library, since that is
@@ -411,11 +442,10 @@ func cloneInto(ctx context.Context, access repoAccess, branch, path, impl string
 // branch given. If the branch does not exist, it is created using the
 // head as the starting point.
 func switchBranch(repo *gogit.Repository, pushBranch string) error {
- remoteBranch := plumbing.NewRemoteReferenceName(originRemote, pushBranch)
  localBranch := plumbing.NewBranchReferenceName(pushBranch)
 
- // is the remote branch already present?
- branchHead, err := repo.Reference(remoteBranch, false)
+ // is the branch already present?
+ _, err := repo.Reference(localBranch, false)
  switch {
  case err == plumbing.ErrReferenceNotFound:
  // make a new branch, starting at HEAD
@@ -430,17 +460,15 @@ func switchBranch(repo *gogit.Repository, pushBranch string) error {
  case err != nil:
  return err
  default:
- // make a local branch that references the remote branch
- branchRef := plumbing.NewHashReference(localBranch, branchHead.Hash())
- if err = repo.Storer.SetReference(branchRef); err != nil {
- return err
- }
+ // local branch found, great
+ break
  }
 
  tree, err := repo.Worktree()
  if err != nil {
  return err
  }
+
  return tree.Checkout(&gogit.CheckoutOptions{
  Branch: localBranch,
  })
@@ -520,6 +548,62 @@ func (r *ImageUpdateAutomationReconciler) getSigningEntity(ctx context.Context,
  return entities[0], nil
 }
 
+var errRemoteBranchMissing = errors.New("remote branch missing")
+
+// fetch gets the remote branch given and updates the local branch
+// head of the same name, so it can be switched to. If the fetch
+// completes, it returns nil; if the remote branch is missing, it
+// returns errRemoteBranchMissing (this is to work in sympathy with
+// `switchBranch`, which will create the branch if it doesn't
+// exist). For any other problem it will return the error.
+func fetch(ctx context.Context, path string, repo *gogit.Repository, branch string, access repoAccess, impl string) error {
+ refspec := fmt.Sprintf("refs/heads/%s:refs/heads/%s", branch, branch)
+ switch impl {
+ case sourcev1.LibGit2Implementation:
+ lg2repo, err := libgit2.OpenRepository(path)
+ if err != nil {
+ return err
+ }
+ return fetchLibgit2(lg2repo, refspec, access)
+ case sourcev1.GoGitImplementation:
+ return fetchGoGit(ctx, repo, refspec, access)
+ default:
+ return fmt.Errorf("unknown git implementation %q", impl)
+ }
+}
+
+func fetchLibgit2(repo *libgit2.Repository, refspec string, access repoAccess) error {
+ origin, err := repo.Remotes.Lookup(originRemote)
+ if err != nil {
+ return err
+ }
+ err = origin.Fetch(
+ []string{refspec},
+ &libgit2.FetchOptions{
+ RemoteCallbacks: access.remoteCallbacks(),
+ }, "",
+ )
+ if err != nil && libgit2.IsErrorCode(err, libgit2.ErrorCodeNotFound) {
+ return errRemoteBranchMissing
+ }
+ return err
+}
+
+func fetchGoGit(ctx context.Context, repo *gogit.Repository, refspec string, access repoAccess) error {
+ err := repo.FetchContext(ctx, &gogit.FetchOptions{
+ RemoteName: originRemote,
+ RefSpecs: []config.RefSpec{config.RefSpec(refspec)},
+ Auth: access.auth.AuthMethod,
+ })
+ if err == gogit.NoErrAlreadyUpToDate {
+ return nil
+ }
+ if _, ok := err.(gogit.NoMatchingRefSpecError); ok {
+ return errRemoteBranchMissing
+ }
+ return err
+}
+
 // push pushes the branch given to the origin using the git library
 // indicated by `impl`. It's passed both the path to the repo and a
 // gogit.Repository value, since the latter may as well be used if the
@@ -533,15 +617,18 @@ func push(ctx context.Context, path string, repo *gogit.Repository, branch strin
  }
  return pushLibgit2(lg2repo, access, branch)
  case sourcev1.GoGitImplementation:
- return pushGoGit(ctx, repo, access)
+ return pushGoGit(ctx, repo, access, branch)
  default:
  return fmt.Errorf("unknown git implementation %q", impl)
  }
 }
 
-func pushGoGit(ctx context.Context, repo *gogit.Repository, access repoAccess) error {
+func pushGoGit(ctx context.Context, repo *gogit.Repository, access repoAccess, branch string) error {
+ refspec := config.RefSpec(fmt.Sprintf("refs/heads/%s:refs/heads/%s", branch, branch))
  err := repo.PushContext(ctx, &gogit.PushOptions{
- Auth: access.auth.AuthMethod,
+ RemoteName: originRemote,
+ Auth: access.auth.AuthMethod,
+ RefSpecs: []config.RefSpec{refspec},
  })
  return gogitPushError(err)
 }
@@ -570,10 +657,7 @@ func pushLibgit2(repo *libgit2.Repository, access repoAccess, branch string) err
  return err
  }
  err = origin.Push([]string{fmt.Sprintf("refs/heads/%s:refs/heads/%s", branch, branch)}, &libgit2.PushOptions{
- RemoteCallbacks: libgit2.RemoteCallbacks{
- CertificateCheckCallback: access.auth.CertCallback,
- CredentialsCallback: access.auth.CredCallback,
- },
+ RemoteCallbacks: access.remoteCallbacks(),
  })
  return libgit2PushError(err)
 }

controllers/update_test.go

@@ -564,7 +564,6 @@ Images:
  )
 
  const latestImage = "helloworld:1.0.1"
- const evenLatestImage = "helloworld:1.2.0"
 
  BeforeEach(func() {
  cloneLocalRepoURL = gitServer.HTTPAddressWithCredentials() + repositoryPath
@@ -726,6 +725,22 @@ Images:
  Expect(commit.Message).To(Equal(commitMessage))
  })
 
+ It("pushes another commit to the existing push branch", func() {
+ // observe the first commit
+ waitForNewHead(localRepo, pushBranch)
+ head, err := localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true)
+ headHash := head.String()
+ Expect(err).NotTo(HaveOccurred())
+
+ // update the policy and expect another commit in the push branch
+ policy.Status.LatestImage = "helloworld:v1.3.0"
+ Expect(k8sClient.Status().Update(context.TODO(), policy)).To(Succeed())
+ waitForNewHead(localRepo, pushBranch)
+ head, err = localRepo.Reference(plumbing.NewRemoteReferenceName(originRemote, pushBranch), true)
+ Expect(err).NotTo(HaveOccurred())
+ Expect(head.String()).NotTo(Equal(headHash))
+ })
+
  AfterEach(func() {
  Expect(k8sClient.Delete(context.Background(), update)).To(Succeed())
  })