Merge pull request #740 from pjbgf/ssh-context

libgit2: enforce context timeout

Author: Paulo Gomes

controllers/gitrepository_controller.go

@@ -721,7 +721,10 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context,
 		}
 	}
 
-	checkoutStrategy, err := strategy.CheckoutStrategyForImplementation(ctx,
+	gitCtx, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
+	defer cancel()
+
+	checkoutStrategy, err := strategy.CheckoutStrategyForImplementation(gitCtx,
 		git.Implementation(obj.Spec.GitImplementation), checkoutOpts)
 	if err != nil {
 		// Do not return err as recovery without changes is impossible.
@@ -753,10 +756,6 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context,
 		}
 	}
 
-	// Checkout HEAD of reference in object
-	gitCtx, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
-	defer cancel()
-
 	commit, err := checkoutStrategy.Checkout(gitCtx, dir, obj.Spec.URL, authOpts)
 	if err != nil {
 		e := serror.NewGeneric(

pkg/git/libgit2/checkout.go

@@ -91,6 +91,7 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
 			TargetURL:    url,
 			AuthOpts:     opts,
 			ProxyOptions: &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
+			Context:      ctx,
 		})
 		url = opts.TransportOptionsURL
 		remoteCallBacks := managed.RemoteCallbacks()

pkg/git/libgit2/managed/options.go

@@ -17,6 +17,7 @@ limitations under the License.
 package managed
 
 import (
+	"context"
 	"sync"
 
 	"github.com/fluxcd/source-controller/pkg/git"
@@ -29,6 +30,7 @@ type TransportOptions struct {
 	TargetURL    string
 	AuthOpts     *git.AuthOptions
 	ProxyOptions *git2go.ProxyOptions
+	Context      context.Context
 }
 
 var (

pkg/git/libgit2/managed/ssh.go

@@ -92,6 +92,7 @@ type sshSmartSubtransport struct {
 	currentStream *sshSmartSubtransportStream
 	addr          string
 	connected     bool
+	ctx           context.Context
 }
 
 func (t *sshSmartSubtransport) Action(transportOptionsURL string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) {
@@ -103,6 +104,8 @@ func (t *sshSmartSubtransport) Action(transportOptionsURL string, action git2go.
 		return nil, fmt.Errorf("could not find transport options for object: %s", transportOptionsURL)
 	}
 
+	t.ctx = opts.Context
+
 	u, err := url.Parse(opts.TargetURL)
 	if err != nil {
 		return nil, err
@@ -206,16 +209,33 @@ func (t *sshSmartSubtransport) Action(transportOptionsURL string, action git2go.
 	// xref: https://github.com/golang/crypto/blob/eb4f295cb31f7fb5d52810411604a2638c9b19a2/ssh/session.go#L553-L558
 	go func() error {
 		defer w.Close()
+
+		var cancel context.CancelFunc
+		ctx := t.ctx
+
+		// When context is nil, creates a new with internal SSH connection timeout.
+		if ctx == nil {
+			ctx, cancel = context.WithTimeout(context.Background(), sshConnectionTimeOut)
+			defer cancel()
+		}
+
 		for {
-			if !t.connected {
+			select {
+			case <-ctx.Done():
+				t.Close()
 				return nil
-			}
 
-			_, err := io.Copy(w, reader)
-			if err != nil {
-				return err
+			default:
+				if !t.connected {
+					return nil
+				}
+
+				_, err := io.Copy(w, reader)
+				if err != nil {
+					return err
+				}
+				time.Sleep(5 * time.Millisecond)
 			}
-			time.Sleep(5 * time.Millisecond)
 		}
 	}()
 

pkg/git/strategy/proxy/strategy_proxy_test.go

@@ -292,7 +292,7 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) {
 
 				return nil, func() {}
 			},
-			shortTimeout:  false,
+			shortTimeout:  true,
 			wantUsedProxy: false,
 			wantError:     true,
 		},