Add support for experimental hermetic execution mode to TaskRuns #3956

priyawadhwa · 2021-05-18T00:29:22Z

Changes

This PR adds support for an experimental hermetic execution mode. If users specify this on their TaskRun, then all user step containers are run without network access. Any containers created or injected by tekton are not affected, and sidecar containers are not affected (since they aren't controlled by the entrypointer, we can't make them hermetic as of now).

This PR also adds an integration test to make sure that network access isn't available when hermetic mode is enabled!

TEP: https://github.com/tektoncd/community/blob/main/teps/0025-hermekton.md

/kind feature

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Docs included if any changes are user facing
Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

Add support for experimental hermetic execution mode to TaskRuns

Add support for experimental hermetic execution mode to TaskRuns

cc @dlorenc

tekton-robot · 2021-05-18T00:31:13Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	73.3%	-10.7
pkg/pod/pod.go	86.6%	84.5%	-2.1

tekton-robot · 2021-05-18T01:07:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	73.3%	-10.7
pkg/pod/pod.go	86.6%	84.5%	-2.1

cmd/entrypoint/runner.go

pkg/apis/pipeline/v1beta1/taskrun_types.go

pkg/pod/pod.go

afrittoli · 2021-05-18T10:47:47Z

In terms of documentation, it feels to me like hermetic execution mode would deserve a dedicated file in the docs folder.
We could then link to it from the various resource docs, taskrun.md for this PR.

pkg/pod/pod.go

test/hermetic_taskrun_test.go

test/README.md

tekton-robot · 2021-05-18T20:42:46Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	84.6%	-2.0

tekton-robot · 2021-05-18T20:46:14Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	84.6%	-2.0

tekton-robot · 2021-05-18T22:07:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

tekton-robot · 2021-05-18T22:16:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

tekton-robot · 2021-05-18T23:15:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	77.8%	-6.2
pkg/pod/pod.go	86.6%	87.2%	0.6

tekton-robot · 2021-05-18T23:34:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

tekton-robot · 2021-05-19T00:20:46Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

tekton-robot · 2021-05-19T00:38:44Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

tekton-robot · 2021-05-19T00:51:15Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

tekton-robot · 2021-05-19T01:23:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

ghost · 2021-05-19T16:10:55Z

Integration tests should be back in working order once #3960 is merged. You'll need to rebase once that's in.

priyawadhwa · 2021-05-19T16:11:28Z

Thanks @sbwsg!

tekton-robot · 2021-05-19T16:12:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	84.0%	81.5%	-2.5
pkg/pod/pod.go	86.6%	87.2%	0.6

chmouel · 2021-05-19T16:31:58Z

test/hermetic_taskrun_test.go

+// it does this by first running the TaskRun normally to make sure it passes
+// Then, it enables hermetic mode and makes sure the same TaskRun fails because it no longer has access to a network.
+func TestHermeticTaskRun(t *testing.T) {
+	ctx := context.Background()


I think we had that issue before for other test calling dropNetworking()

if this test is running on a dev laptop environment that is not Linux, the test will fail.

Since calling to dropNetworking which is only available on Linux https://github.com/tektoncd/pipeline/blob/567dce3cc/cmd/entrypoint/namespaces.go#L10 would panic()

ah nm this is the e2e tests, i didn't realize so forget my comment :)

Right @chmouel this is not gonna be a problem. One problem though might be running this test against OpenShift or a cluster that enforce some lower privileges (default to user, drop some privileges) that might make the calls in dropNetworking to fail 🤔 I am not entirely sure though.

Not just OpenShift but i guess dropNetworking would be a privileged operation in general.

vdemeester

@priyawadhwa @dlorenc any reason for what is happening in dropNetworking to be in the entrypoint and not outside (on the spec level — enforce by the controller through an annotation or something) ? Does using this feature requires that the user that runs the container (USER in Dockerfile, …) to be root, or something else ?

chmouel · 2021-05-19T16:46:48Z

If we open the door to netns manipulation from entrypoint or via api perhaps we can get userns too 😍

dlorenc · 2021-05-19T17:06:24Z

@priyawadhwa @dlorenc any reason for what is happening in dropNetworking to be in the entrypoint and not outside (on the spec level — enforce by the controller through an annotation or something) ?

Not sure I understand this part - there's some discussion of other ways to do this in the TEP. The short answer is, there aren't any, or weren't when I last tried: https://github.com/tektoncd/community/blob/main/teps/0025-hermekton.md

Does using this feature requires that the user that runs the container (USER in Dockerfile, …) to be root, or something else ?

There's some discussion on what is required here: b90a0ef#diff-e991ea79e58bc08e494f6f5f3bfbfeaa7d55b1bfbb68f90641500c1a7c890d21

In short - it's not exactly clear but it does work in most kernels. I think we can make it work without root ( if it doesn't already ) using some combination of user namespaces and setuid/dropping privleges on the binary itself.

tekton-robot · 2021-05-19T18:00:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	80.0%	77.8%	-2.2
pkg/pod/pod.go	86.6%	87.2%	0.6

priyawadhwa · 2021-05-19T18:38:24Z

/test pull-tekton-pipeline-integration-tests

priyawadhwa · 2021-05-19T18:38:53Z

/test pull-tekton-pipeline-build-tests

priyawadhwa · 2021-05-19T19:12:00Z

/test tekton-pipeline-unit-tests

dlorenc · 2021-05-20T00:43:45Z

/lgtm

nice!!!

ghost · 2021-05-20T10:29:30Z

/approve

tekton-robot · 2021-05-20T10:29:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbwsg

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~~ [sbwsg]

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

ghost · 2021-05-20T14:09:23Z

