Adding create-webhook tekton task #82

akihikokuroda · 2019-08-26T23:43:12Z

This PR is for the issue #80.

This task configures components necessary for the github event processing.
The compoments are:

Selfsigned certificate for the Ingress
Ingress with TLS for the event listener
Github (or Github enterprize) webhook
Event listener

All componet configurations are optional. They can be truned off
by the parameter in the taskrun.

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Release Notes

Describe any user facing changes here, or delete this block.

Examples of user facing changes:
- API changes
- Bug fixes
- Any changes in behavior

akihikokuroda · 2019-08-26T23:44:07Z

I'll add readme file for this.

akihikokuroda · 2019-08-27T16:57:41Z

I added createwebhook.md.

akihikokuroda · 2019-08-27T17:03:37Z

/retest

bobcatfish

Wow! I love that you make a Task to do this, this is super cool :D ❤️

Can we add a link to the new docs somewhere, e.g. from the main README or maybe we can start an install guide (which we would also want to link to from the main README)
One great thing about this being a Task vs. just docs is that we can test it. I'd love to see automated tests to run this - if you don't want to add that @akihikokuroda could you add an issue to the backlog to add the tests?

bobcatfish · 2019-08-28T19:45:56Z

docs/create-webhook.yaml

+  volumes:
+  - name: github-secret
+    secret:
+      secretName: ${inputs.params.GithubSecretName} 


if we want this to work with > 0.6 we should make the ${} into $() instead

I'll change them.

bobcatfish · 2019-08-28T19:49:28Z

docs/create-webhook.yaml

+        exit 0
+      fi
+      apk update
+      apk add openssl


I'm curious why we want to update and install this here - is there another image we can use which would include these prerequisites already?

I looked for the image with the kubectl and the openssl but I couldn't find one so far.

When I tried this task on OpenShift, the apk didn't work with the default security context. So I looked for the images again and updated. Thanks.

Thanks @akihikokuroda !

(I've found the same thing - it's hard to choose the right image to use when you need to use 2 tools together - cuz usually an image will have one but not the other :S)

bobcatfish · 2019-08-28T19:55:44Z

docs/createwebhook.md

+  - value: triggertemplate CR instnace name
+- name: TriggerServiceAccount
+  This param is the service account name set in the event listener 
+  - value: service account name


just a formatting thing, looking at the rendered markdown, using `` around some of the text will make it visually a bit clearer, e.g.:

- name: `TriggerTemplate` This param has the trigger template set in the event listener - value: triggertemplate CR instance name

Yes, it makes the doc more readable. Thanks.

bobcatfish · 2019-08-28T19:55:50Z

docs/createwebhook.md

+  - value: triggerbinding CR instance name
+- name: TriggerTemplate
+  This param has the trigger template set in the event listener 
+  - value: triggertemplate CR instnace name


typo: instnace

I fix it. Thanks.

bobcatfish · 2019-08-28T19:56:33Z

docs/createwebhook.md

+
+The ingress exposes the event listener to the outside of the cluster.
+
+The followig task params must be set for the creation of the ingress.


typo: followig

I fix them. Thanks.

akihikokuroda · 2019-08-29T15:45:59Z

@bobcatfish Thanks for reviewing. I added a comment in the "getting started guide" #13 issue about putting the reference from it. I'll open a separate issue about the testing of the task in the backlog.

akihikokuroda · 2019-08-29T16:02:49Z

I created the issue for the automated test. #86

akihikokuroda · 2019-08-29T21:10:48Z

/hold

akihikokuroda · 2019-08-29T21:11:17Z

I have some issues running this on the OpenShift.

khrm · 2019-08-29T21:33:35Z

docs/create-webhook.yaml

+      apk add curl
+      echo "Create Webhook"
+      if [ $(inputs.params.GithubUrl) = "github.com" ]
+      then


Don't we need secret also for securing endpoint?

Yes, eventually. The handling of the github secret is not in the event listener yet. When it is implemented, the secret should be added.

This task configures components necessary for the github event processing. The compoments are: 1. Selfsigned certificate for the Ingress 2. Ingress with TLS for the event listener 3. Github (or Github enterprize) webhook 4. Event listener All componet configurations are optional. They can be truned off by the parameter in the taskrun.

akihikokuroda · 2019-08-30T17:05:29Z

/hold cancel

bobcatfish · 2019-08-30T17:34:53Z

