From fac15063e0bbdac1e64c2215c7dd93897c6c410d Mon Sep 17 00:00:00 2001
From: Nan Yu <nanyu@google.com>
Date: Fri, 23 Feb 2024 22:01:22 +0000
Subject: [PATCH] fix: update git-sync to v4.2.1 to fix a pulling issue

There is a bug in git-sync v4.1.0. When branches in different remotes
are out of sync, `git-sync` fetches the commit SHA from the last line,
which may not be the latest. This leads to an issue that Config Sync
couldn't pull the latest commit from HEAD.

The issue was fixed in v4.2.0 by
https://github.com/kubernetes/git-sync/pull/845.
This commit updates git-sync to v4.2.1 to include the fix.

It also bumps the debian-base to latest version for CVE fixes.

b/325341042
---
 Makefile                                      |  2 +-
 e2e/testcases/git_sync_test.go                | 27 ++++++++-----------
 .../reconciler-manager-configmap.yaml         |  2 +-
 3 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index b162596675..2fcc6637fc 100644
--- a/Makefile
+++ b/Makefile
@@ -39,7 +39,7 @@ GO_DIR := $(OUTPUT_DIR)/go
 # Base image used for all golang containers
 GOLANG_IMAGE := golang:1.21.5-bookworm
 # Base image used for debian containers
-DEBIAN_BASE_IMAGE := gcr.io/gke-release/debian-base:bookworm-v1.0.1-gke.0
+DEBIAN_BASE_IMAGE := gcr.io/gke-release/debian-base:bookworm-v1.0.1-gke.1
 # Base image used for gcloud install, primarily for test images.
 # We use -slim for a smaller base image where we can choose which components to install.
 # https://cloud.google.com/sdk/docs/downloads-docker#docker_image_options
diff --git a/e2e/testcases/git_sync_test.go b/e2e/testcases/git_sync_test.go
index c91fb684d0..77d8d59b7f 100644
--- a/e2e/testcases/git_sync_test.go
+++ b/e2e/testcases/git_sync_test.go
@@ -18,7 +18,6 @@ import (
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"kpt.dev/configsync/e2e/nomostest"
 	"kpt.dev/configsync/e2e/nomostest/gitproviders"
 	nomostesting "kpt.dev/configsync/e2e/nomostest/testing"
@@ -34,15 +33,9 @@ func TestMultipleRemoteBranchesOutOfSync(t *testing.T) {
 	if err := nt.KubeClient.Get(configsync.RootSyncName, configmanagement.ControllerNamespace, rs); err != nil {
 		nt.T.Fatal(err)
 	}
-	initialSyncedCommit := rs.Status.LastSyncedCommit
 
 	nt.T.Log("Create an extra remote tracking branch")
 	nt.Must(nt.RootRepos[configsync.RootSyncName].Push("HEAD:refs/remotes/upstream/main"))
-	nt.T.Cleanup(func() {
-		// Delete the remote tracking branch in the end so other subsequent tests
-		// can pull from the latest commit, instead of the HEAD of the remote.
-		nt.Must(nt.RootRepos[configsync.RootSyncName].Push(":refs/remotes/upstream/main"))
-	})
 
 	nt.T.Logf("Update the remote main branch by adding a test namespace")
 	nt.Must(nt.RootRepos[configsync.RootSyncName].Add("acme/namespaces/hello/ns.yaml", fake.NamespaceObject("hello")))
@@ -59,16 +52,18 @@ func TestMultipleRemoteBranchesOutOfSync(t *testing.T) {
 		nt.T.Fatal(err)
 	}
 
-	// Apply the mitigation first to validate Config Sync couldn't pull the latest commit.
-	nt.T.Logf("Verify the issue exist with the default branch and revision")
+	nt.T.Logf("Verify git-sync can pull the latest commit with the default branch and revision")
 	nomostest.SetGitBranch(nt, configsync.RootSyncName, gitproviders.MainBranch)
-	if err := nt.WatchForAllSyncs(nomostest.WithRootSha1Func(
-		// DefaultRootSha1Fn returns the hash with `git rev-parse HEAD`, which is
-		// different from `git ls-remote ...`
-		// So, overwrite the root hash with the initial lastSyncedCommit.
-		func(_ *nomostest.NT, _ types.NamespacedName) (string, error) {
-			return initialSyncedCommit, nil
-		})); err != nil {
+	// WatchForAllSyncs validates RootSync's lastSyncedCommit is updated to the
+	// local HEAD with the DefaultRootSha1Fn function.
+	if err := nt.WatchForAllSyncs(); err != nil {
+		nt.T.Fatal(err)
+	}
+
+	nt.T.Logf("Remove the test namespace to make sure git-sync can fetch newer commit")
+	nt.Must(nt.RootRepos[configsync.RootSyncName].Remove("acme/namespaces/hello/ns.yaml"))
+	nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("remove Namespace"))
+	if err := nt.WatchForAllSyncs(); err != nil {
 		nt.T.Fatal(err)
 	}
 	if err := nt.ValidateNotFound("hello", "", &corev1.Namespace{}); err != nil {
diff --git a/manifests/templates/reconciler-manager-configmap.yaml b/manifests/templates/reconciler-manager-configmap.yaml
index ec752869db..c73a673627 100644
--- a/manifests/templates/reconciler-manager-configmap.yaml
+++ b/manifests/templates/reconciler-manager-configmap.yaml
@@ -101,7 +101,7 @@ data:
                - ALL
            imagePullPolicy: IfNotPresent
          - name: git-sync
-           image: gcr.io/config-management-release/git-sync:v4.1.0-gke.7__linux_amd64
+           image: gcr.io/config-management-release/git-sync:v4.2.1-gke.1__linux_amd64
            args: ["--root=/repo/source", "--link=rev", "--max-failures=30", "--error-file=error.json"]
            volumeMounts:
            - name: repo