Skip to content

Commit

Permalink
Reserve /tekton/ paths and "tekton-internal-" volume names
Browse files Browse the repository at this point in the history
This prevents collisions between user-specified volumes and
Tekton-internal volumes used to support execution.

Some validation changes:
- volume names starting with "tekton-internal-" are not valid
- volumeMounts that mount at /tekton/* are not valid

Tekton's own internal volume mounts are already mounted at /tekton/*,
and now all volume names start with "tekton-internal-"

Some Tekton-internal volume names were previously randomized to prevent
collisions, which is no longer necessary, so they're no longer
randomized.

creds-init mounts annotated K8s secrets into the creds-init process.
Previously those were mounted at /var/build-secrets/* (a relic of
knative/build times). Now those are mounted at /tekton/creds-secrets/*

Some init container names were also randomly generated, which is
unnecessary, so that's gone too.
  • Loading branch information
imjasonh committed Dec 11, 2019
1 parent db33fd7 commit b165cca
Show file tree
Hide file tree
Showing 15 changed files with 183 additions and 136 deletions.
10 changes: 5 additions & 5 deletions cmd/entrypoint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ The following flags are available :
content.

The following example of usage for `entrypoint`, wait's for
`/builder/downward/ready` file to exists and have some content before
`/tekton/downward/ready` file to exists and have some content before
executing `/ko-app/bash -- -args mkdir -p /workspace/git-resource`,
and will write to `/builder/tools/0` in casse of succes, or
`/builder/tools/0.err` in case of failure.
and will write to `/tekton/tools/0` in casse of succes, or
`/tekton/tools/0.err` in case of failure.

```
entrypoint \
-wait_file /builder/downward/ready \
-post_file /builder/tools/0" \
-wait_file /tekton/downward/ready \
-post_file /tekton/tools/0" \
-wait_file_content \
-entrypoint /ko-app/bash -- -args mkdir -p /workspace/git-resource
```
3 changes: 1 addition & 2 deletions docs/developers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,11 @@ If the image is a private registry, the service account should include an

## Builder namespace on containers

The `/builder/` namespace is reserved on containers for various system tools,
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
- Default location for output-images `/builder/output-images`

## Handling of injected sidecars

Expand Down
29 changes: 22 additions & 7 deletions pkg/apis/pipeline/v1alpha1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,27 +126,42 @@ func ValidateVolumes(volumes []corev1.Volume) *apis.FieldError {
func validateSteps(steps []Step) *apis.FieldError {
// Task must not have duplicate step names.
names := map[string]struct{}{}
for _, s := range steps {
for idx, s := range steps {
if s.Image == "" {
return apis.ErrMissingField("Image")
}

if s.Script != "" {
if len(s.Command) > 0 {
return &apis.FieldError{
Message: "script cannot be used with command",
Message: fmt.Sprintf("step %d script cannot be used with command", idx),
Paths: []string{"script"},
}
}
}

if s.Name == "" {
continue
if s.Name != "" {
if _, ok := names[s.Name]; ok {
return apis.ErrInvalidValue(s.Name, "name")
}
names[s.Name] = struct{}{}
}
if _, ok := names[s.Name]; ok {
return apis.ErrInvalidValue(s.Name, "name")

for _, vm := range s.VolumeMounts {
if strings.HasPrefix(vm.MountPath, "/tekton/") &&
!strings.HasPrefix(vm.MountPath, "/tekton/home") {
return &apis.FieldError{
Message: fmt.Sprintf("step %d volumeMount cannot be mounted under /tekton/ (volumeMount %q mounted at %q)", idx, vm.Name, vm.MountPath),
Paths: []string{"volumeMounts.mountPath"},
}
}
if strings.HasPrefix(vm.Name, "tekton-internal-") {
return &apis.FieldError{
Message: fmt.Sprintf(`step %d volumeMount name %q cannot start with "tekton-internal-"`, idx, vm.Name),
Paths: []string{"volumeMounts.name"},
}
}
}
names[s.Name] = struct{}{}
}
return nil
}
Expand Down
45 changes: 43 additions & 2 deletions pkg/apis/pipeline/v1alpha1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func TestTaskSpecValidate(t *testing.T) {
}},
},
}, {
name: "valid step with script and args",
name: "valid step with script and args",
fields: fields{
Steps: []v1alpha1.Step{{
Container: corev1.Container{
Expand All @@ -217,6 +217,17 @@ func TestTaskSpecValidate(t *testing.T) {
hello $1`,
}},
},
}, {
name: "valid step with volumeMount under /tekton/home",
fields: fields{
Steps: []v1alpha1.Step{{Container: corev1.Container{
Image: "myimage",
VolumeMounts: []corev1.VolumeMount{{
Name: "foo",
MountPath: "/tekton/home",
}},
}}},
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -643,9 +654,39 @@ func TestTaskSpecValidateError(t *testing.T) {
}},
},
expectedError: apis.FieldError{
Message: "script cannot be used with command",
Message: "step 0 script cannot be used with command",
Paths: []string{"steps.script"},
},
}, {
name: "step volume mounts under /tekton/",
fields: fields{
Steps: []v1alpha1.Step{{Container: corev1.Container{
Image: "myimage",
VolumeMounts: []corev1.VolumeMount{{
Name: "foo",
MountPath: "/tekton/foo",
}},
}}},
},
expectedError: apis.FieldError{
Message: `step 0 volumeMount cannot be mounted under /tekton/ (volumeMount "foo" mounted at "/tekton/foo")`,
Paths: []string{"steps.volumeMounts.mountPath"},
},
}, {
name: "step volume mount name starts with tekton-internal-",
fields: fields{
Steps: []v1alpha1.Step{{Container: corev1.Container{
Image: "myimage",
VolumeMounts: []corev1.VolumeMount{{
Name: "tekton-internal-foo",
MountPath: "/this/is/fine",
}},
}}},
},
expectedError: apis.FieldError{
Message: `step 0 volumeMount name "tekton-internal-foo" cannot start with "tekton-internal-"`,
Paths: []string{"steps.volumeMounts.name"},
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/credentials/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

// VolumePath is the path where build secrets are written.
// It is mutable and exported for testing.
var VolumePath = "/var/build-secrets"
var VolumePath = "/tekton/creds-secrets"

// Builder is the interface for a credential initializer of any type.
type Builder interface {
Expand Down
5 changes: 2 additions & 3 deletions pkg/pod/creds_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/tektoncd/pipeline/pkg/credentials"
"github.com/tektoncd/pipeline/pkg/credentials/dockercreds"
"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
"github.com/tektoncd/pipeline/pkg/names"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -65,7 +64,7 @@ func credsInit(credsImage string, serviceAccountName, namespace string, kubeclie
}

if matched {
name := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("secret-volume-%s", secret.Name))
name := fmt.Sprintf("tekton-internal-secret-volume-%s", secret.Name)
volumeMounts = append(volumeMounts, corev1.VolumeMount{
Name: name,
MountPath: credentials.VolumeName(secret.Name),
Expand All @@ -87,7 +86,7 @@ func credsInit(credsImage string, serviceAccountName, namespace string, kubeclie
}

return &corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("credential-initializer"),
Name: "credential-initializer",
Image: credsImage,
Command: []string{"/ko-app/creds-init"},
Args: args,
Expand Down
6 changes: 3 additions & 3 deletions pkg/pod/creds_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func TestCredsInit(t *testing.T) {
},
},
want: &corev1.Container{
Name: "credential-initializer-mz4c7",
Name: "credential-initializer",
Image: images.CredsImage,
Command: []string{"/ko-app/creds-init"},
Args: []string{
Expand All @@ -110,8 +110,8 @@ func TestCredsInit(t *testing.T) {
},
Env: envVars,
VolumeMounts: append(volumeMounts, corev1.VolumeMount{
Name: "secret-volume-my-creds-9l9zj",
MountPath: "/var/build-secrets/my-creds",
Name: "tekton-internal-secret-volume-my-creds",
MountPath: "/tekton/creds-secrets/my-creds",
}),
},
}} {
Expand Down
4 changes: 2 additions & 2 deletions pkg/pod/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ import (
)

const (
toolsVolumeName = "tools"
toolsVolumeName = "tekton-internal-tools"
mountPoint = "/tekton/tools"
entrypointBinary = mountPoint + "/entrypoint"

downwardVolumeName = "downward"
downwardVolumeName = "tekton-internal-downward"
downwardMountPoint = "/tekton/downward"
downwardMountReadyFile = "ready"
readyAnnotation = "tekton.dev/ready"
Expand Down
16 changes: 9 additions & 7 deletions pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ import (
)

const (
workspaceDir = "/workspace"
homeDir = "/tekton/home"
workspaceVolumeName = "tekton-internal-workspace"
homeVolumeName = "tekton-internal-home"
workspaceDir = "/workspace"
homeDir = "/tekton/home"

taskRunLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey
ManagedByLabelKey = "app.kubernetes.io/managed-by"
Expand All @@ -51,17 +53,17 @@ var (
Value: homeDir,
}}
implicitVolumeMounts = []corev1.VolumeMount{{
Name: "workspace",
Name: "tekton-internal-workspace",
MountPath: workspaceDir,
}, {
Name: "tekton-home",
Name: "tekton-internal-home",
MountPath: homeDir,
}}
implicitVolumes = []corev1.Volume{{
Name: "workspace",
Name: "tekton-internal-workspace",
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
}, {
Name: "tekton-home",
Name: "tekton-internal-home",
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
}}
)
Expand Down Expand Up @@ -99,7 +101,7 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
}

// Initialize any workingDirs under /workspace.
if workingDirInit := workingDirInit(images.ShellImage, stepContainers, implicitVolumeMounts); workingDirInit != nil {
if workingDirInit := workingDirInit(images.ShellImage, stepContainers); workingDirInit != nil {
initContainers = append(initContainers, *workingDirInit)
}

Expand Down
28 changes: 14 additions & 14 deletions pkg/pod/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ func TestMakePod(t *testing.T) {
names.TestingSeed()

secretsVolumeMount := corev1.VolumeMount{
Name: "secret-volume-multi-creds-9l9zj",
MountPath: "/var/build-secrets/multi-creds",
Name: "tekton-internal-secret-volume-multi-creds",
MountPath: "/tekton/creds-secrets/multi-creds",
}
secretsVolume := corev1.Volume{
Name: "secret-volume-multi-creds-9l9zj",
Name: "tekton-internal-secret-volume-multi-creds",
VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "multi-creds"}},
}

Expand Down Expand Up @@ -116,7 +116,7 @@ func TestMakePod(t *testing.T) {
ServiceAccountName: "service-account",
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: []corev1.Container{{
Name: "credential-initializer-mz4c7",
Name: "credential-initializer",
Image: images.CredsImage,
Command: []string{"/ko-app/creds-init"},
Args: []string{
Expand Down Expand Up @@ -279,7 +279,7 @@ func TestMakePod(t *testing.T) {
want: &corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: []corev1.Container{{
Name: "working-dir-initializer-mz4c7",
Name: "working-dir-initializer",
Image: images.ShellImage,
Command: []string{"sh"},
Args: []string{"-c", fmt.Sprintf("mkdir -p %s", filepath.Join(workspaceDir, "test"))},
Expand Down Expand Up @@ -455,22 +455,22 @@ print("Hello from Python")`,
want: &corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: []corev1.Container{{
Name: "place-scripts-9l9zj",
Name: "place-scripts",
Image: images.ShellImage,
Command: []string{"sh"},
TTY: true,
Args: []string{"-c", `tmpfile="/tekton/scripts/script-0-mz4c7"
Args: []string{"-c", `tmpfile="/tekton/scripts/script-0-9l9zj"
touch ${tmpfile} && chmod +x ${tmpfile}
cat > ${tmpfile} << 'script-heredoc-randomly-generated-mssqb'
cat > ${tmpfile} << 'script-heredoc-randomly-generated-mz4c7'
#!/bin/sh
echo hello from step one
script-heredoc-randomly-generated-mssqb
tmpfile="/tekton/scripts/script-1-78c5n"
script-heredoc-randomly-generated-mz4c7
tmpfile="/tekton/scripts/script-1-mssqb"
touch ${tmpfile} && chmod +x ${tmpfile}
cat > ${tmpfile} << 'script-heredoc-randomly-generated-6nl7g'
cat > ${tmpfile} << 'script-heredoc-randomly-generated-78c5n'
#!/usr/bin/env python
print("Hello from Python")
script-heredoc-randomly-generated-6nl7g
script-heredoc-randomly-generated-78c5n
`},
VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount},
}, {
Expand All @@ -490,7 +490,7 @@ script-heredoc-randomly-generated-6nl7g
"-post_file",
"/tekton/tools/0",
"-entrypoint",
"/tekton/scripts/script-0-mz4c7",
"/tekton/scripts/script-0-9l9zj",
"--",
"template",
"args",
Expand All @@ -509,7 +509,7 @@ script-heredoc-randomly-generated-6nl7g
"-post_file",
"/tekton/tools/1",
"-entrypoint",
"/tekton/scripts/script-1-78c5n",
"/tekton/scripts/script-1-mssqb",
"--",
"template",
"args",
Expand Down
5 changes: 2 additions & 3 deletions pkg/pod/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ import (
)

const (
scriptsVolumeName = "scripts"
scriptsVolumeName = "tekton-internal-scripts"
scriptsDir = "/tekton/scripts"
defaultShebang = "#!/bin/sh\n"
)

var (
// Volume definition attached to Pods generated from TaskRuns that have
// steps that specify a Script.
// TODO(#1605): Generate volumeMount names, to avoid collisions.
scriptsVolume = corev1.Volume{
Name: scriptsVolumeName,
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
Expand All @@ -54,7 +53,7 @@ var (
func convertScripts(shellImage string, steps []v1alpha1.Step) (*corev1.Container, []corev1.Container) {
placeScripts := false
placeScriptsInit := corev1.Container{
Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("place-scripts"),
Name: "place-scripts",
Image: shellImage,
TTY: true,
Command: []string{"sh"},
Expand Down
Loading

0 comments on commit b165cca

Please sign in to comment.