Remove dependency on libgit2 credentials callback

Injects transport and auth options at the transport level directly to
bypass the inbuilt credentials callback because of it's several
shortcomings. Moves some of the pre-existing logic from the reconciler
to the checkout implementation.

Signed-off-by: Sanskar Jaiswal <[email protected]>

Author: aryan9600

controllers/gitrepository_controller.go

@@ -458,27 +458,15 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 	repositoryURL := obj.Spec.URL
 	// managed GIT transport only affects the libgit2 implementation
 	if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation {
-		// 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.
+		// We set the TransportAuthID of this set of authentication options here by constructing
+		// a unique ID that won't clash in a multi tenant environment. This unique ID is used by
+		// libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in
+		// libgit2, which is inflexible and unstable.
 		if strings.HasPrefix(repositoryURL, "http") {
-			// 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", obj.Name, obj.UID, obj.Generation)
-			managed.AddTransportOptions(repositoryURL,
-				managed.TransportOptions{
-					TargetURL: obj.Spec.URL,
-					CABundle:  authOpts.CAFile,
-				})
-
-			// We remove the options from memory, to avoid accumulating unused options over time.
-			defer managed.RemoveTransportOptions(repositoryURL)
+			authOpts.TransportAuthID = fmt.Sprintf("http://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
+		}
+		if strings.HasPrefix(repositoryURL, "ssh") {
+			authOpts.TransportAuthID = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
 		}
 	}
 

pkg/git/libgit2/checkout.go

@@ -69,6 +69,22 @@ type CheckoutBranch struct {
 func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
 	defer recoverPanic(&err)
 
+	if managed.Enabled() {
+		// We store the target url and auth options mapped to a unique ID. We overwrite the target url
+		// with the TransportAuthID, because managed transports don't provide a way for any kind of
+		// dependency injection. This lets us have a way of doing interop between application level code
+		// and transport level code.
+		// Performing all fetch operations with the TransportAuthID as the url, lets the managed
+		// transport action use it to fetch the registered transport options which contains the
+		// _actual_ target url and the correct credentials to use.
+		managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
+			TargetURL: url,
+			AuthOpts:  opts,
+		})
+		url = opts.TransportAuthID
+		defer managed.RemoveTransportOptions(opts.TransportAuthID)
+	}
+
 	remoteCallBacks := RemoteCallbacks(ctx, opts)
 	proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}
 
@@ -170,6 +186,15 @@ type CheckoutTag struct {
 func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
 	defer recoverPanic(&err)
 
+	if managed.Enabled() {
+		managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
+			TargetURL: url,
+			AuthOpts:  opts,
+		})
+		url = opts.TransportAuthID
+		defer managed.RemoveTransportOptions(opts.TransportAuthID)
+	}
+
 	remoteCallBacks := RemoteCallbacks(ctx, opts)
 	proxyOpts := &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}
 
