Port workspace and script without shebang to v1alpha2

go.mod

@@ -4,7 +4,6 @@ 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/prometheus v0.1.0 // indirect
 	contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
 	github.com/Azure/azure-sdk-for-go v36.1.0+incompatible // indirect
@@ -52,7 +51,7 @@ require (
 	golang.org/x/sys v0.0.0-20191119060738-e882bf8e40c2 // indirect
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
 	golang.org/x/tools v0.0.0-20191118222007-07fc4c7f2b98 // indirect
-	google.golang.org/api v0.10.0
+	google.golang.org/api v0.10.0 // indirect
 	google.golang.org/appengine v1.6.5 // indirect
 	google.golang.org/grpc v1.24.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect

pkg/apis/pipeline/v1alpha1/workspace_types.go

@@ -17,52 +17,12 @@ limitations under the License.
 package v1alpha1
 
 import (
-	"path/filepath"
-
-	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
-	corev1 "k8s.io/api/core/v1"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha2"
 )
 
 // WorkspaceDeclaration is a declaration of a volume that a Task requires.
-type WorkspaceDeclaration struct {
-	// Name is the name by which you can bind the volume at runtime.
-	Name string `json:"name"`
-	// Description is an optional human readable description of this volume.
-	// +optional
-	Description string `json:"description,omitempty"`
-	// MountPath overrides the directory that the volume will be made available at.
-	// +optional
-	MountPath string `json:"mountPath,omitempty"`
-	// ReadOnly dictates whether a mounted volume is writable. By default this
-	// field is false and so mounted volumes are writable.
-	ReadOnly bool `json:"readOnly,omitempty"`
-}
-
-// GetMountPath returns the mountPath for w which is the MountPath if provided or the
-// default if not.
-func (w *WorkspaceDeclaration) GetMountPath() string {
-	if w.MountPath != "" {
-		return w.MountPath
-	}
-	return filepath.Join(pipeline.WorkspaceDir, w.Name)
-}
+type WorkspaceDeclaration = v1alpha2.WorkspaceDeclaration
 
 // WorkspaceBinding maps a Task's declared workspace to a Volume.
 // Currently we only support PersistentVolumeClaims and EmptyDir.
-type WorkspaceBinding struct {
-	// Name is the name of the workspace populated by the volume.
-	Name string `json:"name"`
-	// SubPath is optionally a directory on the volume which should be used
-	// for this binding (i.e. the volume will be mounted at this sub directory).
-	// +optional
-	SubPath string `json:"subPath,omitempty"`
-	// PersistentVolumeClaimVolumeSource represents a reference to a
-	// PersistentVolumeClaim in the same namespace. Either this OR EmptyDir can be used.
-	// +optional
-	PersistentVolumeClaim *corev1.PersistentVolumeClaimVolumeSource `json:"persistentVolumeClaim,omitempty"`
-	// EmptyDir represents a temporary directory that shares a Task's lifetime.
-	// More info: https://kubernetes.io/docs/concepts/storage/volumes#emptydir
-	// Either this OR PersistentVolumeClaim can be used.
-	// +optional
-	EmptyDir *corev1.EmptyDirVolumeSource `json:"emptyDir,omitempty"`
-}
+type WorkspaceBinding = v1alpha2.WorkspaceBinding

pkg/apis/pipeline/v1alpha1/workspace_validation.go

@@ -15,35 +15,3 @@ limitations under the License.
 */
 
 package v1alpha1
-
-import (
-	"context"
-
-	"k8s.io/apimachinery/pkg/api/equality"
-	"knative.dev/pkg/apis"
-)
-
-// Validate looks at the Volume provided in wb and makes sure that it is valid.
-// This means that only one VolumeSource can be specified, and also that the
-// supported VolumeSource is itself valid.
-func (b *WorkspaceBinding) Validate(ctx context.Context) *apis.FieldError {
-	if equality.Semantic.DeepEqual(b, &WorkspaceBinding{}) || b == nil {
-		return apis.ErrMissingField(apis.CurrentField)
-	}
-
-	// Users should only provide one supported VolumeSource.
-	if b.PersistentVolumeClaim != nil && b.EmptyDir != nil {
-		return apis.ErrMultipleOneOf("workspace.persistentvolumeclaim", "workspace.emptydir")
-	}
-
-	// Users must provide at least one supported VolumeSource.
-	if b.PersistentVolumeClaim == nil && b.EmptyDir == nil {
-		return apis.ErrMissingOneOf("workspace.persistentvolumeclaim", "workspace.emptydir")
-	}
-
-	// For a PersistentVolumeClaim to work, you must at least provide the name of the PVC to use.
-	if b.PersistentVolumeClaim != nil && b.PersistentVolumeClaim.ClaimName == "" {
-		return apis.ErrMissingField("workspace.persistentvolumeclaim.claimname")
-	}
-	return nil
-}

pkg/apis/pipeline/v1alpha2/task_types.go

@@ -82,6 +82,9 @@ type TaskSpec struct {
 	// Sidecars are run alongside the Task's step containers. They begin before
 	// the steps start and end after the steps complete.
 	Sidecars []corev1.Container `json:"sidecars,omitempty"`
+
+	// Workspaces are the volumes that this Task requires.
+	Workspaces []WorkspaceDeclaration
 }
 
 // Step embeds the Container type, which allows it to include fields not

pkg/apis/pipeline/v1alpha2/task_validation.go

@@ -19,6 +19,7 @@ package v1alpha2
 import (
 	"context"
 	"fmt"
+	"path/filepath"
 	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
@@ -49,6 +50,9 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError {
 	if err := ValidateVolumes(ts.Volumes).ViaField("volumes"); err != nil {
 		return err
 	}
+	if err := ValidateDeclaredWorkspaces(ts.Workspaces, ts.Steps, ts.StepTemplate); err != nil {
+		return err
+	}
 	mergedSteps, err := MergeStepsWithStepTemplate(ts.StepTemplate, ts.Steps)
 	if err != nil {
 		return &apis.FieldError{
@@ -90,6 +94,44 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError {
 	return nil
 }
 
+// a mount path which conflicts with any other declared workspaces, with the explicitly
+// declared volume mounts, or with the stepTemplate. The names must also be unique.
+func ValidateDeclaredWorkspaces(workspaces []WorkspaceDeclaration, steps []Step, stepTemplate *corev1.Container) *apis.FieldError {
+	mountPaths := map[string]struct{}{}
+	for _, step := range steps {
+		for _, vm := range step.VolumeMounts {
+			mountPaths[filepath.Clean(vm.MountPath)] = struct{}{}
+		}
+	}
+	if stepTemplate != nil {
+		for _, vm := range stepTemplate.VolumeMounts {
+			mountPaths[filepath.Clean(vm.MountPath)] = struct{}{}
+		}
+	}
+
+	wsNames := map[string]struct{}{}
+	for _, w := range workspaces {
+		// Workspace names must be unique
+		if _, ok := wsNames[w.Name]; ok {
+			return &apis.FieldError{
+				Message: fmt.Sprintf("workspace name %q must be unique", w.Name),
+				Paths:   []string{"workspaces.name"},
+			}
+		}
+		wsNames[w.Name] = struct{}{}
+		// Workspaces must not try to use mount paths that are already used
+		mountPath := filepath.Clean(w.GetMountPath())
+		if _, ok := mountPaths[mountPath]; ok {
+			return &apis.FieldError{
+				Message: fmt.Sprintf("workspace mount path %q must be unique", mountPath),
+				Paths:   []string{"workspaces.mountpath"},
+			}
+		}
+		mountPaths[mountPath] = struct{}{}
+	}
+	return nil
+}
+
 func ValidateVolumes(volumes []corev1.Volume) *apis.FieldError {
 	// Task must not have duplicate volume names.
 	vols := map[string]struct{}{}
@@ -108,33 +150,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.Args) > 0 || len(s.Command) > 0 {
-				return &apis.FieldError{
-					Message: "script cannot be used with args or command",
-					Paths:   []string{"script"},
-				}
-			}
-			if !strings.HasPrefix(strings.TrimSpace(s.Script), "#!") {
+			if len(s.Command) > 0 {
 				return &apis.FieldError{
-					Message: "script must start with a shebang (#!)",
+					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
 }