The PR isn't merging and this might be either because we typically squash commits down to 1 prior to merge or it might mean a rebase is needed.

Suggest doing both of these and force pushing once more. Hopefully once that's done tide should be content to merge.

This PR adds supoprt for an experimental hermetic execution mode. If users specify this on their TaskRun, then all user containers are run without network access. Any containers created or injected by tekton (init containers or sidecar containers) are not affected, and user sidecar containers are also not affected. Some notes around this PR: 1. Adds documentation around hermetic execution mode and points to it from taskrun.md 2. Removes the API change & instead specify execution mode as an annotation on a TaskRun 3. Also puts hermetic execution mode behind the `alpha` feature flag 4. Adds a unit test to make sure that the TEKTON_HERMETIC env var is set such that it can't be overridden Relevant TEP: https://github.com/tektoncd/community/blob/main/teps/0025-hermekton.md

priyawadhwa · 2021-05-20T19:39:22Z

Thanks for the tip @sbwsg, I've rebased & squashed!

tekton-robot · 2021-05-20T19:40:54Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
cmd/entrypoint/runner.go	80.0%	77.8%	-2.2
pkg/pod/pod.go	86.6%	87.2%	0.6

ghost · 2021-05-20T20:04:37Z

re-adding the lgtm 🤞

/lgtm

priyawadhwa · 2021-05-20T20:07:33Z

Yay! Thanks everyone!

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels May 18, 2021

tekton-robot requested review from dibyom and jerop May 18, 2021 00:29

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 18, 2021

priyawadhwa force-pushed the hermetic branch from e481b80 to 4b79eb9 Compare May 18, 2021 01:05

dlorenc reviewed May 18, 2021

View reviewed changes

cmd/entrypoint/runner.go Outdated Show resolved Hide resolved

pkg/apis/pipeline/v1beta1/taskrun_types.go Outdated Show resolved Hide resolved

pkg/pod/pod.go Outdated Show resolved Hide resolved

ghost reviewed May 18, 2021

View reviewed changes

pkg/pod/pod.go Show resolved Hide resolved

test/hermetic_taskrun_test.go Show resolved Hide resolved

test/hermetic_taskrun_test.go Show resolved Hide resolved

test/README.md Show resolved Hide resolved

priyawadhwa force-pushed the hermetic branch from 3a688ba to a7219ea Compare May 18, 2021 22:05

priyawadhwa force-pushed the hermetic branch from a376df3 to d5ba50c Compare May 18, 2021 23:13

priyawadhwa force-pushed the hermetic branch from d5ba50c to fccb00e Compare May 18, 2021 23:32

priyawadhwa force-pushed the hermetic branch from fccb00e to 41cc8de Compare May 19, 2021 00:18

priyawadhwa force-pushed the hermetic branch from 41cc8de to d3dd268 Compare May 19, 2021 00:36

priyawadhwa force-pushed the hermetic branch from d3dd268 to b19db42 Compare May 19, 2021 00:49

priyawadhwa force-pushed the hermetic branch from b19db42 to 9560020 Compare May 19, 2021 01:21

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 19, 2021

chmouel reviewed May 19, 2021

View reviewed changes

vdemeester reviewed May 19, 2021

View reviewed changes

tekton-robot assigned dlorenc May 20, 2021

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 20, 2021

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 20, 2021

priyawadhwa force-pushed the hermetic branch from df2d786 to 19d9eda Compare May 20, 2021 19:38

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label May 20, 2021

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 20, 2021

tekton-robot merged commit b7fa888 into tektoncd:main May 20, 2021

priyawadhwa deleted the hermetic branch May 20, 2021 20:07

priyawadhwa changed the title ~~Add support for experiemental hermetic execution mode to TaskRuns~~ Add support for experimental hermetic execution mode to TaskRuns May 20, 2021

priyawadhwa mentioned this pull request May 20, 2021

Hermekton Beta Tracking Issue #3965

Open

7 tasks

Add support for experimental hermetic execution mode to TaskRuns #3956

Add support for experimental hermetic execution mode to TaskRuns #3956

Conversation

priyawadhwa commented May 18, 2021 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented May 18, 2021

tekton-robot commented May 18, 2021

afrittoli commented May 18, 2021

tekton-robot commented May 18, 2021

tekton-robot commented May 18, 2021

tekton-robot commented May 18, 2021

tekton-robot commented May 18, 2021

tekton-robot commented May 18, 2021

tekton-robot commented May 18, 2021

tekton-robot commented May 19, 2021

tekton-robot commented May 19, 2021

tekton-robot commented May 19, 2021

tekton-robot commented May 19, 2021

ghost commented May 19, 2021

priyawadhwa commented May 19, 2021

tekton-robot commented May 19, 2021

chmouel May 19, 2021

Choose a reason for hiding this comment

chmouel May 19, 2021

Choose a reason for hiding this comment

vdemeester May 19, 2021

Choose a reason for hiding this comment

chmouel May 19, 2021

Choose a reason for hiding this comment

vdemeester left a comment • edited Loading

Choose a reason for hiding this comment

chmouel commented May 19, 2021

dlorenc commented May 19, 2021

tekton-robot commented May 19, 2021

priyawadhwa commented May 19, 2021

priyawadhwa commented May 19, 2021

priyawadhwa commented May 19, 2021

dlorenc commented May 20, 2021

ghost commented May 20, 2021

tekton-robot commented May 20, 2021

ghost commented May 20, 2021

priyawadhwa commented May 20, 2021

tekton-robot commented May 20, 2021

ghost commented May 20, 2021

priyawadhwa commented May 20, 2021

priyawadhwa commented May 18, 2021 •

edited

Loading

vdemeester left a comment •

edited

Loading