[pipeline/trusted-resources]Add admission webhook #841

Yongxuanzhang · 2022-03-02T20:48:57Z

Changes

This commit is a followup work for Trust Task.(Forked from tektoncd/community#537)
Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup.
This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.

Submitter Checklist

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

Includes docs (if user facing)
Commit messages follow commit message best practices
Commit messages includes a project tag in the subject - e.g. [OCI], [hub], [results], [task-loops]

See the contribution guide
for more details.

Yongxuanzhang · 2022-03-03T19:01:07Z

/assign @wlynch

Yongxuanzhang · 2022-03-03T19:39:33Z

/test pull-tekton-experimental-build-tests

pipeline/trusted-resources/.gitignore

pipeline/trusted-resources/cmd/webhook_trusted_resource/main.go

wlynch · 2022-03-03T17:46:27Z

pipeline/trusted-resources/config/200-role.yaml

+  - apiGroups: ["coordination.k8s.io"]
+    resources: ["leases"]
+    verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]


Is it possible to scope down these permissions to particular resources?

Not sure if makes sense, I create another role and rolebinding with different names than that in pipeline

pipeline/trusted-resources/config/200-role.yaml

pipeline/trusted-resources/config/config-feature-flags.yaml

pipeline/trusted-resources/config/webhook-trusted-resources.yaml

wlynch · 2022-03-03T19:39:54Z

pipeline/trusted-resources/examples/1-test-taskrun.yaml

+apiVersion: tekton.dev/v1beta1
+metadata:
+  annotations:
+      tekton.dev/signature: MEYCIQDhKiJpPylEFo5RmBEZV96luADJdhSQcE7EZuOgL7hk8wIhAICKk6o5ldqo2sN0R6GhE7GpuEkblbjeJilNms78E+ap


How are these generated? (e.g. if I modify this or want to create another, what do I need to do?)

This is just an example showing how to use taskrun with this webhook deployed (also for demo and manually tetsting).
I'm not sure if we need to include the signing code just for generating the signatures? I can add them into a folder like etc/...

Or should I just comment out these signatures and add some comments?

It's fine to have the signatures here - the missing piece is how these were generated.

At minimum, we should have a readme to document how to generate these. If you want to include a tool to make this easier that works to!

wlynch · 2022-03-03T19:54:09Z

pipeline/trusted-resources/cmd/webhook_trusted_resource/main.go

