Skip to content

Commit

Permalink
Allow disabling implicit HOME overwrite
Browse files Browse the repository at this point in the history
Tekton currently overwrites a Task container's HOME environment variable
so that it always points to /tekton/home. This behaviour causes issues
for any image that expects this variable to be set to some predefined
value.

This PR introduces a feature flag configmap and a flag to disable
Tekton's HOME behaviour. The default behaviour remains the same but in
an upcoming release we will change this default so that Tekton no longer
overwrites the HOME variable.
  • Loading branch information
Scott authored and tekton-robot committed Feb 26, 2020
1 parent 472df74 commit 6877e66
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 14 deletions.
30 changes: 30 additions & 0 deletions config/config-feature-flags.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright 2019 The Tekton Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: v1
kind: ConfigMap
metadata:
name: feature-flags
namespace: tekton-pipelines
data:
# Setting this flag to "true" will prevent Tekton overriding 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"
13 changes: 8 additions & 5 deletions docs/developers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,14 @@ If the image is a private registry, the service account should include an

## Builder namespace on containers

The `/tekton/` namespace is reserved on containers for various system tools,
such as the following:

- The environment variable HOME is set to `/tekton/home`, used by the builder
tools and injected on into all of the step containers
The `/tekton/` directory is reserved on containers for internal usage. Examples
of how this directory is used:

- Task Results are written to `/tekton/results`
- Various tools like the entrypoint are placed in `/tekton/tools`
- The termination log message is written to `/tekton/termination`
- Sequencing step containers is done using both `/tekton/downward/ready`
and numbered files in `/tekton/tools`

## Handling of injected sidecars

Expand Down
33 changes: 29 additions & 4 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ for more information_

### How are resources shared between tasks

