From 685d9277c45929a9ad5321d7a1a54700558c6996 Mon Sep 17 00:00:00 2001
From: chlins <chlins.zhang@gmail.com>
Date: Wed, 9 Sep 2020 19:27:27 +0800
Subject: [PATCH] fix(webhook): fix the verify cert logic of webhook endpoint

Signed-off-by: chlins <chlins.zhang@gmail.com>
---
 src/jobservice/job/impl/notification/http_helper.go   |  8 --------
 .../job/impl/notification/http_helper_test.go         |  7 ++++---
 src/jobservice/job/impl/notification/slack_job.go     | 11 +++++------
 src/jobservice/job/impl/notification/webhook_job.go   | 11 +++++------
 4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/src/jobservice/job/impl/notification/http_helper.go b/src/jobservice/job/impl/notification/http_helper.go
index f9bb42b37..ec519762f 100644
--- a/src/jobservice/job/impl/notification/http_helper.go
+++ b/src/jobservice/job/impl/notification/http_helper.go
@@ -31,11 +31,3 @@ func init() {
 		Transport: commonhttp.GetHTTPTransport(commonhttp.InsecureTransport),
 	}
 }
-
-// GetHTTPInstance ...
-func GetHTTPInstance(insec bool) *http.Client {
-	if insec {
-		return httpHelper.clients[insecure]
-	}
-	return httpHelper.clients[secure]
-}
diff --git a/src/jobservice/job/impl/notification/http_helper_test.go b/src/jobservice/job/impl/notification/http_helper_test.go
index 78eda8247..61287f55a 100644
--- a/src/jobservice/job/impl/notification/http_helper_test.go
+++ b/src/jobservice/job/impl/notification/http_helper_test.go
@@ -1,15 +1,16 @@
 package notification
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestHttpHelper(t *testing.T) {
-	c1 := GetHTTPInstance(true)
+	c1 := httpHelper.clients[insecure]
 	assert.NotNil(t, c1)
 
-	c2 := GetHTTPInstance(false)
+	c2 := httpHelper.clients[secure]
 	assert.NotNil(t, c2)
 
 	_, ok := httpHelper.clients["notExists"]
diff --git a/src/jobservice/job/impl/notification/slack_job.go b/src/jobservice/job/impl/notification/slack_job.go
index 51cc475e9..99731ed58 100644
--- a/src/jobservice/job/impl/notification/slack_job.go
+++ b/src/jobservice/job/impl/notification/slack_job.go
@@ -93,13 +93,12 @@ func (sj *SlackJob) Run(ctx job.Context, params job.Parameters) error {
 func (sj *SlackJob) init(ctx job.Context, params map[string]interface{}) error {
 	sj.logger = ctx.GetLogger()
 
-	// default use insecure transport
-	sj.client = GetHTTPInstance(true)
+	// default use secure transport
+	sj.client = httpHelper.clients[secure]
 	if v, ok := params["skip_cert_verify"]; ok {
-		if insecure, ok := v.(bool); ok {
-			if insecure {
-				sj.client = GetHTTPInstance(false)
-			}
+		if skipCertVerify, ok := v.(bool); ok && skipCertVerify {
+			// if skip cert verify is true, it means not verify remote cert, use insecure client
+			sj.client = httpHelper.clients[insecure]
 		}
 	}
 	return nil
diff --git a/src/jobservice/job/impl/notification/webhook_job.go b/src/jobservice/job/impl/notification/webhook_job.go
index bb7096d95..1f7fdd16a 100644
--- a/src/jobservice/job/impl/notification/webhook_job.go
+++ b/src/jobservice/job/impl/notification/webhook_job.go
@@ -71,13 +71,12 @@ func (wj *WebhookJob) init(ctx job.Context, params map[string]interface{}) error
 	wj.logger = ctx.GetLogger()
 	wj.ctx = ctx
 
-	// default use insecure transport
-	wj.client = GetHTTPInstance(true)
+	// default use secure transport
+	wj.client = httpHelper.clients[secure]
 	if v, ok := params["skip_cert_verify"]; ok {
-		if insecure, ok := v.(bool); ok {
-			if insecure {
-				wj.client = GetHTTPInstance(false)
-			}
+		if skipCertVerify, ok := v.(bool); ok && skipCertVerify {
+			// if skip cert verify is true, it means not verify remote cert, use insecure client
+			wj.client = httpHelper.clients[insecure]
 		}
 	}
 	return nil