Merge pull request #410 from pjbgf/early-eof

Enrich `early EOF` error message

Author: Paulo Gomes

DEVELOPMENT.md

@@ -155,4 +155,32 @@ Deploy `image-automation-controller` into the cluster that is configured in the
 
 ```sh
 make deploy
-```
+```
+
+### Debugging controller with VSCode
+
+Create a `.vscode/launch.json` file:
+```json
+{
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "Launch Package",
+            "type": "go",
+            "request": "launch",
+            "mode": "auto",
+            "envFile": "${workspaceFolder}/build/.env",
+            "program": "${workspaceFolder}/main.go"
+        }
+    ]
+}
+```
+
+Create the environment file containing details on how to load 
+`libgit2` dependencies:
+```bash
+make env
+```
+
+Start debugging by either clicking `Run` > `Start Debugging` or using
+the relevant shortcut.

Makefile

@@ -301,3 +301,15 @@ endif
 .PHONY: help
 help:  ## Display this help menu
 	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf "  \033[36m%-20s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
+
+# Creates an env file that can be used to load all image-automation-controller's
+# dependencies this is handy when you want to run adhoc debug sessions on tests or
+# start the controller in a new debug session.
+env: $(LIBGIT2)
+	echo 'GO_ENABLED="1"' > $(BUILD_DIR)/.env
+	echo 'PKG_CONFIG_PATH="$(PKG_CONFIG_PATH)"' >> $(BUILD_DIR)/.env
+	echo 'LIBRARY_PATH="$(LIBRARY_PATH)"' >> $(BUILD_DIR)/.env
+	echo 'CGO_CFLAGS="$(CGO_CFLAGS)"' >> $(BUILD_DIR)/.env
+	echo 'CGO_LDFLAGS="$(CGO_LDFLAGS)"' >> $(BUILD_DIR)/.env
+	echo 'KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS)' >> $(BUILD_DIR)/.env
+	echo 'GIT_CONFIG_GLOBAL=/dev/null' >> $(BUILD_DIR)/.env

controllers/git_test.go

@@ -14,6 +14,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/fluxcd/pkg/gittestserver"
+	"github.com/fluxcd/source-controller/pkg/git"
 )
 
 func populateRepoFromFixture(repo *libgit2.Repository, fixture string) error {
@@ -141,13 +142,13 @@ func TestPushRejected(t *testing.T) {
 	repoURL := gitServer.HTTPAddressWithCredentials() + "/appconfig.git"
 	cloneCtx, cancel := context.WithTimeout(ctx, time.Second*10)
 	defer cancel()
-	repo, err := clone(cloneCtx, repoURL, "test")
+	repo, err := clone(cloneCtx, repoURL, "test", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer repo.Free()
 
-	cleanup, err := configureTransportOptsForRepo(repo)
+	cleanup, err := configureTransportOptsForRepo(repo, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -172,6 +173,54 @@ func TestPushRejected(t *testing.T) {
 	}
 }
 
+func TestEarlyEOF(t *testing.T) {
+	g := NewWithT(t)
+
+	gitServer, err := gittestserver.NewTempGitServer()
+	g.Expect(err).ToNot(HaveOccurred())
+	defer os.RemoveAll(gitServer.Root())
+
+	username := "norris"
+	password := "chuck"
+
+	gitServer.
+		AutoCreate().
+		KeyDir(filepath.Join(t.TempDir(), "keys")).
+		Auth(username, password).
+		ReadOnly(true)
+
+	err = gitServer.StartHTTP()
+	g.Expect(err).ToNot(HaveOccurred())
+
+	err = initGitRepo(gitServer, "testdata/appconfig", "test", "/appconfig.git")
+	g.Expect(err).ToNot(HaveOccurred())
+
+	repoURL := gitServer.HTTPAddressWithCredentials() + "/appconfig.git"
+	cloneCtx, cancel := context.WithTimeout(ctx, time.Second*10)
+	defer cancel()
+
+	access := repoAccess{
+		auth: &git.AuthOptions{
+			Username: username,
+			Password: password,
+		},
+	}
+
+	repo, err := clone(cloneCtx, repoURL, "test", access.auth)
+	g.Expect(err).ToNot(HaveOccurred())
+
+	defer repo.Free()
+
+	cleanup, err := configureTransportOptsForRepo(repo, access.auth)
+	g.Expect(err).ToNot(HaveOccurred())
+
+	defer cleanup()
+
+	err = push(context.TODO(), repo.Workdir(), "test", access)
+	g.Expect(err).To(HaveOccurred())
+	g.Expect(err.Error()).To(ContainSubstring("early EOF (the SSH key may not have write access to the repository)"))
+}
+
 func Test_switchToBranch(t *testing.T) {
 	g := NewWithT(t)
 	gitServer, err := gittestserver.NewTempGitServer()
@@ -190,7 +239,7 @@ func Test_switchToBranch(t *testing.T) {
 	repoURL := gitServer.HTTPAddressWithCredentials() + "/appconfig.git"
 	cloneCtx, cancel := context.WithTimeout(ctx, time.Second*10)
 	defer cancel()
-	repo, err := clone(cloneCtx, repoURL, branch)
+	repo, err := clone(cloneCtx, repoURL, branch, nil)
 	g.Expect(err).ToNot(HaveOccurred())
 	defer repo.Free()
 
@@ -200,7 +249,7 @@ func Test_switchToBranch(t *testing.T) {
 	target := head.Target()
 
 	// register transport options and update remote to transport url
-	cleanup, err := configureTransportOptsForRepo(repo)
+	cleanup, err := configureTransportOptsForRepo(repo, nil)
 	if err != nil {
 		t.Fatal(err)
 	}

controllers/imageupdateautomation_controller.go

@@ -887,6 +887,9 @@ func push(ctx context.Context, path, branch string, access repoAccess) error {
 		ProxyOptions:    libgit2.ProxyOptions{Type: libgit2.ProxyTypeAuto},
 	})
 	if err != nil {
+		if strings.Contains(err.Error(), "early EOF") {
+			return fmt.Errorf("%w (the SSH key may not have write access to the repository)", err)
+		}
 		return libgit2PushError(err)
 	}
 	return callbackErr

controllers/update_test.go

@@ -54,6 +54,7 @@ import (
 	"github.com/fluxcd/pkg/gittestserver"
 	"github.com/fluxcd/pkg/ssh"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
+	"github.com/fluxcd/source-controller/pkg/git"
 	"github.com/fluxcd/source-controller/pkg/git/libgit2/managed"
 
 	imagev1 "github.com/fluxcd/image-automation-controller/api/v1beta1"
@@ -484,7 +485,7 @@ func TestImageAutomationReconciler_e2e(t *testing.T) {
 			// Clone the repo locally.
 			cloneCtx, cancel := context.WithTimeout(ctx, timeout)
 			defer cancel()
-			localRepo, err := clone(cloneCtx, cloneLocalRepoURL, branch)
+			localRepo, err := clone(cloneCtx, cloneLocalRepoURL, branch, nil)
 			g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo")
 			defer localRepo.Free()
 
@@ -628,7 +629,7 @@ func TestImageAutomationReconciler_e2e(t *testing.T) {
 			// separate localRepo.
 			cloneCtx, cancel := context.WithTimeout(ctx, timeout)
 			defer cancel()
-			localRepo, err := clone(cloneCtx, cloneLocalRepoURL, branch)
+			localRepo, err := clone(cloneCtx, cloneLocalRepoURL, branch, nil)
 			g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo")
 			defer localRepo.Free()
 
@@ -880,7 +881,7 @@ func compareRepoWithExpected(g *WithT, repoURL, branch, fixture string, changeFi
 	changeFixture(expected)
 	cloneCtx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
-	repo, err := clone(cloneCtx, repoURL, branch)
+	repo, err := clone(cloneCtx, repoURL, branch, nil)
 	g.Expect(err).ToNot(HaveOccurred())
 	defer repo.Free()
 	// NOTE: The workdir contains a trailing /. Clean it to not confuse the
@@ -897,7 +898,7 @@ func compareRepoWithExpected(g *WithT, repoURL, branch, fixture string, changeFi
 // it tries to figure it out by looking at the remote url of origin. It returns a function
 // which removes the transport options for this repo and sets the remote url of the origin
 // back to the actual url. Callers are expected to call this function in a deferred manner.
-func configureTransportOptsForRepo(repo *libgit2.Repository) (func(), error) {
+func configureTransportOptsForRepo(repo *libgit2.Repository, authOpts *git.AuthOptions) (func(), error) {
 	origin, err := repo.Remotes.Lookup(originRemote)
 	if err != nil {
 		return nil, err
@@ -911,6 +912,7 @@ func configureTransportOptsForRepo(repo *libgit2.Repository) (func(), error) {
 	transportOptsURL := u.Scheme + "://" + randStringRunes(5)
 	managed.AddTransportOptions(transportOptsURL, managed.TransportOptions{
 		TargetURL: repoURL,
+		AuthOpts:  authOpts,
 	})
 
 	err = repo.Remotes.SetUrl(originRemote, transportOptsURL)
@@ -926,7 +928,7 @@ func configureTransportOptsForRepo(repo *libgit2.Repository) (func(), error) {
 func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path string)) *libgit2.Oid {
 	cloneCtx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
-	repo, err := clone(cloneCtx, repoURL, branch)
+	repo, err := clone(cloneCtx, repoURL, branch, nil)
 	g.Expect(err).ToNot(HaveOccurred())
 	defer repo.Free()
 
@@ -940,7 +942,7 @@ func commitInRepo(g *WithT, repoURL, branch, msg string, changeFiles func(path s
 	id, err := commitWorkDir(repo, branch, msg, sig)
 	g.Expect(err).ToNot(HaveOccurred())
 
-	cleanup, err := configureTransportOptsForRepo(repo)
+	cleanup, err := configureTransportOptsForRepo(repo, nil)
 	g.Expect(err).ToNot(HaveOccurred())
 	defer cleanup()
 	origin, err := repo.Remotes.Lookup(originRemote)
@@ -1177,15 +1179,22 @@ func mockSignature(time time.Time) *git2go.Signature {
 	}
 }
 
-func clone(ctx context.Context, repoURL, branchName string) (*git2go.Repository, error) {
+func clone(ctx context.Context, repoURL, branchName string, authOpts *git.AuthOptions) (*git2go.Repository, error) {
 	dir, err := os.MkdirTemp("", "iac-clone-*")
 	if err != nil {
 		return nil, err
 	}
-	transportOptsURL := "http://" + randStringRunes(5)
+
+	u, err := url.Parse(repoURL)
+	if err != nil {
+		return nil, err
+	}
+
+	transportOptsURL := u.Scheme + "://" + randStringRunes(5)
 	managed.AddTransportOptions(transportOptsURL, managed.TransportOptions{
 		TargetURL: repoURL,
 		Context:   ctx,
+		AuthOpts:  authOpts,
 	})
 	defer managed.RemoveTransportOptions(transportOptsURL)
 
@@ -1214,7 +1223,7 @@ func clone(ctx context.Context, repoURL, branchName string) (*git2go.Repository,
 func waitForNewHead(g *WithT, repo *git2go.Repository, branch, preChangeHash string) {
 	var commitToResetTo *git2go.Commit
 
-	cleanup, err := configureTransportOptsForRepo(repo)
+	cleanup, err := configureTransportOptsForRepo(repo, nil)
 	g.Expect(err).ToNot(HaveOccurred())
 	defer cleanup()
 
@@ -1272,7 +1281,7 @@ func commitIdFromBranch(repo *git2go.Repository, branchName string) string {
 }
 
 func getRemoteHead(repo *git2go.Repository, branchName string) (*git2go.Oid, error) {
-	cleanup, err := configureTransportOptsForRepo(repo)
+	cleanup, err := configureTransportOptsForRepo(repo, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -1460,7 +1469,7 @@ func testWithCustomRepoAndImagePolicy(
 	repoURL := gitServer.HTTPAddressWithCredentials() + repositoryPath
 	cloneCtx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
-	localRepo, err := clone(cloneCtx, repoURL, args.branch)
+	localRepo, err := clone(cloneCtx, repoURL, args.branch, nil)
 	g.Expect(err).ToNot(HaveOccurred(), "failed to clone git repo")
 	defer localRepo.Free()
 

go.mod

@@ -12,17 +12,18 @@ require (
 	github.com/fluxcd/image-reflector-controller/api v0.19.3
 	github.com/fluxcd/pkg/apis/acl v0.0.3
 	github.com/fluxcd/pkg/apis/meta v0.14.2
-	github.com/fluxcd/pkg/gittestserver v0.5.4
+	github.com/fluxcd/pkg/gittestserver v0.6.0
 	github.com/fluxcd/pkg/runtime v0.16.2
 	github.com/fluxcd/pkg/ssh v0.5.0
 	github.com/fluxcd/source-controller v0.25.10
 	github.com/fluxcd/source-controller/api v0.25.10
 	github.com/go-logr/logr v1.2.3
-	github.com/google/go-containerregistry v0.10.0
+	github.com/google/go-containerregistry v0.11.0
 	github.com/libgit2/git2go/v33 v33.0.9
-	github.com/onsi/gomega v1.19.0
+	github.com/onsi/gomega v1.20.0
 	github.com/otiai10/copy v1.7.0
 	github.com/spf13/pflag v1.0.5
+	golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa
 	k8s.io/api v0.24.2
 	k8s.io/apimachinery v0.24.2
 	k8s.io/client-go v0.24.2
@@ -55,7 +56,7 @@ require (
 	github.com/emicklei/go-restful/v3 v3.8.0 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
 	github.com/evanphx/json-patch v5.6.0+incompatible // indirect
-	github.com/fluxcd/gitkit v0.5.1 // indirect
+	github.com/fluxcd/gitkit v0.6.0 // indirect
 	github.com/fluxcd/pkg/gitutil v0.1.0 // indirect
 	github.com/fluxcd/pkg/version v0.1.0 // indirect
 	github.com/fsnotify/fsnotify v1.5.1 // indirect
@@ -96,6 +97,7 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/client_golang v1.12.2 // indirect
 	github.com/prometheus/client_model v0.2.0 // indirect
@@ -110,10 +112,9 @@ require (
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
 	go.uber.org/zap v1.21.0 // indirect
-	golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d
-	golang.org/x/net v0.0.0-20220706163947-c90051bbdb60 // indirect
-	golang.org/x/oauth2 v0.0.0-20220622183110-fd043fe589d2 // indirect
-	golang.org/x/sys v0.0.0-20220624220833-87e55d714810 // indirect
+	golang.org/x/net v0.0.0-20220708220712-1185a9018129 // indirect
+	golang.org/x/oauth2 v0.0.0-20220718184931-c8730f7fcb92 // indirect
+	golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect
 	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 // indirect