Pipelines need a way to share `PipelineResources` between tasks. The alternatives are a
[Persistent volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/),
Pipelines need a way to share `PipelineResources` between tasks. The options are a
[Persistent Volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/),
an [S3 Bucket](https://aws.amazon.com/s3/)
or a [GCS storage bucket](https://cloud.google.com/storage/)
or a [GCS Bucket](https://cloud.google.com/storage/)

The PVC option can be configured using a ConfigMap with the name
`config-artifact-pvc` and the following attributes:
Expand All @@ -153,7 +153,7 @@ The GCS storage bucket or the S3 bucket can be configured using a ConfigMap with
This is a limitation coming from using [gsutil](https://cloud.google.com/storage/docs/gsutil) with a boto configuration
behind the scene to access the S3 bucket.

An typical configuration to use an S3 bucket is available below :
A typical configuration to use an S3 bucket is available below :

```yaml
apiVersion: v1
Expand Down Expand Up @@ -219,6 +219,31 @@ data:
*NOTE:* The `_example` key in the provided [config-defaults.yaml](./../config/config-defaults.yaml)
file contains the keys that can be overriden and their default values.

### Turning On or Off Features of the Pipelines Controller

The ConfigMap `feature-flags` can be used to turn on or off specific features
of the Pipelines Controller.

Supported flags:

- `disable-home-env-overwrite` - Setting this flag to "true" will prevent Tekton
from overwriting Step containers' `$HOME` environment variable. The default
value is "false" and so the default behaviour is for `$HOME` to be overwritten by
Tekton with `/tekton/home`. This default is very likely to change in an upcoming
release. For further reference see https://github.com/tektoncd/pipeline/issues/2013.

Here is an example of the `feature-flags` ConfigMap with `disable-home-env-overwrite`
flipped on:

```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: feature-flags
data:
disable-home-env-overwrite: "true" # Tekton will not overwrite $HOME in Steps.
```

## Custom Releases

The [release Task](./../tekton/README.md) can be used for creating a custom
Expand Down
2 changes: 1 addition & 1 deletion docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ following fields:
- [`serviceAccountName`](#service-account) - Specifies a `ServiceAccount` resource
object that enables your build to run with the defined authentication
information. When a `ServiceAccount` isn't specified, the `default-service-account`
specified in the configmap - config-defaults will be applied.
specified in the configmap `config-defaults` will be applied.
- [`inputs`] - Specifies [input parameters](#input-parameters) and
[input resources](#providing-resources)
- [`outputs`] - Specifies [output resources](#providing-resources)
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.13

require (
cloud.google.com/go v0.47.0 // indirect
cloud.google.com/go/storage v1.0.0
contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
github.com/GoogleCloudPlatform/cloud-builders/gcs-fetcher v0.0.0-20191203181535-308b93ad1f39
github.com/cloudevents/sdk-go v1.0.0
Expand Down Expand Up @@ -44,6 +45,7 @@ require (
golang.org/x/sys v0.0.0-20200212091648-12a6c2dcc1e4 // indirect
golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
golang.org/x/tools v0.0.0-20200214144324-88be01311a71 // indirect
google.golang.org/api v0.15.0
google.golang.org/appengine v1.6.5 // indirect
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
gopkg.in/yaml.v2 v2.2.5 // indirect
Expand Down
31 changes: 27 additions & 4 deletions pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/names"
"github.com/tektoncd/pipeline/pkg/system"
"github.com/tektoncd/pipeline/pkg/version"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -33,6 +34,9 @@ import (
const (
homeDir = "/tekton/home"

featureFlagConfigMapName = "feature-flags"
featureFlagDisableHomeEnvKey = "disable-home-env-overwrite"

taskRunLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey
)

Expand All @@ -47,10 +51,6 @@ var (
Kind: "TaskRun",
}
// These are injected into all of the source/step containers.
implicitEnvVars = []corev1.EnvVar{{
Name: "HOME",
Value: homeDir,
}}
implicitVolumeMounts = []corev1.VolumeMount{{
Name: "tekton-internal-workspace",
MountPath: pipeline.WorkspaceDir,
Expand All @@ -73,6 +73,15 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
var initContainers []corev1.Container
var volumes []corev1.Volume

implicitEnvVars := []corev1.EnvVar{}

if shouldOverrideHomeEnv(kubeclient) {
implicitEnvVars = append(implicitEnvVars, corev1.EnvVar{
Name: "HOME",
Value: homeDir,
})
}

// Add our implicit volumes first, so they can be overridden by the user if they prefer.
volumes = append(volumes, implicitVolumes...)

Expand Down Expand Up @@ -288,3 +297,17 @@ func getLimitRangeMinimum(namespace string, kubeclient kubernetes.Interface) (co

return min, nil
}

// shouldOverrideHomeEnv returns a bool indicating whether a Pod should have its
// $HOME environment variable overwritten with /tekton/home or if it should be
// left unmodified. The default behaviour is to overwrite the $HOME variable
// but this is planned to change in an upcoming release.
//
// For further reference see https://github.com/tektoncd/pipeline/issues/2013
func shouldOverrideHomeEnv(kubeclient kubernetes.Interface) bool {
configMap, err := kubeclient.CoreV1().ConfigMaps(system.GetNamespace()).Get(featureFlagConfigMapName, metav1.GetOptions{})
if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureFlagDisableHomeEnvKey] == "true" {
return false
}
return true
}
47 changes: 47 additions & 0 deletions pkg/pod/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha2"
"github.com/tektoncd/pipeline/pkg/system"
"github.com/tektoncd/pipeline/test/names"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand All @@ -44,6 +45,10 @@ var (
func TestMakePod(t *testing.T) {
names.TestingSeed()

implicitEnvVars := []corev1.EnvVar{{
Name: "HOME",
Value: homeDir,
}}
secretsVolumeMount := corev1.VolumeMount{
Name: "tekton-internal-secret-volume-multi-creds-9l9zj",
MountPath: "/tekton/creds-secrets/multi-creds",
Expand Down Expand Up @@ -795,3 +800,45 @@ func TestMakeLabels(t *testing.T) {
t.Errorf("Diff labels:\n%s", d)
}
}

func TestShouldOverrideHomeEnv(t *testing.T) {
for _, tc := range []struct {
description string
configMap *corev1.ConfigMap
expected bool
}{{
description: "Default behaviour: A missing disable-home-env-overwrite flag should result in true",
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: featureFlagConfigMapName, Namespace: system.GetNamespace()},
Data: map[string]string{},
},
expected: true,
}, {
description: "Setting disable-home-env-overwrite to false should result in true",
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: featureFlagConfigMapName, Namespace: system.GetNamespace()},
Data: map[string]string{
featureFlagDisableHomeEnvKey: "false",
},
},
expected: true,
}, {
description: "Setting disable-home-env-overwrite to true should result in false",
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: featureFlagConfigMapName, Namespace: system.GetNamespace()},
Data: map[string]string{
featureFlagDisableHomeEnvKey: "true",
},
},
expected: false,
}} {
t.Run(tc.description, func(t *testing.T) {
kubeclient := fakek8s.NewSimpleClientset(
tc.configMap,
)
if result := shouldOverrideHomeEnv(kubeclient); result != tc.expected {
t.Errorf("Expected %t Received %t", tc.expected, result)
}
})
}
}

0 comments on commit 6877e66

Please sign in to comment.