From 7e68d4e74c71714720f365d9bfef30578d4c6db9 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 sslVerify parameter in their resource with value set to "false". Value is true by default. --- cmd/pullrequest-init/main.go | 14 +++--- docs/resources.md | 4 ++ .../v1alpha1/pull_request_resource.go | 33 +++++++++----- .../v1alpha1/pull_request_resource_test.go | 40 ++++++++++++----- pkg/pullrequest/scm.go | 45 ++++++++++++++----- pkg/pullrequest/scm_test.go | 29 +++++++++--- 6 files changed, 120 insertions(+), 45 deletions(-) diff --git a/cmd/pullrequest-init/main.go b/cmd/pullrequest-init/main.go index 5e23ff9a8eb..3f4db365dcb 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") + sslVerify = flag.Bool("sslVerify", true, "Enable/Disable SSL verification in the git client. Defaults to true") ) 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, *sslVerify) if err != nil { logger.Fatalf("error creating GitHub client: %v", err) } diff --git a/docs/resources.md b/docs/resources.md index 94943349134..0d165da11ec 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. `sslVerify`: represents whether or not the client should verify certificates + from the git server. Valid values are `"true"` or `"false"`, the default being + `"true"`. #### Statuses @@ -456,6 +459,7 @@ 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 `sslVerify` parameter to `"false"`. ```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..e113d30f9d9 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,33 @@ 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:"-"` + SSLVerify bool `json:"sslVerify"` } // 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, + SSLVerify: true, } for _, param := range r.Spec.Params { if strings.EqualFold(param.Name, "URL") { prResource.URL = param.Value } else if strings.EqualFold(param.Name, "Provider") { prResource.Provider = param.Value + } else if strings.EqualFold(param.Name, "SSLVerify") { + 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.SSLVerify = verify } } @@ -89,10 +98,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, + "sslVerify": strconv.FormatBool(s.SSLVerify), } } @@ -115,6 +125,9 @@ func (s *PullRequestResource) getSteps(mode string, sourcePath string) []Step { if s.Provider != "" { args = append(args, []string{"-provider", s.Provider}...) } + if !s.SSLVerify { + args = append(args, "-sslVerify=false") + } 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..4cae40e211a 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", + SSLVerify: true, } 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", + SSLVerify: true, }, 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", + SSLVerify: true, 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", + SSLVerify: false, + }, + 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, "-sslVerify=false"}, + Env: []corev1.EnvVar{}, + }}}, }} } diff --git a/pkg/pullrequest/scm.go b/pkg/pullrequest/scm.go index 0301beb8299..f8f12dabc55 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,13 @@ 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, sslVerify bool) (*Handler, error) { u, err := url.Parse(raw) if err != nil { return nil, err @@ -49,16 +49,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, sslVerify, logger) case "gitlab": - handler, err = gitlabHandlerFromURL(u, token, logger) + handler, err = gitlabHandlerFromURL(u, token, sslVerify, 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, sslVerify 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 +83,32 @@ 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{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: !sslVerify}, + }, + }, + } + } else { + client.Client = &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: !sslVerify}, + }, + } } + + 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, sslVerify 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 +139,24 @@ 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{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: !sslVerify}, + }, + }, + } + } else { + client.Client = &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: !sslVerify}, }, } } + return NewHandler(logger, client, project, prInt), nil } diff --git a/pkg/pullrequest/scm_test.go b/pkg/pullrequest/scm_test.go index 5aec293c334..5df3720d73f 100644 --- a/pkg/pullrequest/scm_test.go +++ b/pkg/pullrequest/scm_test.go @@ -17,17 +17,19 @@ limitations under the License. package pullrequest import ( - "net/url" - "testing" - "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" + "net/http" + "net/url" + "strconv" + "testing" ) func TestNewSCMHandler(t *testing.T) { tests := []struct { name string raw string + sslVerify bool wantBaseURL string wantRepo string wantNum int @@ -36,6 +38,7 @@ func TestNewSCMHandler(t *testing.T) { { name: "github", raw: "https://github.com/foo/bar/pull/1", + sslVerify: true, wantBaseURL: "https://api.github.com/", wantRepo: "foo/bar", wantNum: 1, @@ -44,6 +47,7 @@ func TestNewSCMHandler(t *testing.T) { { name: "custom github", raw: "https://github.tekton.dev/foo/baz/pull/2", + sslVerify: true, wantBaseURL: "https://github.tekton.dev/api/v3/", wantRepo: "foo/baz", wantNum: 2, @@ -52,6 +56,7 @@ func TestNewSCMHandler(t *testing.T) { { name: "gitlab", raw: "https://gitlab.com/foo/bar/merge_requests/3", + sslVerify: true, wantBaseURL: "https://gitlab.com/", wantRepo: "foo/bar", wantNum: 3, @@ -60,22 +65,24 @@ func TestNewSCMHandler(t *testing.T) { { name: "gitlab multi-level", raw: "https://gitlab.com/foo/bar/baz/merge_requests/3", + sslVerify: false, 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", + sslVerify: false, + 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.sslVerify) if err != nil { if !tt.wantErr { t.Errorf("NewSCMHandler() error = %v, wantErr %v", err, tt.wantErr) @@ -91,6 +98,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.sslVerify { + t.Errorf("NewSCMHandler() [InsecureSkipVerify] = %v, want %v", strconv.FormatBool(transport.TLSClientConfig.InsecureSkipVerify), !tt.sslVerify) + } + default: + t.Errorf("NewSCMHandler() client.Client.Transport was not of type http.Transport") + } }) } }