Disable the default workingDir and HOME overrides #3878

ghost · 2021-04-13T14:44:43Z

Changes

Prior to this commit Steps were given a default HOME env var of "/tekton/home" and a default workingDir of "/workspace". These defaults collide with any value set by the Step's image Dockerfile.

This commit removes the default home and workingDir overrides (except in those few cases where they're still expected, like PipelineResources).

See https://groups.google.com/g/tekton-dev/c/C-PL8VYN51E/m/el5Fca_PDAAJ for our tekton-dev announcement of this change.

See #1836 for the original problem description and workingDir tracking issue.

See #2013 for the HOME change tracking issue.

See https://github.com/tektoncd/pipeline/blob/main/docs/deprecations.md for our documented dates for these deprecations.

See https://github.com/tektoncd/pipeline/blob/main/api_compatibility_policy.md#alpha-beta-and-ga
for our beta deprecation policy.

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

Steps no longer receive a default HOME env var of "/tekton/home", nor a default workingDir of "/workspace". 

action required: if you rely on these defaults you now have a choice to make: either update your Task steps to include the workingDir and HOME env or set the "disable-home-env-overwrite" and "disable-working-directory-overwrite" feature flags to "false". We plan to continue supporting these flags for another 9 months following the release of 0.24 and then they too will be removed.

ghost · 2021-04-13T14:45:53Z

/kind feature

tekton-robot · 2021-04-13T14:46: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
pkg/pod/pod.go	87.4%	86.6%	-0.8

tekton-robot · 2021-04-13T16:27:45Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

tekton-robot · 2021-04-13T16:43:30Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

tekton-robot · 2021-04-13T16:44:32Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

tekton-robot · 2021-04-26T17:55:49Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

tekton-robot · 2021-05-04T17:23:56Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

ghost · 2021-05-04T17:40:39Z

=== RUN   TestRealRunnerTimeout
    runner_test.go:37: step didn't timeout
--- FAIL: TestRealRunnerTimeout (0.04s)

This doesn't reproduce for me locally. Retrying.

/test tekton-pipeline-unit-tests

ghost · 2021-05-04T17:40:57Z

This PR should be up to date now with the latest enable-api-fields flag.

tekton-robot · 2021-05-06T08:01:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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

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

afrittoli

Thank you for this, really nice!
I think we should start the deprecation of the feature flag right away, but it's also ok if you prefer to do it in a different PR.
Either case I'd like to start the deprecation in v0.24.

docs/deprecations.md

examples/v1beta1/taskruns/authenticating-git-commands.yaml

tekton-robot · 2021-05-06T12:24:53Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

ghost · 2021-05-06T16:48:17Z

/test pull-tekton-pipeline-build-tests

ghost · 2021-05-07T14:30:48Z

I don't understand why these tests are all failing. They're all complaining about a lint error that doesn't exist in the latest commit:

=========================================
==== TESTING IF GOLINT HAS BEEN DONE ====
=========================================
level=warning msg="[runner] Can't run linter goanalysis_metalinter: deadcode: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/pipeline/pkg/pod/pod_test.go:941:17: undeclared name: implicitEnvVars]"
level=error msg="Running error: deadcode: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/pipeline/pkg/pod/pod_test.go:941:17: undeclared name: implicitEnvVars]"
==============================
==== GO LINT TESTS FAILED ====
==============================

implicitEnvVars does not exist in pod_test.go anymore but somehow these errors persist.

I'm going to try opening a new PR to see if that gets things moving again.

ghost · 2021-05-07T14:45:13Z

/reopen

tekton-robot · 2021-05-07T14:45:15Z

@sbwsg: Reopened this PR.

In response to this:

/reopen

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.

tekton-robot · 2021-05-07T14:50:03Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

afrittoli · 2021-05-07T15:14:42Z

I don't understand why these tests are all failing. They're all complaining about a lint error that doesn't exist in the latest commit:
=========================================
==== TESTING IF GOLINT HAS BEEN DONE ====
=========================================
level=warning msg="[runner] Can't run linter goanalysis_metalinter: deadcode: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/pipeline/pkg/pod/pod_test.go:941:17: undeclared name: implicitEnvVars]"
level=error msg="Running error: deadcode: analysis skipped: errors in package: [/home/prow/go/src/github.com/tektoncd/pipeline/pkg/pod/pod_test.go:941:17: undeclared name: implicitEnvVars]"
==============================
==== GO LINT TESTS FAILED ====
==============================
implicitEnvVars does not exist in pod_test.go anymore but somehow these errors persist.

I'm going to try opening a new PR to see if that gets things moving again.

It looks like you might need a rebase?

tekton-robot · 2021-05-10T12:45:22Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

ghost · 2021-05-10T13:10:03Z

so close. added a workingDir to the step-script examples which write to a file.

tekton-robot · 2021-05-10T13:11:21Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

afrittoli

/lgtm

afrittoli · 2021-05-10T13:52:50Z

        >>> Container step-git-clone-and-check:
        + '[' -d /tekton/home/.ssh ]
        + cp /messages/known_hosts /home/nonroot/ssh/known_hosts
        cp: can't create '/home/nonroot/ssh/known_hosts': No such file or directory

