fix: Respect template.HTTP.timeoutSeconds (#7136)

Signed-off-by: Simon Behar <simbeh7@gmail.com>
argoproj · Nov 17, 2021 · a4390dd · a4390dd
1 parent c1553df
commit a4390dd
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 12 deletions.
diff --git a/workflow/executor/agent.go b/workflow/executor/agent.go
@@ -108,6 +108,9 @@ func (ae *AgentExecutor) Agent(ctx context.Context) error {
 }
 
 func (ae *AgentExecutor) executeHTTPTemplate(ctx context.Context, tmpl wfv1.Template) (*wfv1.Outputs, error) {
+	if tmpl.HTTP == nil {
+		return nil, fmt.Errorf("attempting to execute template that is not of type HTTP")
+	}
 	httpTemplate := tmpl.HTTP
 	request, err := http.NewRequest(httpTemplate.Method, httpTemplate.URL, bytes.NewBufferString(httpTemplate.Body))
 	if err != nil {
@@ -125,7 +128,7 @@ func (ae *AgentExecutor) executeHTTPTemplate(ctx context.Context, tmpl wfv1.Temp
 		}
 		request.Header.Add(header.Name, value)
 	}
-	response, err := argohttp.SendHttpRequest(request)
+	response, err := argohttp.SendHttpRequest(request, httpTemplate.TimeoutSeconds)
 	if err != nil {
 		return nil, err
 	}

diff --git a/workflow/executor/http/http.go b/workflow/executor/http/http.go
@@ -4,20 +4,23 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"time"
 
 	log "github.com/sirupsen/logrus"
 )
 
-func SendHttpRequest(request *http.Request) (string, error) {
-	out, err := http.DefaultClient.Do(request)
-
+func SendHttpRequest(request *http.Request, timeout *int64) (string, error) {
+	httpClient := http.DefaultClient
+	if timeout != nil {
+		httpClient.Timeout = time.Duration(*timeout) * time.Second
+	}
+	out, err := httpClient.Do(request)
 	if err != nil {
 		return "", err
 	}
-	// Close the connection
 	defer out.Body.Close()
 
-	log.WithFields(log.Fields{"url": request.URL, "status": out.Status}).Info("HTTP request made")
+	log.WithFields(log.Fields{"url": request.URL, "status": out.Status}).Info("HTTP Request Sent")
 	data, err := ioutil.ReadAll(out.Body)
 	if err != nil {
 		return "", err
@@ -27,5 +30,4 @@ func SendHttpRequest(request *http.Request) (string, error) {
 	}
 
 	return string(data), nil
-
 }
diff --git a/workflow/executor/http/http_test.go b/workflow/executor/http/http_test.go
@@ -6,22 +6,31 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"k8s.io/utils/pointer"
 )
 
 func TestSendHttpRequest(t *testing.T) {
 	t.Run("SuccessfulRequest", func(t *testing.T) {
-		request, err := http.NewRequest(http.MethodGet, "http://www.google.com", bytes.NewBuffer([]byte{}))
+		request, err := http.NewRequest(http.MethodGet, "http://httpstat.us/200", bytes.NewBuffer([]byte{}))
 		assert.NoError(t, err)
-		response, err := SendHttpRequest(request)
+		_, err = SendHttpRequest(request, nil)
 		assert.NoError(t, err)
-		assert.NotEmpty(t, response)
 	})
 	t.Run("NotFoundRequest", func(t *testing.T) {
-		request, err := http.NewRequest(http.MethodGet, "http://www.notfound.com/test", bytes.NewBuffer([]byte{}))
+		request, err := http.NewRequest(http.MethodGet, "http://httpstat.us/404", bytes.NewBuffer([]byte{}))
 		assert.NoError(t, err)
-		response, err := SendHttpRequest(request)
+		response, err := SendHttpRequest(request, nil)
 		assert.Error(t, err)
 		assert.Empty(t, response)
 		assert.Equal(t, "404 Not Found", err.Error())
 	})
+	t.Run("TimeoutRequest", func(t *testing.T) {
+		// Request sleeps for 4 seconds, but timeout is 2
+		request, err := http.NewRequest(http.MethodGet, "http://httpstat.us/200?sleep=4000", bytes.NewBuffer([]byte{}))
+		assert.NoError(t, err)
+		response, err := SendHttpRequest(request, pointer.Int64Ptr(2))
+		assert.Error(t, err)
+		assert.Empty(t, response)
+		assert.Equal(t, `Get "http://httpstat.us/200?sleep=4000": context deadline exceeded (Client.Timeout exceeded while awaiting headers)`, err.Error())
+	})
 }