[release/0.26] Add V2 physical container volume references

api/v2/common_types.go

@@ -12,6 +12,8 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+
+	"github.com/microsoft/dcp/pkg/commonapi"
 )
 
 // ConditionType identifies a condition reported by a V2 resource.
@@ -97,3 +99,28 @@ func ValidateNamespacedResourceMetadata(obj metav1.Object) field.ErrorList {
 
 	return errorList
 }
+
+func validateSameNamespaceResourceReference(
+	reference string,
+	namespace string,
+	referencePath *field.Path,
+) field.ErrorList {
+	if reference == "" {
+		return field.ErrorList{field.Required(referencePath, "reference must be set")}
+	}
+
+	namespacedName := commonapi.AsNamespacedName(reference, namespace)
+	errorList := field.ErrorList{}
+	if strings.Contains(reference, string(types.Separator)) {
+		for _, validationMessage := range validation.IsDNS1123Label(namespacedName.Namespace) {
+			errorList = append(errorList, field.Invalid(referencePath, reference, validationMessage))
+		}
+		if namespacedName.Namespace != namespace {
+			errorList = append(errorList, field.Invalid(referencePath, reference, "cross-namespace references are not supported"))
+		}
+	}
+	for _, validationMessage := range validation.IsDNS1123Subdomain(namespacedName.Name) {
+		errorList = append(errorList, field.Invalid(referencePath, reference, validationMessage))
+	}
+	return errorList
+}

api/v2/common_types_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	"github.com/microsoft/dcp/pkg/commonapi"
 )
@@ -68,6 +69,73 @@ func TestValidateNamespacedResourceMetadata(t *testing.T) {
 	}
 }
 
