update kubernetes/openshift endpoint validation (#1098)

* update endpoint validation

Signed-off-by: Stephanie <[email protected]>

* update unit test

Signed-off-by: Stephanie <[email protected]>

* update the test error check to be more clear

Signed-off-by: Stephanie <[email protected]>

* add unit test for two kube components

Signed-off-by: Stephanie <[email protected]>

---------

Signed-off-by: Stephanie <[email protected]>

Author: yangcao77

Diff for: pkg/validation/components.go

@@ -149,8 +149,8 @@ func ValidateComponents(components []v1alpha2.Component) (returnedErr error) {
 					returnedErr = multierror.Append(returnedErr, resolveErrorMessageWithImportAttributes(err, component.Attributes))
 				}
 			}
-
-			err := validateEndpoints(component.Openshift.Endpoints, processedEndPointPort, processedEndPointName)
+			currentComponentEndPointPort := make(map[int]bool)
+			err := validateDuplicatedName(component.Openshift.Endpoints, processedEndPointName, currentComponentEndPointPort)
 			if len(err) > 0 {
 				for _, endpointErr := range err {
 					returnedErr = multierror.Append(returnedErr, resolveErrorMessageWithImportAttributes(endpointErr, component.Attributes))
@@ -163,7 +163,8 @@ func ValidateComponents(components []v1alpha2.Component) (returnedErr error) {
 					returnedErr = multierror.Append(returnedErr, resolveErrorMessageWithImportAttributes(err, component.Attributes))
 				}
 			}
-			err := validateEndpoints(component.Kubernetes.Endpoints, processedEndPointPort, processedEndPointName)
+			currentComponentEndPointPort := make(map[int]bool)
+			err := validateDuplicatedName(component.Kubernetes.Endpoints, processedEndPointName, currentComponentEndPointPort)
 			if len(err) > 0 {
 				for _, endpointErr := range err {
 					returnedErr = multierror.Append(returnedErr, resolveErrorMessageWithImportAttributes(endpointErr, component.Attributes))

Diff for: pkg/validation/components_test.go

@@ -327,11 +327,32 @@ func TestValidateComponents(t *testing.T) {
 			wantErr: []string{sameTargetPortErr},
 		},
 		{
-			name: "Invalid container with same target ports in a single component",
+			name: "Valid container with same target ports in a single component",
 			components: []v1alpha2.Component{
 				generateDummyContainerComponent("name1", nil, []v1alpha2.Endpoint{endpointUrl18080, endpointUrl28080}, nil, v1alpha2.Annotation{}, false),
 			},
-			wantErr: []string{sameTargetPortErr},
+		},
+		{
+			name: "Invalid Kube components with the same endpoint names",
+			components: []v1alpha2.Component{
+				generateDummyKubernetesComponent("name1", []v1alpha2.Endpoint{endpointUrl18080}, ""),
+				generateDummyKubernetesComponent("name2", []v1alpha2.Endpoint{endpointUrl18081}, ""),
+			},
+			wantErr: []string{sameEndpointNameErr},
+		},
+		{
+			name: "Valid Kube component with the same endpoint target ports as the container component's",
+			components: []v1alpha2.Component{
+				generateDummyContainerComponent("name1", nil, []v1alpha2.Endpoint{endpointUrl18080}, nil, v1alpha2.Annotation{}, false),
+				generateDummyKubernetesComponent("name2", []v1alpha2.Endpoint{endpointUrl28080}, ""),
+			},
+		},
+		{
+			name: "Invalid Kube components with the same endpoint names",
+			components: []v1alpha2.Component{
+				generateDummyKubernetesComponent("name1", []v1alpha2.Endpoint{endpointUrl18080}, ""),
+				generateDummyKubernetesComponent("name2", []v1alpha2.Endpoint{endpointUrl28080}, ""),
+			},
 		},
 		{
 			name: "Valid containers with valid resource requirement",
@@ -532,14 +553,19 @@ func TestValidateComponents(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			err := ValidateComponents(tt.components)
 
-			if merr, ok := err.(*multierror.Error); ok && tt.wantErr != nil {
-				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")
+			merr, ok := err.(*multierror.Error)
+			if ok {
+				if tt.wantErr != nil {
+					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 {
+					t.Errorf("Error should be nil, got %v", err)
 				}
-			} else {
-				assert.Equal(t, nil, err, "Error should be nil")
+			} else if tt.wantErr != nil {
+				t.Errorf("Error should not be nil, want %v, got %v", tt.wantErr, err)
 			}
 		})
 	}

Diff for: pkg/validation/endpoints.go

@@ -10,20 +10,30 @@ import "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 func validateEndpoints(endpoints []v1alpha2.Endpoint, processedEndPointPort map[int]bool, processedEndPointName map[string]bool) (errList []error) {
 	currentComponentEndPointPort := make(map[int]bool)
 
+	errList = validateDuplicatedName(endpoints, processedEndPointName, currentComponentEndPointPort)
+	portErrorList := validateDuplicatedPort(processedEndPointPort, currentComponentEndPointPort)
+	errList = append(errList, portErrorList...)
+
+	return errList
+}
+
+func validateDuplicatedName(endpoints []v1alpha2.Endpoint, processedEndPointName map[string]bool, currentComponentEndPointPort map[int]bool) (errList []error) {
 	for _, endPoint := range endpoints {
 		if _, ok := processedEndPointName[endPoint.Name]; ok {
 			errList = append(errList, &InvalidEndpointError{name: endPoint.Name})
 		}
 		processedEndPointName[endPoint.Name] = true
 		currentComponentEndPointPort[endPoint.TargetPort] = true
 	}
+	return errList
+}
 
+func validateDuplicatedPort(processedEndPointPort map[int]bool, currentComponentEndPointPort map[int]bool) (errList []error) {
 	for targetPort := range currentComponentEndPointPort {
 		if _, ok := processedEndPointPort[targetPort]; ok {
 			errList = append(errList, &InvalidEndpointError{port: targetPort})
 		}
 		processedEndPointPort[targetPort] = true
 	}
-
 	return errList
 }

Diff for: pkg/validation/validation-rule.md

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