Okay let's do it @akihikokuroda !

I think we should link to this from the main README too but not a big deal and we can always add it later :D

/lgtm
/meow space

tekton-robot · 2019-08-30T17:34:54Z

@bobcatfish:

In response to this:

Okay let's do it @akihikokuroda !

I think we should link to this from the main README too but not a big deal and we can always add it later :D

/lgtm
/meow space

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bobcatfish · 2019-08-30T17:37:32Z

hm still not a space cat

bobcatfish · 2019-08-30T17:38:07Z

bobcatfish · 2019-08-30T17:38:14Z

/approve

tekton-robot · 2019-08-30T17:38:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akihikokuroda, bobcatfish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [bobcatfish]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Removing myself from approvers/reviewers

tekton-robot requested review from iancoffey and vtereso August 26, 2019 23:43

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 26, 2019

akihikokuroda force-pushed the webhooktask branch from 2f77b23 to b187dce Compare August 27, 2019 16:56

akihikokuroda force-pushed the webhooktask branch from b187dce to a0ada06 Compare August 27, 2019 17:00

akihikokuroda force-pushed the webhooktask branch from a0ada06 to 81831e5 Compare August 27, 2019 17:07

bobcatfish reviewed Aug 28, 2019

View reviewed changes

akihikokuroda mentioned this pull request Aug 28, 2019

Create getting started guide #13

Closed

akihikokuroda force-pushed the webhooktask branch from 81831e5 to e2dff45 Compare August 29, 2019 15:57

akihikokuroda mentioned this pull request Aug 29, 2019

Create automated test for create webhook task #86

Closed

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 29, 2019

khrm reviewed Aug 29, 2019

View reviewed changes

akihikokuroda force-pushed the webhooktask branch from e2dff45 to 0103d76 Compare August 30, 2019 16:59

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2019

tekton-robot assigned bobcatfish Aug 30, 2019

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 30, 2019

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 30, 2019

tekton-robot merged commit e80e8f3 into tektoncd:master Aug 30, 2019

akihikokuroda deleted the webhooktask branch August 30, 2019 18:17

piyush-garg pushed a commit to piyush-garg/triggers that referenced this pull request Mar 24, 2020

Merge pull request tektoncd#82 from bbrowning/patch-1

c177886

Removing myself from approvers/reviewers

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding create-webhook tekton task #82

Adding create-webhook tekton task #82

akihikokuroda commented Aug 26, 2019 •

edited

Loading

akihikokuroda commented Aug 26, 2019

akihikokuroda commented Aug 27, 2019

akihikokuroda commented Aug 27, 2019

bobcatfish left a comment

bobcatfish Aug 28, 2019

akihikokuroda Aug 29, 2019

bobcatfish Aug 28, 2019

akihikokuroda Aug 29, 2019

akihikokuroda Aug 30, 2019

bobcatfish Aug 30, 2019

bobcatfish Aug 28, 2019

akihikokuroda Aug 29, 2019

bobcatfish Aug 28, 2019

akihikokuroda Aug 29, 2019

bobcatfish Aug 28, 2019

akihikokuroda Aug 29, 2019

akihikokuroda commented Aug 29, 2019

akihikokuroda commented Aug 29, 2019

akihikokuroda commented Aug 29, 2019

akihikokuroda commented Aug 29, 2019

khrm Aug 29, 2019

akihikokuroda Aug 30, 2019

akihikokuroda commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

tekton-robot commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

tekton-robot commented Aug 30, 2019


		The ingress exposes the event listener to the outside of the cluster.

		The followig task params must be set for the creation of the ingress.

Adding create-webhook tekton task #82

Adding create-webhook tekton task #82

Conversation

akihikokuroda commented Aug 26, 2019 • edited Loading

Changes

Submitter Checklist

Release Notes

akihikokuroda commented Aug 26, 2019

akihikokuroda commented Aug 27, 2019

akihikokuroda commented Aug 27, 2019

bobcatfish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akihikokuroda commented Aug 29, 2019

akihikokuroda commented Aug 29, 2019

akihikokuroda commented Aug 29, 2019

akihikokuroda commented Aug 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akihikokuroda commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

tekton-robot commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

bobcatfish commented Aug 30, 2019

tekton-robot commented Aug 30, 2019

akihikokuroda commented Aug 26, 2019 •

edited

Loading