diff --git a/pkg/validation/components.go b/pkg/validation/components.go index e72d33255..997e4daa1 100644 --- a/pkg/validation/components.go +++ b/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 --git a/pkg/validation/components_test.go b/pkg/validation/components_test.go index 2657c6a39..b6213beea 100644 --- a/pkg/validation/components_test.go +++ b/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 --git a/pkg/validation/endpoints.go b/pkg/validation/endpoints.go index 698b6ecd5..83992a28b 100644 --- a/pkg/validation/endpoints.go +++ b/pkg/validation/endpoints.go @@ -10,6 +10,14 @@ 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}) @@ -17,13 +25,15 @@ func validateEndpoints(endpoints []v1alpha2.Endpoint, processedEndPointPort map[ 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 --git a/pkg/validation/validation-rule.md b/pkg/validation/validation-rule.md index c8ff0cccb..216e1b2ef 100644 --- a/pkg/validation/validation-rule.md +++ b/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: