From 823ebbbbe06d16d0a320c87c4e66100462327844 Mon Sep 17 00:00:00 2001
From: Chmouel Boudjnah <chmouel@chmouel.com>
Date: Thu, 23 Nov 2023 11:58:33 +0100
Subject: [PATCH] Set timeout for http.Client

Used a default of 100 seconds for the timeout.  This is the same value
as what c# uses by default. I didn't find any other good default.

Make annotation_tasks_install.go use the common function instead of
defining it's own.

Fixes #1514

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
---
 pkg/matcher/annotation_tasks_install.go | 10 +---------
 pkg/params/clients/clients.go           | 22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/pkg/matcher/annotation_tasks_install.go b/pkg/matcher/annotation_tasks_install.go
index c5689a75e..5c55537c1 100644
--- a/pkg/matcher/annotation_tasks_install.go
+++ b/pkg/matcher/annotation_tasks_install.go
@@ -4,8 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io"
-	"net/http"
 	"os"
 	"regexp"
 	"strings"
@@ -106,16 +104,10 @@ func (rt RemoteTasks) getRemote(ctx context.Context, uri string, fromHub bool) (
 
 	switch {
 	case strings.HasPrefix(uri, "https://"), strings.HasPrefix(uri, "http://"): // if it starts with http(s)://, it is a remote resource
-		req, _ := http.NewRequestWithContext(ctx, http.MethodGet, uri, nil)
-		res, err := rt.Run.Clients.HTTP.Do(req)
+		data, err := rt.Run.Clients.GetURL(ctx, uri)
 		if err != nil {
 			return "", err
 		}
-		if res.StatusCode != http.StatusOK {
-			return "", fmt.Errorf("cannot get remote resource: %s: %s", uri, res.Status)
-		}
-		data, _ := io.ReadAll(res.Body)
-		defer res.Body.Close()
 		rt.Logger.Infof("successfully fetched %s from remote https url", uri)
 		return string(data), nil
 	case fromHub && strings.Contains(uri, "://"): // if it contains ://, it is a remote custom catalog
diff --git a/pkg/params/clients/clients.go b/pkg/params/clients/clients.go
index 04b9739db..f53248e12 100644
--- a/pkg/params/clients/clients.go
+++ b/pkg/params/clients/clients.go
@@ -4,7 +4,9 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
+	"time"
 
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/consoleui"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/generated/clientset/versioned"
@@ -18,6 +20,13 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 )
 
+const (
+	// most programming languages  do not have a timeout, but c# does a default
+	// of 100 seconds so using that value.
+	ConnectMaxWaitTime = 100 * time.Second
+	RequestMaxWaitTime = 100 * time.Second
+)
+
 type Clients struct {
 	ClientInitialized bool
 	PipelineAsCode    versioned.Interface
@@ -30,7 +39,10 @@ type Clients struct {
 }
 
 func (c *Clients) GetURL(ctx context.Context, url string) ([]byte, error) {
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+	nctx, cancel := context.WithTimeout(ctx, RequestMaxWaitTime)
+	defer cancel()
+
+	req, err := http.NewRequestWithContext(nctx, http.MethodGet, url, nil)
 	if err != nil {
 		return []byte{}, err
 	}
@@ -125,6 +137,14 @@ func (c *Clients) NewClients(ctx context.Context, info *info.Info) error {
 	}()
 	c.Log = logger
 
+	c.HTTP = http.Client{
+		Timeout: RequestMaxWaitTime,
+		Transport: &http.Transport{
+			DialContext: (&net.Dialer{
+				Timeout: ConnectMaxWaitTime,
+			}).DialContext,
+		},
+	}
 	config, err := c.kubeConfig(info)
 	if err != nil {
 		return err