+	// Set up a signal context with our webhook options
+	ctx = webhook.WithOptions(ctx, webhook.Options{
+		ServiceName: serviceName,
+		Port:        8443,


Part of me wonders how difficult it would be to have a local test that stands up a local instance and verifies this works properly with client-go 🤔 maybe this is just something we save for a kind test though?

If it's easy enough to do, can we add one? Otherwise let's make sure to add a kind test in another PR.

As discussed this should be e2e/integration tests? I will see if I can add them in another pr.

wlynch

lgtm! one last thing

pipeline/trusted-resources/config/config-feature-flags.yaml

Yongxuanzhang · 2022-03-07T15:55:47Z

lgtm! one last thing

Ah I see! It is just used for the examples testing, I will also comment out and document it.

wlynch · 2022-03-07T16:31:21Z

Ah I see! It is just used for the examples testing, I will also comment out and document it.

Not just for testing! 😄
We're also using the examples in our README for how to get started (which is a great thing to do). Since we aren't going into detail for how the signatures are created, it's hard to follow how I as a user can create a signature for my own Task.
I'd recommend adding this documentation to the readme.

Yongxuanzhang · 2022-03-07T17:17:56Z

Ah I see! It is just used for the examples testing, I will also comment out and document it.

Not just for testing! 😄 We're also using the examples in our README for how to get started (which is a great thing to do). Since we aren't going into detail for how the signatures are created, it's hard to follow how I as a user can create a signature for my own Task. I'd recommend adding this documentation to the readme.

adding some code to generate signature from yaml files in another pr

Yongxuanzhang · 2022-03-07T20:05:47Z

/test tekton-experimental-unit-tests

This commit is a followup work for Trust Task.(Forked from tektoncd/community#537) Prior this commit we have added the verification of taskspec but didn’t include the webhook configuration and setup. This commit includes the configuration and setup of the admission webhook and related docs.With this commit we can deploy a standalone webhookfor verification.

wlynch

/lgtm

tekton-robot · 2022-03-07T23:26:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wlynch

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:

~~pipeline/trusted-resources/OWNERS~~ [wlynch]

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

tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 2, 2022

tekton-robot requested review from bobcatfish and wlynch March 2, 2022 20:49

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 2, 2022

Yongxuanzhang force-pushed the trusted-resources-webhook branch from 3565ec7 to c7ba8d2 Compare March 3, 2022 17:05

Yongxuanzhang marked this pull request as ready for review March 3, 2022 17:05

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2022

Yongxuanzhang force-pushed the trusted-resources-webhook branch from c7ba8d2 to d0743d4 Compare March 3, 2022 18:56

tekton-robot assigned wlynch Mar 3, 2022

wlynch reviewed Mar 3, 2022

View reviewed changes

Yongxuanzhang requested a review from wlynch March 4, 2022 16:01

Yongxuanzhang force-pushed the trusted-resources-webhook branch from 0c15e99 to 78ecbb3 Compare March 7, 2022 15:05

wlynch reviewed Mar 7, 2022

View reviewed changes

pipeline/trusted-resources/config/config-feature-flags.yaml Outdated Show resolved Hide resolved

Yongxuanzhang force-pushed the trusted-resources-webhook branch from 0709034 to 4c07c10 Compare March 7, 2022 15:59

Yongxuanzhang requested a review from wlynch March 7, 2022 16:07

Yongxuanzhang force-pushed the trusted-resources-webhook branch 5 times, most recently from d29efc4 to 86a312b Compare March 7, 2022 19:56

Yongxuanzhang force-pushed the trusted-resources-webhook branch 2 times, most recently from 3234639 to 6e4b1a9 Compare March 7, 2022 22:25

Yongxuanzhang force-pushed the trusted-resources-webhook branch from 6e4b1a9 to fe13351 Compare March 7, 2022 22:30

wlynch approved these changes Mar 7, 2022

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 7, 2022

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2022

tekton-robot merged commit ef53772 into tektoncd:main Mar 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pipeline/trusted-resources]Add admission webhook #841

[pipeline/trusted-resources]Add admission webhook #841

Yongxuanzhang commented Mar 2, 2022 •

edited

Loading

Yongxuanzhang commented Mar 3, 2022

Yongxuanzhang commented Mar 3, 2022

wlynch Mar 3, 2022

Yongxuanzhang Mar 3, 2022

wlynch Mar 3, 2022

Yongxuanzhang Mar 3, 2022

Yongxuanzhang Mar 7, 2022 •

edited

Loading

wlynch Mar 7, 2022

wlynch Mar 3, 2022

Yongxuanzhang Mar 3, 2022

wlynch left a comment

Yongxuanzhang commented Mar 7, 2022

wlynch commented Mar 7, 2022

Yongxuanzhang commented Mar 7, 2022 •

edited

Loading

Yongxuanzhang commented Mar 7, 2022

wlynch left a comment

tekton-robot commented Mar 7, 2022

[pipeline/trusted-resources]Add admission webhook #841

[pipeline/trusted-resources]Add admission webhook #841

Conversation

Yongxuanzhang commented Mar 2, 2022 • edited Loading

Changes

Submitter Checklist

Yongxuanzhang commented Mar 3, 2022

Yongxuanzhang commented Mar 3, 2022

wlynch Mar 3, 2022

Choose a reason for hiding this comment

Yongxuanzhang Mar 3, 2022

Choose a reason for hiding this comment

wlynch Mar 3, 2022

Choose a reason for hiding this comment

Yongxuanzhang Mar 3, 2022

Choose a reason for hiding this comment

Yongxuanzhang Mar 7, 2022 • edited Loading

Choose a reason for hiding this comment

wlynch Mar 7, 2022

Choose a reason for hiding this comment

wlynch Mar 3, 2022

Choose a reason for hiding this comment

Yongxuanzhang Mar 3, 2022

Choose a reason for hiding this comment

wlynch left a comment

Choose a reason for hiding this comment

Yongxuanzhang commented Mar 7, 2022

wlynch commented Mar 7, 2022

Yongxuanzhang commented Mar 7, 2022 • edited Loading

Yongxuanzhang commented Mar 7, 2022

wlynch left a comment

Choose a reason for hiding this comment

tekton-robot commented Mar 7, 2022

Yongxuanzhang commented Mar 2, 2022 •

edited

Loading

Yongxuanzhang Mar 7, 2022 •

edited

Loading

Yongxuanzhang commented Mar 7, 2022 •

edited

Loading