Skip to content

Commit

Permalink
Issue 1880: Adds the ability to disable the certificate validation in…
Browse files Browse the repository at this point in the history 
… 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.
  • Loading branch information
@dibbles
dibbles committed Jan 16, 2020
1 parent 5e7f2ac commit 41e7b24
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 44 deletions.
14 changes: 7 additions & 7 deletions cmd/pullrequest-init/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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)
}
Expand Down
4 changes: 4 additions & 0 deletions docs/resources.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
38 changes: 26 additions & 12 deletions pkg/apis/pipeline/v1alpha1/pull_request_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"fmt"
"strconv"
"strings"

"github.com/tektoncd/pipeline/pkg/apis/pipeline"
Expand Down Expand Up @@ -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:"-"`
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") {
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, "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
}
}

Expand All @@ -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,
"sslVerify": strconv.FormatBool(s.SSLVerify),
}
}

Expand All @@ -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.SSLVerify {
args = append(args, "-sslVerify=false")
}

evs := []corev1.EnvVar{}
for _, sec := range s.Secrets {
Expand Down
40 changes: 29 additions & 11 deletions pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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{},
}}},
}}
}

Expand Down
50 changes: 40 additions & 10 deletions pkg/pullrequest/scm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package pullrequest

import (
"context"
"fmt"
"net/http"
"net/url"
Expand All @@ -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, sslVerify bool) (*Handler, error) {
u, err := url.Parse(raw)
if err != nil {
return nil, err
Expand All @@ -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, 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)
Expand All @@ -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: !sslVerify},
},
},
}
} else {
client.Client = &http.Client{
Transport: &http.Transport{
/* #nosec G402 */
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, "/")

Expand Down Expand Up @@ -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: !sslVerify},
},
},
}
} else {
client.Client = &http.Client{
Transport: &http.Transport{
/* #nosec G402 */
TLSClientConfig: &tls.Config{InsecureSkipVerify: !sslVerify},
},
}
}

return NewHandler(logger, client, project, prInt), nil
}

Expand Down
24 changes: 20 additions & 4 deletions pkg/pullrequest/scm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package pullrequest

import (
"net/http"
"net/url"
"strconv"
"testing"

"go.uber.org/zap"
Expand All @@ -28,6 +30,7 @@ func TestNewSCMHandler(t *testing.T) {
tests := []struct {
name string
raw string
sslVerify bool
wantBaseURL string
wantRepo string
wantNum int
Expand All @@ -36,6 +39,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,
Expand All @@ -44,6 +48,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,
Expand All @@ -52,6 +57,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,
Expand All @@ -60,22 +66,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)
Expand All @@ -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.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")
}
})
}
}
Expand Down

0 comments on commit 41e7b24

Please sign in to comment.