Allow duplicate endpoint targetPorts if they are in same container

Allow multiple endpoints to have the same target point if and only if
they are attached to the same container. Otherwise, target ports must be
unique unless dedicatedPod is set to true.

Allowing multiple endpoints to use the same port is required for some
applications which require two routes to the same endpoint, in order to
create isolation between components within the browser.

Signed-off-by: Angel Misevski <[email protected]>

Author: amisevsk

pkg/apis/workspaces/v1alpha2/endpoint.go

@@ -50,7 +50,8 @@ type Endpoint struct {
 	// +kubebuilder:validation:MaxLength=63
 	Name string `json:"name"`
 
-	// The port number should be unique.
+	// Port number to be used within the container component. The same port cannot
+	// be used by two different container components.
 	TargetPort int `json:"targetPort"`
 
 	// Describes how the endpoint should be exposed on the network.

pkg/validation/components_test.go

@@ -244,7 +244,7 @@ func TestValidateComponents(t *testing.T) {
 	reservedEnvErr := "env variable .* is reserved and cannot be customized in component.*"
 	invalidSizeErr := "size .* for volume component is invalid"
 	sameEndpointNameErr := "devfile contains multiple endpoint entries with same name.*"
-	sameTargetPortErr := "devfile contains multiple endpoint entries with same TargetPort.*"
+	sameTargetPortErr := "devfile contains multiple containers with same endpoint targetPort.*"
 	invalidURIErr := ".*invalid URI for request"
 	imageCompTwoRemoteErr := "component .* should have one remote only"
 	imageCompNoRemoteErr := "component .* should have at least one remote"
@@ -533,9 +533,10 @@ func TestValidateComponents(t *testing.T) {
 			err := ValidateComponents(tt.components)
 
 			if merr, ok := err.(*multierror.Error); ok && tt.wantErr != nil {
-				assert.Equal(t, len(tt.wantErr), len(merr.Errors), "Error list length should match")
-				for i := 0; i < len(merr.Errors); i++ {
-					assert.Regexp(t, tt.wantErr[i], merr.Errors[i].Error(), "Error message should match")
+				if assert.Equal(t, len(tt.wantErr), len(merr.Errors), "Error list length should match") {
+					for i := 0; i < len(merr.Errors); i++ {
+						assert.Regexp(t, tt.wantErr[i], merr.Errors[i].Error(), "Error message should match")
+					}
 				}
 			} else {
 				assert.Equal(t, nil, err, "Error should be nil")

pkg/validation/endpoints.go

@@ -5,17 +5,24 @@ import "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 // validateEndpoints checks if
 // 1. all the endpoint names are unique across components
 // 2. endpoint port are unique across component containers
+//   ie; two component containers cannot have the same target port but two endpoints
+//   in a single component container can have the same target port
 func validateEndpoints(endpoints []v1alpha2.Endpoint, processedEndPointPort map[int]bool, processedEndPointName map[string]bool) (errList []error) {
+	currentComponentEndPointPort := make(map[int]bool)
 
 	for _, endPoint := range endpoints {
 		if _, ok := processedEndPointName[endPoint.Name]; ok {
 			errList = append(errList, &InvalidEndpointError{name: endPoint.Name})
 		}
-		if _, ok := processedEndPointPort[endPoint.TargetPort]; ok {
-			errList = append(errList, &InvalidEndpointError{port: endPoint.TargetPort})
-		}
 		processedEndPointName[endPoint.Name] = true
-		processedEndPointPort[endPoint.TargetPort] = true
+		currentComponentEndPointPort[endPoint.TargetPort] = true
+	}
+
+	for targetPort := range currentComponentEndPointPort {
+		if _, ok := processedEndPointPort[targetPort]; ok {
+			errList = append(errList, &InvalidEndpointError{port: targetPort})
+		}
+		processedEndPointPort[targetPort] = true
 	}
 
 	return errList

pkg/validation/endpoints_test.go

@@ -10,7 +10,7 @@ import (
 func TestValidateEndpoints(t *testing.T) {
 
 	duplicateNameErr := "multiple endpoint entries with same name"
-	duplicatePortErr := "devfile contains multiple endpoint entries with same TargetPort"
+	duplicatePortErr := "devfile contains multiple containers with same endpoint targetPort"
 
 	tests := []struct {
 		name                  string
@@ -48,7 +48,6 @@ func TestValidateEndpoints(t *testing.T) {
 			},
 			processedEndpointName: map[string]bool{},
 			processedEndpointPort: map[int]bool{},
-			wantErr:               []string{duplicatePortErr},
 		},
 		{
 			name: "Duplicate endpoint port across components",
@@ -83,9 +82,10 @@ func TestValidateEndpoints(t *testing.T) {
 			err := validateEndpoints(tt.endpoints, tt.processedEndpointPort, tt.processedEndpointName)
 
 			if tt.wantErr != nil {
-				assert.Equal(t, len(tt.wantErr), len(err), "Error list length should match")
-				for i := 0; i < len(err); i++ {
-					assert.Regexp(t, tt.wantErr[i], err[i].Error(), "Error message should match")
+				if assert.Equal(t, len(tt.wantErr), len(err), "Error list length should match") {
+					for i := 0; i < len(err); i++ {
+						assert.Regexp(t, tt.wantErr[i], err[i].Error(), "Error message should match")
+					}
 				}
 			} else {
 				assert.Equal(t, 0, len(err), "Error list should be empty")

pkg/validation/errors.go

@@ -96,7 +96,7 @@ func (e *InvalidEndpointError) Error() string {
 	if e.name != "" {
 		errMsg = fmt.Sprintf("devfile contains multiple endpoint entries with same name: %v", e.name)
 	} else if fmt.Sprint(e.port) != "" {
-		errMsg = fmt.Sprintf("devfile contains multiple endpoint entries with same TargetPort: %v", e.port)
+		errMsg = fmt.Sprintf("devfile contains multiple containers with same endpoint targetPort: %v", e.port)
 	}
 
 	return errMsg

pkg/validation/validation-rule.md

@@ -14,7 +14,8 @@ The validation will be done as part of schema validation, the rule will be intro
 
 ### Endpoints:
 - all the endpoint names are unique across components
-- all the endpoint ports are unique across components. Only exception: container component with `dedicatedpod=true`
+- endpoint ports must be unique across components -- two components cannot have the same target port, but one component may have two endpoints with the same target port. This restriction does not apply to container components with `dedicatedPod` set to `true`.
+
 
 ### Commands:
 1. id must be unique