From b0c3441fff18e8cac4ca1746994e3a2748e9744c Mon Sep 17 00:00:00 2001 From: Billy Lynch Date: Mon, 3 Jun 2019 20:46:01 -0400 Subject: [PATCH] Added additional documentation and testing for PullRequest CRD. --- cmd/pullrequest-init/README.md | 22 +- cmd/pullrequest-init/github.go | 2 +- docs/resources.md | 287 ++++++++++-------- .../v1alpha1/pull_request_resource.go | 64 ++-- .../v1alpha1/pull_request_resource_test.go | 149 +++++++++ 5 files changed, 354 insertions(+), 170 deletions(-) create mode 100644 pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go diff --git a/cmd/pullrequest-init/README.md b/cmd/pullrequest-init/README.md index da3ca9cf270..2f66b007d32 100644 --- a/cmd/pullrequest-init/README.md +++ b/cmd/pullrequest-init/README.md @@ -8,14 +8,14 @@ provider specific payloads. Currently supported providers: -* GitHub +* GitHub ## Generic pull request payload -The payload generated by default aims to abstract the base features of any -pull request provider. Since there is no one true spec for pull requests, not -every feature may be available. The payload is output to `$PATH/pr.json` and -looks like: +The payload generated by default aims to abstract the base features of any pull +request provider. Since there is no one true spec for pull requests, not every +feature may be available. The payload is output to `$PATH/pr.json` and looks +like: ```json { @@ -52,7 +52,11 @@ looks like: GitHub pull requests will output these additional files: -* `$PATH/github/pr.json`: The raw GitHub payload as specified by - https://developer.github.com/v3/pulls/#get-a-single-pull-request -* `$PATH/github/comments/#.json`: Comments associated to the PR as specified by - https://developer.github.com/v3/issues/comments/#get-a-single-comment +* `$PATH/github/pr.json`: The raw GitHub payload as specified by + https://developer.github.com/v3/pulls/#get-a-single-pull-request +* `$PATH/github/comments/#.json`: Comments associated to the PR as specified + by https://developer.github.com/v3/issues/comments/#get-a-single-comment + +The binary will look for GitHub credentials in the `${GITHUBTOKEN}` environment +variable. This should generally be specified as a secret with the field name +`githubToken` in the `PullRequestResource` definition. diff --git a/cmd/pullrequest-init/github.go b/cmd/pullrequest-init/github.go index f67331b0075..7ba11d4d853 100644 --- a/cmd/pullrequest-init/github.go +++ b/cmd/pullrequest-init/github.go @@ -30,7 +30,7 @@ type GitHubHandler struct { // NewGitHubHandler initializes a new handler for interacting with GitHub // resources. func NewGitHubHandler(ctx context.Context, logger *zap.SugaredLogger, rawURL string) (*GitHubHandler, error) { - token := strings.TrimSpace(os.Getenv("GITHUBOAUTHTOKEN")) + token := strings.TrimSpace(os.Getenv("GITHUBTOKEN")) var hc *http.Client if token != "" { ts := oauth2.StaticTokenSource( diff --git a/docs/resources.md b/docs/resources.md index a6a65c91e3c..2bb5b5503ef 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -7,36 +7,37 @@ A `Task` can have multiple inputs and outputs. For example: -- A `Task`'s input could be a GitHub source which contains your application - code. -- A `Task`'s output can be your application container image which can be then - deployed in a cluster. -- A `Task`'s output can be a jar file to be uploaded to a storage bucket. +- A `Task`'s input could be a GitHub source which contains your application + code. +- A `Task`'s output can be your application container image which can be then + deployed in a cluster. +- A `Task`'s output can be a jar file to be uploaded to a storage bucket. ---- +-------------------------------------------------------------------------------- -- [Syntax](#syntax) -- [Resource types](#resource-types) -- [Examples](#examples) +- [Syntax](#syntax) +- [Resource types](#resource-types) +- [Examples](#examples) ## Syntax To define a configuration file for a `PipelineResource`, you can specify the following fields: -- Required: - - [`apiVersion`][kubernetes-overview] - Specifies the API version, for example - `tekton.dev/v1alpha1`. - - [`kind`][kubernetes-overview] - Specify the `PipelineResource` resource - object. - - [`metadata`][kubernetes-overview] - Specifies data to uniquely identify the - `PipelineResource` object, for example a `name`. - - [`spec`][kubernetes-overview] - Specifies the configuration information for - your `PipelineResource` resource object. - - [`type`](#resource-types) - Specifies the `type` of the `PipelineResource` -- Optional: - - [`params`](#resource-types) - Parameters which are specific to each type of - `PipelineResource` +- Required: + - [`apiVersion`][kubernetes-overview] - Specifies the API version, for + example `tekton.dev/v1alpha1`. + - [`kind`][kubernetes-overview] - Specify the `PipelineResource` resource + object. + - [`metadata`][kubernetes-overview] - Specifies data to uniquely identify + the `PipelineResource` object, for example a `name`. + - [`spec`][kubernetes-overview] - Specifies the configuration information + for your `PipelineResource` resource object. + - [`type`](#resource-types) - Specifies the `type` of the + `PipelineResource` +- Optional: + - [`params`](#resource-types) - Parameters which are specific to each type + of `PipelineResource` [kubernetes-overview]: https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields @@ -45,12 +46,12 @@ following fields: The following `PipelineResources` are currently supported: -- [Git Resource](#git-resource) -- [Image Resource](#image-resource) -- [Cluster Resource](#cluster-resource) -- [Storage Resource](#storage-resource) - - [GCS Storage Resource](#gcs-storage-resource) - - [BuildGCS Storage Resource](#buildgcs-storage-resource) +- [Git Resource](#git-resource) +- [Image Resource](#image-resource) +- [Cluster Resource](#cluster-resource) +- [Storage Resource](#storage-resource) + - [GCS Storage Resource](#gcs-storage-resource) + - [BuildGCS Storage Resource](#buildgcs-storage-resource) ### Git Resource @@ -78,13 +79,13 @@ spec: Params that can be added are the following: -1. `url`: represents the location of the git repository, you can use this to - change the repo, e.g. [to use a fork](#using-a-fork) -1. `revision`: Git - [revision](https://git-scm.com/docs/gitrevisions#_specifying_revisions) - (branch, tag, commit SHA or ref) to clone. You can use this to control what - commit [or branch](#using-a-branch) is used. _If no revision is specified, - the resource will default to `latest` from `master`._ +1. `url`: represents the location of the git repository, you can use this to + change the repo, e.g. [to use a fork](#using-a-fork) +1. `revision`: Git + [revision](https://git-scm.com/docs/gitrevisions#_specifying_revisions) + (branch, tag, commit SHA or ref) to clone. You can use this to control what + commit [or branch](#using-a-branch) is used. _If no revision is specified, + the resource will default to `latest` from `master`._ #### Using a fork @@ -129,6 +130,52 @@ spec: value: refs/pull/52525/head ``` +### Pull Request Resource + +Pull Request resource represents a pull request event from a source control +system. + +Adding the Pull Request resource as an input to a Task will populate the +workspace with a JSON payload containing generic pull request related metadata +such as base/head commit, comments, and labels. The payloads will also contain +links to raw service-specific payloads where appropriate. + +Adding the Pull Request resource as an output of a Task will update the source +control system with any changes made to the pull request resource during the +pipeline. + +See [types.go](cmd/pullrequest-init/types.go) for the full payload spec. + +To create a pull request resource using the `PipelineResource` CRD: + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: PipelineResource +metadata: + name: wizzbang-pr + namespace: default +spec: + type: pullRequest + params: + - name: url + value: https://github.com/wizzbangcorp/wizzbang/pulls/1 + secrets: + - fieldName: githubToken + secretName: github-secrets + secretKey: token +``` + +Params that can be added are the following: + +1. `url`: represents the location of the pull request to fetch. + +#### GitHub + +The Pull Request resource will look for GitHub OAuth authentication tokens in +spec secrets with a field name called `githubToken`. + +URLs should be of the form: https://github.com/tektoncd/pipelines/pulls/1 + ### Image Resource An Image resource represents an image that lives in a remote repository. It is @@ -138,14 +185,14 @@ registry. Params that can be added are the following: -1. `url`: The complete path to the image, including the registry and the image - tag -1. `digest`: The - [image digest](https://success.docker.com/article/images-tagging-vs-digests) - which uniquely identifies a particular build of an image with a particular - tag. _While this can be provided as a parameter, there is not yet a way to - update this value after an image is built, but this is planned in - [#216](https://github.com/tektoncd/pipeline/issues/216)._ +1. `url`: The complete path to the image, including the registry and the image + tag +1. `digest`: The + [image digest](https://success.docker.com/article/images-tagging-vs-digests) + which uniquely identifies a particular build of an image with a particular + tag. _While this can be provided as a parameter, there is not yet a way to + update this value after an image is built, but this is planned in + [#216](https://github.com/tektoncd/pipeline/issues/216)._ For example: @@ -230,17 +277,17 @@ cluster. The kubeconfig will be placed in The Cluster resource has the following parameters: -- `name` (required): The name to be given to the target cluster, will be used in - the kubeconfig and also as part of the path to the kubeconfig file -- `url` (required): Host url of the master node -- `username` (required): the user with access to the cluster -- `password`: to be used for clusters with basic auth -- `token`: to be used for authentication, if present will be used ahead of the - password -- `insecure`: to indicate server should be accessed without verifying the TLS - certificate. -- `cadata` (required): holds PEM-encoded bytes (typically read from a root - certificates bundle). +- `name` (required): The name to be given to the target cluster, will be used + in the kubeconfig and also as part of the path to the kubeconfig file +- `url` (required): Host url of the master node +- `username` (required): the user with access to the cluster +- `password`: to be used for clusters with basic auth +- `token`: to be used for authentication, if present will be used ahead of the + password +- `insecure`: to indicate server should be accessed without verifying the TLS + certificate. +- `cadata` (required): holds PEM-encoded bytes (typically read from a root + certificates bundle). Note: Since only one authentication technique is allowed per user, either a `token` or a `password` should be provided, if both are provided, the `password` @@ -369,18 +416,18 @@ spec: Params that can be added are the following: -1. `location`: represents the location of the blob storage. -1. `type`: represents the type of blob storage. For GCS storage resource this - value should be set to `gcs`. -1. `dir`: represents whether the blob storage is a directory or not. By default - storage artifact is considered not a directory. +1. `location`: represents the location of the blob storage. +1. `type`: represents the type of blob storage. For GCS storage resource this + value should be set to `gcs`. +1. `dir`: represents whether the blob storage is a directory or not. By default + storage artifact is considered not a directory. - - If artifact is a directory then `-r`(recursive) flag is used to copy all - files under source directory to GCS bucket. Eg: - `gsutil cp -r source_dir/* gs://some-bucket` - - If artifact is a single file like zip, tar files then copy will be only 1 - level deep(no recursive). It will not trigger copy of sub directories in - source directory. Eg: `gsutil cp source.tar gs://some-bucket.tar`. + - If artifact is a directory then `-r`(recursive) flag is used to copy all + files under source directory to GCS bucket. Eg: `gsutil cp -r + source_dir/* gs://some-bucket` + - If artifact is a single file like zip, tar files then copy will be only + 1 level deep(no recursive). It will not trigger copy of sub directories + in source directory. Eg: `gsutil cp source.tar gs://some-bucket.tar`. Private buckets can also be configured as storage resources. To access GCS private buckets, service accounts are required with correct permissions. The @@ -388,43 +435,43 @@ private buckets, service accounts are required with correct permissions. The information. Below is an example on how to create a storage resource with service account. -1. Refer to - [official documentation](https://cloud.google.com/compute/docs/access/service-accounts) - on how to create service accounts and configuring IAM permissions to access - bucket. -1. Create a Kubernetes secret from downloaded service account json key - - ```bash - kubectl create secret generic bucket-sa --from-file=./service_account.json - ``` - -1. To access GCS private bucket environment variable - [`GOOGLE_APPLICATION_CREDENTIALS`](https://cloud.google.com/docs/authentication/production) - should be set so apply above created secret to the GCS storage resource under - `fieldName` key. - - ```yaml - apiVersion: tekton.dev/v1alpha1 - kind: PipelineResource - metadata: - name: wizzbang-storage - namespace: default - spec: - type: storage - params: - - name: type - value: gcs - - name: location - value: gs://some-private-bucket - - name: dir - value: "y" - secrets: - - fieldName: GOOGLE_APPLICATION_CREDENTIALS - secretName: bucket-sa - secretKey: service_account.json - ``` - ---- +1. Refer to + [official documentation](https://cloud.google.com/compute/docs/access/service-accounts) + on how to create service accounts and configuring IAM permissions to access + bucket. +1. Create a Kubernetes secret from downloaded service account json key + + ```bash + kubectl create secret generic bucket-sa --from-file=./service_account.json + ``` + +1. To access GCS private bucket environment variable + [`GOOGLE_APPLICATION_CREDENTIALS`](https://cloud.google.com/docs/authentication/production) + should be set so apply above created secret to the GCS storage resource + under `fieldName` key. + + ```yaml + apiVersion: tekton.dev/v1alpha1 + kind: PipelineResource + metadata: + name: wizzbang-storage + namespace: default + spec: + type: storage + params: + - name: type + value: gcs + - name: location + value: gs://some-private-bucket + - name: dir + value: "y" + secrets: + - fieldName: GOOGLE_APPLICATION_CREDENTIALS + secretName: bucket-sa + secretKey: service_account.json + ``` + +-------------------------------------------------------------------------------- #### BuildGCS Storage Resource @@ -463,24 +510,24 @@ spec: Params that can be added are the following: -1. `location`: represents the location of the blob storage. -1. `type`: represents the type of blob storage. For BuildGCS, this value should - be set to `build-gcs` -1. `artifactType`: represent the type of GCS resource. Right now, we support - following types: - - - `Archive`: - - Archive indicates that resource fetched is an archive file. Currently, - Build GCS resource only supports `.zip` archive. - - It unzips the archive and places all the files in the directory which is - set at runtime. - - If `artifactType` is set to `Archive`, `location` should point to a - `.zip` file. - - [`Manifest`](https://github.com/GoogleCloudPlatform/cloud-builders/tree/master/gcs-fetcher#source-manifests): - - Manifest indicates that resource should be fetched using a source - manifest file. - - If `artifactType` is set to `Manifest`, `location` should point to a - source manifest file. +1. `location`: represents the location of the blob storage. +1. `type`: represents the type of blob storage. For BuildGCS, this value should + be set to `build-gcs` +1. `artifactType`: represent the type of GCS resource. Right now, we support + following types: + + - `Archive`: + - Archive indicates that resource fetched is an archive file. + Currently, Build GCS resource only supports `.zip` archive. + - It unzips the archive and places all the files in the directory + which is set at runtime. + - If `artifactType` is set to `Archive`, `location` should point to a + `.zip` file. + - [`Manifest`](https://github.com/GoogleCloudPlatform/cloud-builders/tree/master/gcs-fetcher#source-manifests): + - Manifest indicates that resource should be fetched using a source + manifest file. + - If `artifactType` is set to `Manifest`, `location` should point to a + source manifest file. Private buckets other than ones accessible by [TaskRun Service Account](./taskruns.md#service-account) can not be configured diff --git a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go index 1be5d6b8b8a..d3a4f73480b 100644 --- a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Knative Authors. +Copyright 2019 The Tekton Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -25,8 +25,12 @@ import ( corev1 "k8s.io/api/core/v1" ) +const ( + prSource = "pr-source" + githubTokenEnv = "githubToken" +) + var ( - prSource = "pr-source" // The container that we use to implement the PR source step. prImage = flag.String("pr-image", "override-with-pr:latest", "The container image containing our PR binary.") @@ -35,11 +39,14 @@ var ( // PullRequestResource is an endpoint from which to get data which is required // by a Build/Task for context. type PullRequestResource struct { - Name string `json:"name"` - Type PipelineResourceType `json:"type"` - URL string `json:"url"` - GithubOauthToken string `json:"githubOauthToken"` - //Secrets holds a struct to indicate a field name and corresponding secret name to populate it + Name string `json:"name"` + Type PipelineResourceType `json:"type"` + + DestinationDir string `json:"destinationDir"` + // GitHub URL pointing to the pull request. + // Example: https://github.com/owner/repo/pulls/1 + URL string `json:"url"` + // Secrets holds a struct to indicate a field name and corresponding secret name to populate it. Secrets []SecretParam `json:"secrets"` } @@ -91,45 +98,20 @@ func (s *PullRequestResource) Replacements() map[string]string { } func (s *PullRequestResource) GetDownloadContainerSpec() ([]corev1.Container, error) { - args := []string{"-url", s.URL, "-path", s.Name, "-mode", "download"} - - evs := []corev1.EnvVar{} - for _, sec := range s.Secrets { - switch { - case strings.EqualFold(sec.FieldName, "githubOauthToken"): - fmt.Println("ADDING ENV VAR") - ev := corev1.EnvVar{ - Name: strings.ToUpper(sec.FieldName), - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: sec.SecretName, - }, - Key: sec.SecretKey, - }, - }, - } - evs = append(evs, ev) - } - } - - return []corev1.Container{{ - Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(prSource + "-" + s.Name), - Image: *prImage, - Command: []string{"/ko-app/pullrequest-init"}, - Args: args, - WorkingDir: workspaceDir, - Env: evs, - }}, nil + return s.getContainerSpec("download") } func (s *PullRequestResource) GetUploadContainerSpec() ([]corev1.Container, error) { - args := []string{"-url", s.URL, "-path", s.Name, "-mode", "upload"} + return s.getContainerSpec("upload") +} + +func (s *PullRequestResource) getContainerSpec(mode string) ([]corev1.Container, error) { + args := []string{"-url", s.URL, "-path", s.DestinationDir, "-mode", mode} evs := []corev1.EnvVar{} for _, sec := range s.Secrets { switch { - case strings.EqualFold(sec.FieldName, "githubOauthToken"): + case strings.EqualFold(sec.FieldName, githubTokenEnv): ev := corev1.EnvVar{ Name: strings.ToUpper(sec.FieldName), ValueFrom: &corev1.EnvVarSource{ @@ -156,4 +138,6 @@ func (s *PullRequestResource) GetUploadContainerSpec() ([]corev1.Container, erro } // SetDestinationDirectory sets the destination directory at runtime like where is the resource going to be copied to -func (s *PullRequestResource) SetDestinationDirectory(destDir string) {} +func (s *PullRequestResource) SetDestinationDirectory(dir string) { + s.DestinationDir = dir +} diff --git a/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go new file mode 100644 index 00000000000..9f08ce2ccd9 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go @@ -0,0 +1,149 @@ +package v1alpha1 + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/test/names" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestPullRequest_NewResource(t *testing.T) { + url := "https://github.com/tektoncd/pipeline/pulls/1" + pr := &PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: PipelineResourceSpec{ + Type: PipelineResourceTypePullRequest, + Params: []Param{{ + Name: "type", + Value: "github", + }, { + Name: "url", + Value: url, + }}, + SecretParams: []SecretParam{{ + FieldName: "githubToken", + SecretKey: "test-secret-key", + SecretName: "test-secret-name", + }}, + }, + } + got, err := NewPullRequestResource(pr) + if err != nil { + t.Fatalf("Error creating storage resource: %s", err.Error()) + } + + want := &PullRequestResource{ + Name: pr.Name, + Type: PipelineResourceTypePullRequest, + URL: url, + Secrets: pr.Spec.SecretParams, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Error(diff) + } +} + +func TestPullRequest_NewResource_error(t *testing.T) { + pr := &PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: PipelineResourceSpec{ + Type: PipelineResourceTypeGit, + }, + } + if _, err := NewPullRequestResource(pr); err == nil { + t.Error("NewPullRequestResource() want error, got nil") + } +} + +type testcase struct { + in *PullRequestResource + out []corev1.Container +} + +func containerTestCases(mode string) []testcase { + return []testcase{ + { + in: &PullRequestResource{ + Name: "nocreds", + DestinationDir: "/workspace", + URL: "https://example.com", + }, + out: []corev1.Container{{ + Name: "pr-source-nocreds-9l9zj", + Image: "override-with-pr:latest", + WorkingDir: workspaceDir, + Command: []string{"/ko-app/pullrequest-init"}, + Args: []string{"-url", "https://example.com", "-path", "/workspace", "-mode", mode}, + Env: []corev1.EnvVar{}, + }}, + }, + { + in: &PullRequestResource{ + Name: "creds", + DestinationDir: "/workspace", + URL: "https://example.com", + Secrets: []SecretParam{{ + FieldName: "githubToken", + SecretName: "github-creds", + SecretKey: "token", + }}, + }, + out: []corev1.Container{{ + Name: "pr-source-creds-mz4c7", + Image: "override-with-pr:latest", + WorkingDir: workspaceDir, + Command: []string{"/ko-app/pullrequest-init"}, + Args: []string{"-url", "https://example.com", "-path", "/workspace", "-mode", mode}, + Env: []corev1.EnvVar{{ + Name: "GITHUBTOKEN", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "github-creds", + }, + Key: "token", + }, + }, + }}, + }}, + }, + } +} + +func TestPullRequest_GetDownloadContainerSpec(t *testing.T) { + names.TestingSeed() + + for _, tc := range containerTestCases("download") { + t.Run(tc.in.GetName(), func(t *testing.T) { + got, err := tc.in.GetDownloadContainerSpec() + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tc.out, got); diff != "" { + t.Error(diff) + } + }) + } +} + +func TestPullRequest_GetUploadContainerSpec(t *testing.T) { + names.TestingSeed() + + for _, tc := range containerTestCases("upload") { + t.Run(tc.in.GetName(), func(t *testing.T) { + got, err := tc.in.GetUploadContainerSpec() + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tc.out, got); diff != "" { + t.Error(diff) + } + }) + } +}