Skip to content

Commit

Permalink
Disable the default workingDir and HOME overrides
Browse files Browse the repository at this point in the history
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 tektoncd#1836 for the
original problem description and workingDir tracking issue.

See tektoncd#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.
,
  • Loading branch information
Scott authored and tekton-robot committed May 10, 2021
1 parent ea9158f commit b86a9a2
Show file tree
Hide file tree
Showing 24 changed files with 148 additions and 138 deletions.
18 changes: 5 additions & 13 deletions config/config-feature-flags.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,18 @@ data:
# https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#affinity-assistant-and-specifying-workspace-order-in-a-pipeline
# or https://github.com/tektoncd/pipeline/pull/2630 for more info.
disable-affinity-assistant: "false"
# Setting this flag to "true" will prevent Tekton overriding your
# Setting this flag to "false" will allow Tekton to override your
# Task container's $HOME environment variable.
#
# The default behaviour currently is for Tekton to override the
# $HOME environment variable but this will change in an upcoming
# release.
#
# See https://github.com/tektoncd/pipeline/issues/2013 for more
# info.
disable-home-env-overwrite: "false"
# Setting this flag to "true" will prevent Tekton overriding your
disable-home-env-overwrite: "true"
# Setting this flag to "false" will allow Tekton to override your
# Task container's working directory.
#
# The default behaviour currently is for Tekton to override the
# working directory if not set by the user but this will change
# in an upcoming release.
#
# See https://github.com/tektoncd/pipeline/issues/1836 for more
# info.
disable-working-directory-overwrite: "false"
disable-working-directory-overwrite: "true"
# Setting this flag to "true" will prevent Tekton scanning attached
# service accounts and injecting any credentials it finds into your
# Steps.
Expand All @@ -61,7 +53,7 @@ data:
# Note: setting this to "true" will prevent PipelineResources from
# working.
#
# See https://github.com/tektoncd/pipeline/issues/1836 for more
# See https://github.com/tektoncd/pipeline/issues/2791 for more
# info.
disable-creds-init: "false"
# This option should be set to false when Pipelines is running in a
Expand Down
7 changes: 3 additions & 4 deletions docs/deprecations.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