@@ -249,6 +274,15 @@ type CheckoutCommit struct {
 func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
 	defer recoverPanic(&err)
 
+	if managed.Enabled() {
+		managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
+			TargetURL: url,
+			AuthOpts:  opts,
+		})
+		url = opts.TransportAuthID
+		defer managed.RemoveTransportOptions(opts.TransportAuthID)
+	}
+
 	repo, err := git2go.Clone(url, path, &git2go.CloneOptions{
 		FetchOptions: git2go.FetchOptions{
 			DownloadTags:    git2go.DownloadTagsNone,
@@ -278,6 +312,15 @@ type CheckoutSemVer struct {
 func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) {
 	defer recoverPanic(&err)
 
+	if managed.Enabled() {
+		managed.AddTransportOptions(opts.TransportAuthID, managed.TransportOptions{
+			TargetURL: url,
+			AuthOpts:  opts,
+		})
+		url = opts.TransportAuthID
+		defer managed.RemoveTransportOptions(opts.TransportAuthID)
+	}
+
 	verConstraint, err := semver.NewConstraint(c.SemVer)
 	if err != nil {
 		return nil, fmt.Errorf("semver parse error: %w", err)

pkg/git/libgit2/managed/http.go

@@ -86,7 +86,7 @@ type httpSmartSubtransport struct {
 	httpTransport *http.Transport
 }
 
-func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) {
+func (t *httpSmartSubtransport) Action(transportAuthID string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) {
 	var proxyFn func(*http.Request) (*url.URL, error)
 	proxyOpts, err := t.transport.SmartProxyOptions()
 	if err != nil {
@@ -109,7 +109,7 @@ func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServ
 	t.httpTransport.Proxy = proxyFn
 	t.httpTransport.DisableCompression = false
 
-	client, req, err := createClientRequest(targetUrl, action, t.httpTransport)
+	client, req, err := createClientRequest(transportAuthID, action, t.httpTransport)
 	if err != nil {
 		return nil, err
 	}
@@ -142,36 +142,22 @@ func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServ
 	return stream, nil
 }
 
-func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t *http.Transport) (*http.Client, *http.Request, error) {
+func createClientRequest(transportAuthID string, action git2go.SmartServiceAction, t *http.Transport) (*http.Client, *http.Request, error) {
 	var req *http.Request
 	var err error
 
 	if t == nil {
 		return nil, nil, fmt.Errorf("failed to create client: transport cannot be nil")
 	}
 
-	finalUrl := targetUrl
-	opts, found := transportOptions(targetUrl)
-	if found {
-		if opts.TargetURL != "" {
-			// override target URL only if options are found and a new targetURL
-			// is provided.
-			finalUrl = opts.TargetURL
-		}
+	opts, found := getTransportOptions(transportAuthID)
 
-		// Add any provided certificate to the http transport.
-		if len(opts.CABundle) > 0 {
-			cap := x509.NewCertPool()
-			if ok := cap.AppendCertsFromPEM(opts.CABundle); !ok {
-				return nil, nil, fmt.Errorf("failed to use certificate from PEM")
-			}
-			t.TLSClientConfig = &tls.Config{
-				RootCAs: cap,
-			}
-		}
+	if !found {
+		return nil, nil, fmt.Errorf("failed to create client: could not find transport options for the object: %s", transportAuthID)
 	}
+	targetURL := opts.TargetURL
 
-	if len(finalUrl) > URLMaxLength {
+	if len(targetURL) > URLMaxLength {
 		return nil, nil, fmt.Errorf("URL exceeds the max length (%d)", URLMaxLength)
 	}
 
@@ -182,20 +168,20 @@ func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t *
 
 	switch action {
 	case git2go.SmartServiceActionUploadpackLs:
-		req, err = http.NewRequest("GET", finalUrl+"/info/refs?service=git-upload-pack", nil)
+		req, err = http.NewRequest("GET", targetURL+"/info/refs?service=git-upload-pack", nil)
 
 	case git2go.SmartServiceActionUploadpack:
-		req, err = http.NewRequest("POST", finalUrl+"/git-upload-pack", nil)
+		req, err = http.NewRequest("POST", targetURL+"/git-upload-pack", nil)
 		if err != nil {
 			break
 		}
 		req.Header.Set("Content-Type", "application/x-git-upload-pack-request")
 
 	case git2go.SmartServiceActionReceivepackLs:
-		req, err = http.NewRequest("GET", finalUrl+"/info/refs?service=git-receive-pack", nil)
+		req, err = http.NewRequest("GET", targetURL+"/info/refs?service=git-receive-pack", nil)
 
 	case git2go.SmartServiceActionReceivepack:
-		req, err = http.NewRequest("POST", finalUrl+"/git-receive-pack", nil)
+		req, err = http.NewRequest("POST", targetURL+"/git-receive-pack", nil)
 		if err != nil {
 			break
 		}
@@ -209,6 +195,20 @@ func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t *
 		return nil, nil, err
 	}
 
+	// Add any provided certificate to the http transport.
+	if opts.AuthOpts != nil {
+		req.SetBasicAuth(opts.AuthOpts.Username, opts.AuthOpts.Password)
+		if len(opts.AuthOpts.CAFile) > 0 {
+			certPool := x509.NewCertPool()
+			if ok := certPool.AppendCertsFromPEM(opts.AuthOpts.CAFile); !ok {
+				return nil, nil, fmt.Errorf("failed to use certificate from PEM")
+			}
+			t.TLSClientConfig = &tls.Config{
+				RootCAs: certPool,
+			}
+		}
+	}
+
 	req.Header.Set("User-Agent", "git/2.0 (flux-libgit2)")
 	return client, req, nil
 }
@@ -239,7 +239,6 @@ type httpSmartSubtransportStream struct {
 	recvReply   sync.WaitGroup
 	httpError   error
 	m           sync.RWMutex
-	targetURL   string
 }
 
 func newManagedHttpStream(owner *httpSmartSubtransport, req *http.Request, client *http.Client) *httpSmartSubtransportStream {
@@ -324,29 +323,8 @@ func (self *httpSmartSubtransportStream) sendRequest() error {
 
 	var resp *http.Response
 	var err error
-	var userName string
-	var password string
-
-	// Obtain the credentials and use them if available.
-	cred, err := self.owner.transport.SmartCredentials("", git2go.CredentialTypeUserpassPlaintext)
-	if err != nil {
-		// Passthrough error indicates that no credentials were provided.
-		// Continue without credentials.
-		if err.Error() != git2go.ErrorCodePassthrough.String() {
-			return err
-		}
-	}
-
-	if cred != nil {
-		defer cred.Free()
-
-		userName, password, err = cred.GetUserpassPlaintext()
-		if err != nil {
-			return err
-		}
-	}
-
 	var content []byte
+
 	for {
 		req := &http.Request{
 			Method: self.req.Method,
@@ -365,7 +343,6 @@ func (self *httpSmartSubtransportStream) sendRequest() error {
 			req.ContentLength = -1
 		}
 
-		req.SetBasicAuth(userName, password)
 		traceLog.Info("[http]: new request", "method", req.Method, "URL", req.URL)
 		resp, err = self.client.Do(req)
 		if err != nil {