+func TestValidateSameNamespaceResourceReference(t *testing.T) {
+	testCases := []struct {
+		name          string
+		reference     string
+		expectedError string
+	}{
+		{
+			name:      "name",
+			reference: "test-resource",
+		},
+		{
+			name:      "explicit same namespace",
+			reference: "test-namespace/test-resource",
+		},
+		{
+			name:          "missing reference",
+			expectedError: "reference must be set",
+		},
+		{
+			name:          "cross namespace",
+			reference:     "other-namespace/test-resource",
+			expectedError: "cross-namespace references are not supported",
+		},
+		{
+			name:          "missing explicit namespace",
+			reference:     "/test-resource",
+			expectedError: "lowercase RFC 1123 label",
+		},
+		{
+			name:          "missing explicit name",
+			reference:     "test-namespace/",
+			expectedError: "lowercase RFC 1123 subdomain",
+		},
+		{
+			name:          "additional separator",
+			reference:     "test-namespace/test-resource/extra",
+			expectedError: "lowercase RFC 1123 subdomain",
+		},
+		{
+			name:          "invalid explicit namespace",
+			reference:     "INVALID/test-resource",
+			expectedError: "lowercase RFC 1123 label",
+		},
+		{
+			name:          "invalid name",
+			reference:     "INVALID",
+			expectedError: "lowercase RFC 1123 subdomain",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			errorList := validateSameNamespaceResourceReference(
+				tc.reference,
+				"test-namespace",
+				field.NewPath("spec", "reference"),
+			)
+			if tc.expectedError == "" {
+				require.Empty(t, errorList)
+			} else {
+				require.NotEmpty(t, errorList)
+				require.Contains(t, errorList.ToAggregate().Error(), tc.expectedError)
+			}
+		})
+	}
+}
+
 func TestResourceCreationProhibited(t *testing.T) {
 	commonapi.ResourceCreationProhibited.Store(true)
 	defer commonapi.ResourceCreationProhibited.Store(false)

api/v2/container_types.go

@@ -36,9 +36,12 @@ const (
 type VolumeMount struct {
 	Type VolumeMountType `json:"type"`
 
-	// Bind mounts: the host directory to mount.
-	// Volume mounts: name of the volume to mount.
-	Source string `json:"source"`
+	// Source is the host directory to mount for bind mounts.
+	Source string `json:"source,omitempty"`
+
+	// VolumeRef identifies a PhysicalContainerVolume in the same namespace using <name> or <namespace>/<name>.
+	// Cross-namespace references are not supported.
+	VolumeRef string `json:"volumeRef,omitempty"`
 
 	// The path within the container that the mount will use.
 	Target string `json:"target"`
@@ -48,6 +51,32 @@ type VolumeMount struct {
 	ReadOnly bool `json:"readOnly,omitempty"`
 }
 
+func ValidateVolumeMounts(mounts []VolumeMount, namespace string, mountsPath *field.Path) field.ErrorList {
+	errorList := field.ErrorList{}
+
+	for i, mount := range mounts {
+		mountPath := mountsPath.Index(i)
+		switch mount.Type {
+		case BindMount:
+			if mount.Source == "" {
+				errorList = append(errorList, field.Required(mountPath.Child("source"), "source must be set for bind mounts"))
+			}
+			if mount.VolumeRef != "" {
+				errorList = append(errorList, field.Forbidden(mountPath.Child("volumeRef"), "volumeRef cannot be set for bind mounts"))
+			}
+		case NamedVolumeMount:
+			if mount.Source != "" {
+				errorList = append(errorList, field.Forbidden(mountPath.Child("source"), "source cannot be set for volume mounts"))
+			}
+			errorList = append(errorList, validateSameNamespaceResourceReference(mount.VolumeRef, namespace, mountPath.Child("volumeRef"))...)
+		default:
+			errorList = append(errorList, field.NotSupported(mountPath.Child("type"), mount.Type, []string{string(BindMount), string(NamedVolumeMount)}))
+		}
+	}
+
+	return errorList
+}
+
 // ContainerPort describes a port, or contiguous range of ports, to publish from a container.
 // +k8s:openapi-gen=true
 type ContainerPort struct {
@@ -130,7 +159,8 @@ func ValidateContainerPorts(ports []ContainerPort, portsPath *field.Path) field.
 // ContainerNetworkConnectionConfig describes a PhysicalContainerNetwork to attach to a container.
 // +k8s:openapi-gen=true
 type ContainerNetworkConnectionConfig struct {
-	// Name of the PhysicalContainerNetwork to connect to in the container's namespace.
+	// Name identifies a PhysicalContainerNetwork in the same namespace using <name> or <namespace>/<name>.
+	// Cross-namespace references are not supported.
 	Name string `json:"name"`
 
 	// Aliases of the container on the network.

api/v2/doc.go

@@ -7,10 +7,8 @@
 //
 // The V2 API is unstable and under active development. Its resources, fields, and
 // semantics may change in breaking ways between DCP releases until the physical and
-// logical resource layers described in docs/v2-resource-plan.md are complete. In
-// particular, PhysicalContainer is expected to replace direct runtime network and
-// volume names with references to V2 resources. Callers outside DCP should not depend
-// on V2 API stability yet.
+// logical resource layers described in plan/v2-resource-plan.md are complete. Callers
+// outside DCP should not depend on V2 API stability yet.
 //
 // +kubebuilder:object:generate=true
 // +groupName=usvc-dev.developer.microsoft.com

api/v2/physical_container_network_connection_types.go

@@ -13,7 +13,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	apiserver_resource "github.com/tilt-dev/tilt-apiserver/pkg/server/builder/resource"
@@ -27,10 +26,12 @@ import (
 // Both references resolve within the connection resource's namespace.
 // +k8s:openapi-gen=true
 type PhysicalContainerNetworkConnectionSpec struct {
-	// ContainerRef is the name of the PhysicalContainer to connect.
+	// ContainerRef identifies the PhysicalContainer using <name> or <namespace>/<name>.
+	// Cross-namespace references are not supported.
 	ContainerRef string `json:"containerRef"`
 
-	// NetworkRef is the name of the PhysicalContainerNetwork to connect to.
+	// NetworkRef identifies the PhysicalContainerNetwork using <name> or <namespace>/<name>.
+	// Cross-namespace references are not supported.
 	NetworkRef string `json:"networkRef"`
 
 	// Aliases contains network-scoped aliases for the container.
@@ -101,8 +102,8 @@ func (connection *PhysicalContainerNetworkConnection) Validate(ctx context.Conte
 	}
 
 	errorList = append(errorList, commonapi.ValidateAnnotationsSize(connection.Annotations, field.NewPath("metadata", "annotations"))...)
-	errorList = append(errorList, validatePhysicalResourceReference(connection.Spec.ContainerRef, specPath.Child("containerRef"))...)
-	errorList = append(errorList, validatePhysicalResourceReference(connection.Spec.NetworkRef, specPath.Child("networkRef"))...)
+	errorList = append(errorList, validateSameNamespaceResourceReference(connection.Spec.ContainerRef, connection.Namespace, specPath.Child("containerRef"))...)
+	errorList = append(errorList, validateSameNamespaceResourceReference(connection.Spec.NetworkRef, connection.Namespace, specPath.Child("networkRef"))...)
 	return errorList
 }
 
@@ -117,18 +118,6 @@ func (connection *PhysicalContainerNetworkConnection) ValidateUpdate(ctx context
 	}
 }
 
-func validatePhysicalResourceReference(reference string, referencePath *field.Path) field.ErrorList {
-	if reference == "" {
-		return field.ErrorList{field.Required(referencePath, "reference must be set")}
-	}
-
-	errorList := field.ErrorList{}
-	for _, validationMessage := range validation.IsDNS1123Subdomain(reference) {
-		errorList = append(errorList, field.Invalid(referencePath, reference, validationMessage))
-	}
-	return errorList
-}
-
 // PhysicalContainerNetworkConnectionList contains a list of PhysicalContainerNetworkConnection instances.
 // +k8s:openapi-gen=true
 // +kubebuilder:object:root=true

api/v2/physical_container_network_connection_types_test.go

@@ -33,6 +33,17 @@ func TestPhysicalContainerNetworkConnectionValidate(t *testing.T) {
 			},
 			valid: true,
 		},
+		{
+			name: "valid explicit same namespace references",
+			connection: PhysicalContainerNetworkConnection{
+				ObjectMeta: metav1.ObjectMeta{Name: "connection", Namespace: "namespace"},
+				Spec: PhysicalContainerNetworkConnectionSpec{
+					ContainerRef: "namespace/container",
+					NetworkRef:   "namespace/network",
+				},
+			},
+			valid: true,
+		},
 		{
 			name: "missing container reference",
 			connection: PhysicalContainerNetworkConnection{
@@ -71,6 +82,26 @@ func TestPhysicalContainerNetworkConnectionValidate(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "cross-namespace container reference",
+			connection: PhysicalContainerNetworkConnection{
+				ObjectMeta: metav1.ObjectMeta{Name: "connection", Namespace: "namespace"},
+				Spec: PhysicalContainerNetworkConnectionSpec{
+					ContainerRef: "other-namespace/container",
+					NetworkRef:   "network",
+				},
+			},
+		},
+		{
+			name: "cross-namespace network reference",
+			connection: PhysicalContainerNetworkConnection{
+				ObjectMeta: metav1.ObjectMeta{Name: "connection", Namespace: "namespace"},
+				Spec: PhysicalContainerNetworkConnectionSpec{
+					ContainerRef: "container",
+					NetworkRef:   "other-namespace/network",
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {

api/v2/physical_container_types.go

@@ -16,7 +16,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	apiserver_resource "github.com/tilt-dev/tilt-apiserver/pkg/server/builder/resource"
@@ -65,6 +64,15 @@ const (
 	// PhysicalContainerReasonImageLookupFailed indicates that the referenced PhysicalContainerImage could not be read.
 	PhysicalContainerReasonImageLookupFailed ConditionReason = "ImageLookupFailed"
 
+	// PhysicalContainerReasonVolumeNotFound indicates that a referenced PhysicalContainerVolume does not exist.
+	PhysicalContainerReasonVolumeNotFound ConditionReason = "VolumeNotFound"
+
+	// PhysicalContainerReasonVolumeNotReady indicates that a referenced PhysicalContainerVolume is not ready.
+	PhysicalContainerReasonVolumeNotReady ConditionReason = "VolumeNotReady"
+
+	// PhysicalContainerReasonVolumeLookupFailed indicates that a referenced PhysicalContainerVolume could not be read.
+	PhysicalContainerReasonVolumeLookupFailed ConditionReason = "VolumeLookupFailed"
+
 	// PhysicalContainerReasonNetworkNotFound indicates that a referenced PhysicalContainerNetwork does not exist.
 	PhysicalContainerReasonNetworkNotFound ConditionReason = "NetworkNotFound"
 
@@ -169,7 +177,8 @@ type PhysicalContainerConfig struct {
 	// RetainRuntimeContainer keeps a runtime container created by this resource in place when the resource is deleted.
 	RetainRuntimeContainer bool `json:"retainRuntimeContainer,omitempty"`
 
-	// ImageRef is the name of a PhysicalContainerImage in the same namespace to use when creating a new runtime container.
+	// ImageRef identifies a PhysicalContainerImage in the same namespace using <name> or <namespace>/<name>.
+	// Cross-namespace references are not supported.
 	ImageRef string `json:"imageRef,omitempty"`
 
 	// ContainerName is the runtime name to use when creating a new container.
@@ -354,13 +363,7 @@ func (pc *PhysicalContainer) Validate(ctx context.Context) field.ErrorList {
 
 	container := pc.Spec.Container
 	containerPath := specPath.Child("container")
-	if container.ImageRef == "" {
-		errorList = append(errorList, field.Required(containerPath.Child("imageRef"), "imageRef must be set"))
-	} else {
-		for _, validationMessage := range validation.IsDNS1123Subdomain(container.ImageRef) {
-			errorList = append(errorList, field.Invalid(containerPath.Child("imageRef"), container.ImageRef, validationMessage))
-		}
-	}
+	errorList = append(errorList, validateSameNamespaceResourceReference(container.ImageRef, pc.Namespace, containerPath.Child("imageRef"))...)
 	if container.ContainerName != "" && !validContainerNameRegexp.MatchString(container.ContainerName) {
 		errorList = append(errorList, field.Invalid(containerPath.Child("containerName"), container.ContainerName, fmt.Sprintf("containerName must match regex '%s'", validContainerName)))
 	}
@@ -370,8 +373,9 @@ func (pc *PhysicalContainer) Validate(ctx context.Context) field.ErrorList {
 
 	networksPath := containerPath.Child("networks")
 	for i, network := range container.Networks {
-		errorList = append(errorList, validatePhysicalResourceReference(network.Name, networksPath.Index(i).Child("name"))...)
+		errorList = append(errorList, validateSameNamespaceResourceReference(network.Name, pc.Namespace, networksPath.Index(i).Child("name"))...)
 	}
+	errorList = append(errorList, ValidateVolumeMounts(container.VolumeMounts, pc.Namespace, containerPath.Child("volumeMounts"))...)
 	errorList = append(errorList, ValidateContainerPorts(container.Ports, containerPath.Child("ports"))...)
 	errorList = append(errorList, validateLabels(container.Labels, containerPath.Child("labels"))...)