From 3c2ee1f8d93df72cc8eb8946c2f65dd648121556 Mon Sep 17 00:00:00 2001 From: Duane Appleby Date: Tue, 14 Jan 2020 09:52:15 +0000 Subject: [PATCH] Issue 1880: Adds the ability to disable the certificate validation in the client interacting with the git server performing actions related to the use of the pipeline resource of type pullrequest. To disable, user specifies insecure-skip-tls-verify parameter in their resource with value set to "true". Value is false by default. --- cmd/pullrequest-init/main.go | 14 ++-- docs/resources.md | 5 ++ .../v1alpha1/pull_request_resource.go | 38 ++++++--- .../v1alpha1/pull_request_resource_test.go | 40 ++++++--- pkg/pullrequest/scm.go | 50 ++++++++--- pkg/pullrequest/scm_test.go | 84 +++++++++++-------- 6 files changed, 157 insertions(+), 74 deletions(-) diff --git a/cmd/pullrequest-init/main.go b/cmd/pullrequest-init/main.go index 5e23ff9a8eb..00f569b0384 100644 --- a/cmd/pullrequest-init/main.go +++ b/cmd/pullrequest-init/main.go @@ -19,18 +19,18 @@ import ( "context" "flag" "fmt" - "os" - "github.com/tektoncd/pipeline/pkg/pullrequest" "go.uber.org/zap" "knative.dev/pkg/logging" + "os" ) var ( - prURL = flag.String("url", "", "The url of the pull request to initialize.") - path = flag.String("path", "", "Path of directory under which PR will be copied") - mode = flag.String("mode", "download", "Whether to operate in download or upload mode") - provider = flag.String("provider", "", "The SCM provider to use. Optional") + prURL = flag.String("url", "", "The url of the pull request to initialize.") + path = flag.String("path", "", "Path of directory under which PR will be copied") + mode = flag.String("mode", "download", "Whether to operate in download or upload mode") + provider = flag.String("provider", "", "The SCM provider to use. Optional") + skipTLSVerify = flag.Bool("insecure-skip-tls-verify", false, "Enable skipping TLS certificate verification in the git client. Defaults to false") ) func main() { @@ -45,7 +45,7 @@ func main() { ctx := context.Background() token := os.Getenv("AUTH_TOKEN") - client, err := pullrequest.NewSCMHandler(logger, *prURL, *provider, token) + client, err := pullrequest.NewSCMHandler(logger, *prURL, *provider, token, *skipTLSVerify) if err != nil { logger.Fatalf("error creating GitHub client: %v", err) } diff --git a/docs/resources.md b/docs/resources.md index 94943349134..69136117598 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -439,6 +439,9 @@ Params that can be added are the following: 1. `url`: represents the location of the pull request to fetch. 1. `provider`: represents the SCM provider to use. This will be "guessed" based on the url if not set. Valid values are `github` or `gitlab` today. +1. `insecure-skip-tls-verify`: represents whether to skip verification of certificates + from the git server. Valid values are `"true"` or `"false"`, the default being + `"false"`. #### Statuses @@ -456,6 +459,8 @@ URLs should be of the form: https://github.com/tektoncd/pipeline/pull/1 The PullRequest resource works with self hosted or enterprise GitHub/GitLab instances. Simply provide the pull request URL and set the `provider` parameter. +If you need to skip certificate validation set the `insecure-skip-tls-verify` +parameter to `"true"`. ```yaml apiVersion: tekton.dev/v1alpha1 diff --git a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go index 288613eb136..bf0025809ab 100644 --- a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "fmt" + "strconv" "strings" "github.com/tektoncd/pipeline/pkg/apis/pipeline" @@ -46,25 +47,34 @@ type PullRequestResource struct { // Secrets holds a struct to indicate a field name and corresponding secret name to populate it. Secrets []SecretParam `json:"secrets"` - PRImage string `json:"-"` + PRImage string `json:"-"` + InsecureSkipTLSVerify bool `json:"insecure-skip-tls-verify"` } // NewPullRequestResource create a new git resource to pass to a Task func NewPullRequestResource(prImage string, r *PipelineResource) (*PullRequestResource, error) { if r.Spec.Type != PipelineResourceTypePullRequest { - return nil, fmt.Errorf("PipelineResource: Cannot create a PR resource from a %s Pipeline Resource", r.Spec.Type) + return nil, fmt.Errorf("cannot create a PR resource from a %s Pipeline Resource", r.Spec.Type) } prResource := PullRequestResource{ - Name: r.Name, - Type: r.Spec.Type, - Secrets: r.Spec.SecretParams, - PRImage: prImage, + Name: r.Name, + Type: r.Spec.Type, + Secrets: r.Spec.SecretParams, + PRImage: prImage, + InsecureSkipTLSVerify: false, } for _, param := range r.Spec.Params { - if strings.EqualFold(param.Name, "URL") { + switch { + case strings.EqualFold(param.Name, "URL"): prResource.URL = param.Value - } else if strings.EqualFold(param.Name, "Provider") { + case strings.EqualFold(param.Name, "Provider"): prResource.Provider = param.Value + case strings.EqualFold(param.Name, "insecure-skip-tls-verify"): + verify, err := strconv.ParseBool(param.Value) + if err != nil { + return nil, fmt.Errorf("error occurred converting %q to boolean in Pipeline Resource %s", param.Value, r.Name) + } + prResource.InsecureSkipTLSVerify = verify } } @@ -89,10 +99,11 @@ func (s *PullRequestResource) GetURL() string { // Replacements is used for template replacement on a PullRequestResource inside of a Taskrun. func (s *PullRequestResource) Replacements() map[string]string { return map[string]string{ - "name": s.Name, - "type": string(s.Type), - "url": s.URL, - "provider": s.Provider, + "name": s.Name, + "type": string(s.Type), + "url": s.URL, + "provider": s.Provider, + "insecure-skip-tls-verify": strconv.FormatBool(s.InsecureSkipTLSVerify), } } @@ -115,6 +126,9 @@ func (s *PullRequestResource) getSteps(mode string, sourcePath string) []Step { if s.Provider != "" { args = append(args, []string{"-provider", s.Provider}...) } + if s.InsecureSkipTLSVerify { + args = append(args, "-insecure-skip-tls-verify=true") + } evs := []corev1.EnvVar{} for _, sec := range s.Secrets { diff --git a/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go index 0c11219b453..a2dee9e62bd 100644 --- a/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go @@ -41,12 +41,13 @@ func TestPullRequest_NewResource(t *testing.T) { } want := &v1alpha1.PullRequestResource{ - Name: pr.Name, - Type: v1alpha1.PipelineResourceTypePullRequest, - URL: url, - Provider: "github", - Secrets: pr.Spec.SecretParams, - PRImage: "override-with-pr:latest", + Name: pr.Name, + Type: v1alpha1.PipelineResourceTypePullRequest, + URL: url, + Provider: "github", + Secrets: pr.Spec.SecretParams, + PRImage: "override-with-pr:latest", + InsecureSkipTLSVerify: false, } if diff := cmp.Diff(want, got); diff != "" { t.Error(diff) @@ -70,9 +71,10 @@ const workspace = "/workspace" func containerTestCases(mode string) []testcase { return []testcase{{ in: &v1alpha1.PullRequestResource{ - Name: "nocreds", - URL: "https://example.com", - PRImage: "override-with-pr:latest", + Name: "nocreds", + URL: "https://example.com", + PRImage: "override-with-pr:latest", + InsecureSkipTLSVerify: false, }, out: []v1alpha1.Step{{Container: corev1.Container{ Name: "pr-source-nocreds-9l9zj", @@ -84,8 +86,9 @@ func containerTestCases(mode string) []testcase { }}}, }, { in: &v1alpha1.PullRequestResource{ - Name: "creds", - URL: "https://example.com", + Name: "creds", + URL: "https://example.com", + InsecureSkipTLSVerify: false, Secrets: []v1alpha1.SecretParam{{ FieldName: "authToken", SecretName: "github-creds", @@ -112,6 +115,21 @@ func containerTestCases(mode string) []testcase { }, }}, }}}, + }, { + in: &v1alpha1.PullRequestResource{ + Name: "nocreds", + URL: "https://example.com", + PRImage: "override-with-pr:latest", + InsecureSkipTLSVerify: true, + }, + out: []v1alpha1.Step{{Container: corev1.Container{ + Name: "pr-source-nocreds-mssqb", + Image: "override-with-pr:latest", + WorkingDir: pipeline.WorkspaceDir, + Command: []string{"/ko-app/pullrequest-init"}, + Args: []string{"-url", "https://example.com", "-path", workspace, "-mode", mode, "-insecure-skip-tls-verify=true"}, + Env: []corev1.EnvVar{}, + }}}, }} } diff --git a/pkg/pullrequest/scm.go b/pkg/pullrequest/scm.go index 0301beb8299..68bb3967ad7 100644 --- a/pkg/pullrequest/scm.go +++ b/pkg/pullrequest/scm.go @@ -17,7 +17,6 @@ limitations under the License. package pullrequest import ( - "context" "fmt" "net/http" "net/url" @@ -26,12 +25,14 @@ import ( "golang.org/x/oauth2" + "crypto/tls" + "github.com/jenkins-x/go-scm/scm/driver/github" "github.com/jenkins-x/go-scm/scm/driver/gitlab" "go.uber.org/zap" ) -func NewSCMHandler(logger *zap.SugaredLogger, raw, provider, token string) (*Handler, error) { +func NewSCMHandler(logger *zap.SugaredLogger, raw, provider, token string, skipTLSVerify bool) (*Handler, error) { u, err := url.Parse(raw) if err != nil { return nil, err @@ -49,16 +50,16 @@ func NewSCMHandler(logger *zap.SugaredLogger, raw, provider, token string) (*Han var handler *Handler switch provider { case "github": - handler, err = githubHandlerFromURL(u, token, logger) + handler, err = githubHandlerFromURL(u, token, skipTLSVerify, logger) case "gitlab": - handler, err = gitlabHandlerFromURL(u, token, logger) + handler, err = gitlabHandlerFromURL(u, token, skipTLSVerify, logger) default: return nil, fmt.Errorf("unsupported pr url: %s", raw) } return handler, err } -func githubHandlerFromURL(u *url.URL, token string, logger *zap.SugaredLogger) (*Handler, error) { +func githubHandlerFromURL(u *url.URL, token string, skipTLSVerify bool, logger *zap.SugaredLogger) (*Handler, error) { split := strings.Split(u.Path, "/") if len(split) < 5 { return nil, fmt.Errorf("could not determine PR from URL: %v", u) @@ -83,17 +84,34 @@ func githubHandlerFromURL(u *url.URL, token string, logger *zap.SugaredLogger) ( } } ownerRepo := fmt.Sprintf("%s/%s", owner, repo) - h := NewHandler(logger, client, ownerRepo, prNumber) + if token != "" { ts := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: token}, ) - h.client.Client = oauth2.NewClient(context.Background(), ts) + client.Client = &http.Client{ + Transport: &oauth2.Transport{ + Source: ts, + Base: &http.Transport{ + /* #nosec G402 */ + TLSClientConfig: &tls.Config{InsecureSkipVerify: skipTLSVerify}, + }, + }, + } + } else { + client.Client = &http.Client{ + Transport: &http.Transport{ + /* #nosec G402 */ + TLSClientConfig: &tls.Config{InsecureSkipVerify: skipTLSVerify}, + }, + } } + + h := NewHandler(logger, client, ownerRepo, prNumber) return h, nil } -func gitlabHandlerFromURL(u *url.URL, token string, logger *zap.SugaredLogger) (*Handler, error) { +func gitlabHandlerFromURL(u *url.URL, token string, skipTLSVerify bool, logger *zap.SugaredLogger) (*Handler, error) { // The project name can be multiple /'s deep, so split on / and work from right to left. split := strings.Split(u.Path, "/") @@ -124,14 +142,26 @@ func gitlabHandlerFromURL(u *url.URL, token string, logger *zap.SugaredLogger) ( return nil, fmt.Errorf("error creating client: %w", err) } } + if token != "" { client.Client = &http.Client{ Transport: &gitlabClient{ - token: token, - transport: http.DefaultTransport, + token: token, + transport: &http.Transport{ + /* #nosec G402 */ + TLSClientConfig: &tls.Config{InsecureSkipVerify: skipTLSVerify}, + }, + }, + } + } else { + client.Client = &http.Client{ + Transport: &http.Transport{ + /* #nosec G402 */ + TLSClientConfig: &tls.Config{InsecureSkipVerify: skipTLSVerify}, }, } } + return NewHandler(logger, client, project, prInt), nil } diff --git a/pkg/pullrequest/scm_test.go b/pkg/pullrequest/scm_test.go index 5aec293c334..3da9f6a5843 100644 --- a/pkg/pullrequest/scm_test.go +++ b/pkg/pullrequest/scm_test.go @@ -17,7 +17,9 @@ limitations under the License. package pullrequest import ( + "net/http" "net/url" + "strconv" "testing" "go.uber.org/zap" @@ -26,56 +28,62 @@ import ( func TestNewSCMHandler(t *testing.T) { tests := []struct { - name string - raw string - wantBaseURL string - wantRepo string - wantNum int - wantErr bool + name string + raw string + skipTLSVerify bool + wantBaseURL string + wantRepo string + wantNum int + wantErr bool }{ { - name: "github", - raw: "https://github.com/foo/bar/pull/1", - wantBaseURL: "https://api.github.com/", - wantRepo: "foo/bar", - wantNum: 1, - wantErr: false, + name: "github", + raw: "https://github.com/foo/bar/pull/1", + skipTLSVerify: false, + wantBaseURL: "https://api.github.com/", + wantRepo: "foo/bar", + wantNum: 1, + wantErr: false, }, { - name: "custom github", - raw: "https://github.tekton.dev/foo/baz/pull/2", - wantBaseURL: "https://github.tekton.dev/api/v3/", - wantRepo: "foo/baz", - wantNum: 2, - wantErr: false, + name: "custom github", + raw: "https://github.tekton.dev/foo/baz/pull/2", + skipTLSVerify: false, + wantBaseURL: "https://github.tekton.dev/api/v3/", + wantRepo: "foo/baz", + wantNum: 2, + wantErr: false, }, { - name: "gitlab", - raw: "https://gitlab.com/foo/bar/merge_requests/3", - wantBaseURL: "https://gitlab.com/", - wantRepo: "foo/bar", - wantNum: 3, - wantErr: false, + name: "gitlab", + raw: "https://gitlab.com/foo/bar/merge_requests/3", + skipTLSVerify: false, + wantBaseURL: "https://gitlab.com/", + wantRepo: "foo/bar", + wantNum: 3, + wantErr: false, }, { - name: "gitlab multi-level", - raw: "https://gitlab.com/foo/bar/baz/merge_requests/3", - wantBaseURL: "https://gitlab.com/", - wantRepo: "foo/bar/baz", - wantNum: 3, - wantErr: false, + name: "gitlab multi-level", + raw: "https://gitlab.com/foo/bar/baz/merge_requests/3", + skipTLSVerify: true, + wantBaseURL: "https://gitlab.com/", + wantRepo: "foo/bar/baz", + wantNum: 3, + wantErr: false, }, { - name: "unsupported", - raw: "https://unsupported.com/foo/baz/merge_requests/3", - wantErr: true, + name: "unsupported", + raw: "https://unsupported.com/foo/baz/merge_requests/3", + skipTLSVerify: true, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { observer, _ := observer.New(zap.InfoLevel) logger := zap.New(observer).Sugar() - got, err := NewSCMHandler(logger, tt.raw, "", "") + got, err := NewSCMHandler(logger, tt.raw, "", "", tt.skipTLSVerify) if err != nil { if !tt.wantErr { t.Errorf("NewSCMHandler() error = %v, wantErr %v", err, tt.wantErr) @@ -91,6 +99,14 @@ func TestNewSCMHandler(t *testing.T) { if baseURL := got.client.BaseURL.String(); baseURL != tt.wantBaseURL { t.Errorf("NewSCMHandler() [base url] = %v, want %v", baseURL, tt.wantBaseURL) } + switch transport := got.client.Client.Transport.(type) { + case *http.Transport: + if transport.TLSClientConfig.InsecureSkipVerify != tt.skipTLSVerify { + t.Errorf("NewSCMHandler() [InsecureSkipVerify] = %v, want %v", strconv.FormatBool(transport.TLSClientConfig.InsecureSkipVerify), tt.skipTLSVerify) + } + default: + t.Errorf("NewSCMHandler() client.Client.Transport was not of type http.Transport") + } }) } }