Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reserve /tekton/ paths and "tekton-internal-" volume names #1701

Merged
merged 1 commit into from
Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
defaultScriptPreamble = "#!/bin/sh\nset -xe\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