Should it be /home/nonroot/.ssh/known_hosts instead, like in the non-alpha folder?

ghost · 2021-05-10T13:55:36Z

Yup, that was exactly it, very well spotted.

Prior to this commit Steps were given a default HOME env var and a default workingDir. These defaults collide with any value set by the Step's image Dockerfile. This commit removes the default home and workingDir overrides (except in those few cases where they're still expected, like PipelineResources). See https://groups.google.com/g/tekton-dev/c/C-PL8VYN51E/m/el5Fca_PDAAJ for our tekton-dev announcement of this change. See #1836 for the original problem description and workingDir tracking issue. See #2013 for the HOME change tracking issue. See https://github.com/tektoncd/pipeline/blob/main/docs/deprecations.md for our documented dates for these deprecations. See https://github.com/tektoncd/pipeline/blob/main/api_compatibility_policy.md#alpha-beta-and-ga for our beta deprecation policy. ,

tekton-robot · 2021-05-10T13:57:56Z

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
pkg/pod/pod.go	87.4%	86.6%	-0.8

afrittoli · 2021-05-10T14:32:45Z

/lgtm

https://github.com/tektoncd/pipeline/releases/tag/v0.24.0 pr: tektoncd/pipeline#3878 Signed-off-by: xuzhu-591 <[email protected]>

tekton-robot added the release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. label Apr 13, 2021

tekton-robot requested review from dlorenc and vdemeester April 13, 2021 14:44

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 13, 2021

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 13, 2021

ghost added this to the Pipelines 0.24 milestone Apr 13, 2021

vdemeester approved these changes May 6, 2021

View reviewed changes

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

afrittoli reviewed May 6, 2021

View reviewed changes

docs/deprecations.md Show resolved Hide resolved

examples/v1beta1/taskruns/authenticating-git-commands.yaml Show resolved Hide resolved

ghost mentioned this pull request May 7, 2021

Disable the default workingDir and HOME overrides #3920

Closed

5 tasks

ghost closed this May 7, 2021

tekton-robot reopened this May 7, 2021

afrittoli reviewed May 10, 2021

View reviewed changes

tekton-robot assigned afrittoli May 10, 2021

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

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

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

tekton-robot merged commit b86a9a2 into tektoncd:main May 10, 2021

afrittoli mentioned this pull request Jun 17, 2021

resoruce output paths not effective,resource not copy from /workspace/workspace to /custom #4036

Closed

wlynch mentioned this pull request Sep 14, 2021

Restrict step volumes as read-only #4227

Closed

3 tasks

jerop mentioned this pull request Jan 18, 2022

Update hack scripts to shim temp GOPATH as needed #4465

Merged

3 tasks

xuzhu-591 pushed a commit to xuzhu-591/helm-charts that referenced this pull request Jul 20, 2023

fix: ssh auth

bc026ba

https://github.com/tektoncd/pipeline/releases/tag/v0.24.0 pr: tektoncd/pipeline#3878 Signed-off-by: xuzhu-591 <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable the default workingDir and HOME overrides #3878

Disable the default workingDir and HOME overrides #3878

ghost commented Apr 13, 2021 •

edited by ghost

Loading

ghost commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 26, 2021

tekton-robot commented May 4, 2021

ghost commented May 4, 2021

ghost commented May 4, 2021

tekton-robot commented May 6, 2021

afrittoli left a comment

tekton-robot commented May 6, 2021

ghost commented May 6, 2021

ghost commented May 7, 2021

ghost commented May 7, 2021

tekton-robot commented May 7, 2021

tekton-robot commented May 7, 2021

afrittoli commented May 7, 2021

tekton-robot commented May 10, 2021

ghost commented May 10, 2021

tekton-robot commented May 10, 2021

afrittoli left a comment

afrittoli commented May 10, 2021

ghost commented May 10, 2021

tekton-robot commented May 10, 2021

afrittoli commented May 10, 2021

Disable the default workingDir and HOME overrides #3878

Disable the default workingDir and HOME overrides #3878

Conversation

ghost commented Apr 13, 2021 • edited by ghost Loading

Changes

Submitter Checklist

Release Notes

ghost commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 13, 2021

tekton-robot commented Apr 26, 2021

tekton-robot commented May 4, 2021

ghost commented May 4, 2021

ghost commented May 4, 2021

tekton-robot commented May 6, 2021

afrittoli left a comment

Choose a reason for hiding this comment

tekton-robot commented May 6, 2021

ghost commented May 6, 2021

ghost commented May 7, 2021

ghost commented May 7, 2021

tekton-robot commented May 7, 2021

tekton-robot commented May 7, 2021

afrittoli commented May 7, 2021

tekton-robot commented May 10, 2021

ghost commented May 10, 2021

tekton-robot commented May 10, 2021

afrittoli left a comment

Choose a reason for hiding this comment

afrittoli commented May 10, 2021

ghost commented May 10, 2021

tekton-robot commented May 10, 2021

afrittoli commented May 10, 2021

ghost commented Apr 13, 2021 •

edited by ghost

Loading