<!--
---
linkTitle: "Deprecations"
Expand All @@ -21,9 +20,9 @@ being deprecated.
| Feature Being Deprecated | Deprecation Announcement | [API Compatibility Policy](https://github.com/tektoncd/pipeline/tree/main/api_compatibility_policy.md) | Earliest Date or Release of Removal |
| ------------------------ | ------------------------ | -------------------------------------------------------------------------------------------------------- | ------------------------ |
| [`tekton.dev/task` label on ClusterTasks](https://github.com/tektoncd/pipeline/issues/2533) | [v0.12.0](https://github.com/tektoncd/pipeline/releases/tag/v0.12.0) | Beta | January 30 2021 |
| [Step `$HOME` env var defaults to `/tekton/home`](https://github.com/tektoncd/pipeline/issues/2013) | [v0.11.0-rc1](https://github.com/tektoncd/pipeline/releases/tag/v0.11.0-rc1) | Beta | December 4 2020 |
| [Step `workingDir` defaults to `/workspace`](https://github.com/tektoncd/pipeline/issues/1836) | [v0.11.0-rc1](https://github.com/tektoncd/pipeline/releases/tag/v0.11.0-rc1) | Beta | December 4 2020 |
| [The `TaskRun.Status.ResourceResults.ResourceRef` field is deprecated and will be removed.](https://github.com/tektoncd/pipeline/issues/2694) | [v0.14.0](https://github.com/tektoncd/pipeline/releases/tag/v0.14.0) | Beta | April 30 2021 |
| [The `PipelineRun.Spec.ServiceAccountNames` field is deprecated and will be removed.](https://github.com/tektoncd/pipeline/issues/2614) | [v0.15.0](https://github.com/tektoncd/pipeline/releases/tag/v0.15.0) | Beta | May 15 2021 |
| [`Conditions` CRD is deprecated and will be removed. Use `WhenExpressions` instead.](https://github.com/tektoncd/community/blob/main/teps/0007-conditions-beta.md) | [v0.16.0](https://github.com/tektoncd/pipeline/releases/tag/v0.16.0) | Alpha | Nov 02 2020 |
| [The PascalCase fields in WhenExpressions is deprecated and will be removed](https://github.com/tektoncd/pipeline/pull/3389) | [v0.17.2](https://github.com/tektoncd/pipeline/releases/tag/v0.17.2) | Alpha | Jan 07 2021 |
| [The PascalCase fields in WhenExpressions is deprecated and will be removed](https://github.com/tektoncd/pipeline/pull/3389) | [v0.17.2](https://github.com/tektoncd/pipeline/releases/tag/v0.17.2) | Alpha | Jan 07 2021 |
| [The `disable-home-env-overwrite` flag will be removed](https://github.com/tektoncd/pipeline/issues/2013) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | Beta | February 10 2022 |
| [The `disable-working-dir-overwrite` flag will be removed](https://github.com/tektoncd/pipeline/issues/1836) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | Beta | February 10 2022 |
16 changes: 7 additions & 9 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -320,15 +320,13 @@ To customize the behavior of the Pipelines Controller, modify the ConfigMap `fea
node in the cluster must have an appropriate label matching `topologyKey`. If some or all nodes
are missing the specified `topologyKey` label, it can lead to unintended behavior.
- `disable-home-env-overwrite` - set this flag to `true` to prevent Tekton
from overriding the `$HOME` environment variable for the containers executing your `Steps`.
The default is `false`. For more information, see the [associated issue](https://github.com/tektoncd/pipeline/issues/2013).
- `disable-working-directory-overwrite` - set this flag to `true` to prevent Tekton
from overriding the working directory for the containers executing your `Steps`.
The default value is `false`, which causes Tekton to override the working directory
for each `Step` that does not have its working directory explicitly set with `/workspace`.
For more information, see the [associated issue](https://github.com/tektoncd/pipeline/issues/1836).
- `disable-home-env-overwrite` - set this flag to `false` to allow Tekton
to override the `$HOME` environment variable for the containers executing your `Steps`.
The default is `true`. For more information, see the [associated issue](https://github.com/tektoncd/pipeline/issues/2013).
- `disable-working-directory-overwrite` - set this flag to `false` to allow Tekton
to override the working directory for the containers executing your `Steps`.
The default value is `true`. For more information, see the [associated issue](https://github.com/tektoncd/pipeline/issues/1836).
- `running-in-environment-with-injected-sidecars`: set this flag to `"true"` to allow the
Tekton controller to set the `tekton.dev/ready` annotation at pod creation time for
Expand Down
1 change: 1 addition & 0 deletions examples/v1alpha1/taskruns/dind-sidecar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ spec:
# Use the certs generated by the sidecar daemon.
- name: DOCKER_CERT_PATH
value: /certs/client
workingDir: /workspace
script: |
#!/usr/bin/env sh
set -e
Expand Down
1 change: 1 addition & 0 deletions examples/v1alpha1/taskruns/gcs-resource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ spec:
type: storage
steps:
- image: alpine
workingDir: /workspace
script: unzip source/archive.zip && cat file.txt
inputs:
resources:
Expand Down
5 changes: 5 additions & 0 deletions examples/v1alpha1/taskruns/git-resource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
inputs:
resources:
Expand All @@ -34,6 +35,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
inputs:
resources:
Expand All @@ -58,6 +60,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
inputs:
resources:
Expand All @@ -83,6 +86,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
inputs:
resources:
Expand Down Expand Up @@ -110,6 +114,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
inputs:
resources:
Expand Down
3 changes: 3 additions & 0 deletions examples/v1alpha1/taskruns/home-is-set.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ spec:
taskSpec:
steps:
- image: ubuntu
env:
- name: HOME
value: /tekton/home
script: |
#!/usr/bin/env bash
[[ $HOME == /tekton/home ]]
1 change: 1 addition & 0 deletions examples/v1alpha1/taskruns/step-script.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ spec:
image: ubuntu
securityContext:
runAsUser: 16000
workingDir: /workspace
script: |
#!/usr/bin/env bash
cat > file << EOF
Expand Down
1 change: 1 addition & 0 deletions examples/v1alpha1/taskruns/workingdir.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ spec:
steps:
- name: default
image: ubuntu
workingDir: /workspace
script: |
#!/usr/bin/env bash
[[ $PWD == /workspace ]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ spec:
# When disable-home-env-overwrite is "true", creds-init credentials
# will be copied to /home/nonroot/.ssh by the entrypoint. We just need to
# overwrite the known_hosts file with that of our test git server.
cp /messages/known_hosts /home/nonroot/ssh/known_hosts
cp /messages/known_hosts /home/nonroot/.ssh/known_hosts
fi
git clone root@localhost:/root/repo ./repo
Expand Down
2 changes: 1 addition & 1 deletion examples/v1beta1/taskruns/authenticating-git-commands.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ spec:
# When disable-home-env-overwrite is "true", creds-init credentials
# will be copied to /home/nonroot/.ssh by the entrypoint. We just need to
# overwrite the known_hosts file with that of our test git server.
cp /messages/known_hosts /home/nonroot/ssh/known_hosts
cp /messages/known_hosts /home/nonroot/.ssh/known_hosts
fi
git clone root@localhost:/root/repo ./repo
Expand Down
1 change: 1 addition & 0 deletions examples/v1beta1/taskruns/dind-sidecar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ spec:
# Use the certs generated by the sidecar daemon.
- name: DOCKER_CERT_PATH
value: /certs/client
workingDir: /workspace
script: |
#!/usr/bin/env sh
set -e
Expand Down
1 change: 1 addition & 0 deletions examples/v1beta1/taskruns/gcs-resource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ spec:
type: storage
steps:
- image: alpine
workingDir: /workspace
script: unzip source/archive.zip && cat file.txt
resources:
inputs:
Expand Down
3 changes: 3 additions & 0 deletions examples/v1beta1/taskruns/git-resource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
resources:
inputs:
Expand All @@ -34,6 +35,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
resources:
inputs:
Expand All @@ -58,6 +60,7 @@ spec:
type: git
steps:
- image: ubuntu
workingDir: /workspace
script: cat skaffold/README.md
resources:
inputs:
Expand Down
3 changes: 3 additions & 0 deletions examples/v1beta1/taskruns/home-is-set.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ spec:
taskSpec:
steps:
- image: ubuntu
env:
- name: HOME
value: /tekton/home
script: |
#!/usr/bin/env bash
[[ $HOME == /tekton/home ]]
1 change: 1 addition & 0 deletions examples/v1beta1/taskruns/step-script.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ spec:
image: ubuntu
securityContext:
runAsUser: 16000
workingDir: /workspace
script: |
#!/usr/bin/env bash
cat > file << EOF
Expand Down
1 change: 1 addition & 0 deletions examples/v1beta1/taskruns/workingdir.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ spec:
steps:
- name: default
image: ubuntu
workingDir: /workspace
script: |
#!/usr/bin/env bash
[[ $PWD == /workspace ]]
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ const (
enableTektonOCIBundles = "enable-tekton-oci-bundles"
enableCustomTasks = "enable-custom-tasks"
enableAPIFields = "enable-api-fields"
DefaultDisableHomeEnvOverwrite = false
DefaultDisableWorkingDirOverwrite = false
DefaultDisableHomeEnvOverwrite = true
DefaultDisableWorkingDirOverwrite = true
DefaultDisableAffinityAssistant = false
DefaultDisableCredsInit = false
DefaultRunningInEnvWithInjectedSidecars = true
Expand Down
14 changes: 13 additions & 1 deletion pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
testCases := []testCase{
{
expectedConfig: &config.FeatureFlags{
DisableHomeEnvOverwrite: false,
DisableWorkingDirOverwrite: false,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
EnableAPIFields: "stable",
},
Expand All @@ -61,6 +63,8 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
EnableTektonOCIBundles: true,
EnableCustomTasks: true,

DisableHomeEnvOverwrite: true,
DisableWorkingDirOverwrite: true,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
},
fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks",
Expand All @@ -71,20 +75,28 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
EnableTektonOCIBundles: true,
EnableCustomTasks: true,

DisableHomeEnvOverwrite: true,
DisableWorkingDirOverwrite: true,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
},
fileName: "feature-flags-bundles-and-custom-tasks",
},
}

for _, tc := range testCases {
verifyConfigFileWithExpectedFeatureFlagsConfig(t, tc.fileName, tc.expectedConfig)
fileName := tc.fileName
expectedConfig := tc.expectedConfig
t.Run(fileName, func(t *testing.T) {
verifyConfigFileWithExpectedFeatureFlagsConfig(t, fileName, expectedConfig)
})
}
}

func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) {
FeatureFlagsConfigEmptyName := "feature-flags-empty"
expectedConfig := &config.FeatureFlags{
DisableHomeEnvOverwrite: true,
DisableWorkingDirOverwrite: true,
RunningInEnvWithInjectedSidecars: true,
EnableAPIFields: "stable",
}
Expand Down
8 changes: 5 additions & 3 deletions pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
// Add implicit env vars.
// They're prepended to the list, so that if the user specified any
// themselves their value takes precedence.
for i, s := range stepContainers {
env := append(implicitEnvVars, s.Env...)
stepContainers[i].Env = env
if len(implicitEnvVars) > 0 {
for i, s := range stepContainers {
env := append(implicitEnvVars, s.Env...)
stepContainers[i].Env = env
}
}

// Add implicit volume mounts to each step, unless the step specifies
Expand Down
Loading

0 comments on commit b86a9a2

Please sign in to comment.