validator checks import attributes

pkg/validation/commands.go

@@ -25,7 +25,7 @@ func ValidateCommands(commands []v1alpha2.Command, components []v1alpha2.Compone
 		parentCommands := make(map[string]string)
 		err = validateCommand(command, parentCommands, commandMap, components)
 		if err != nil {
-			return err
+			return resolveErrorMessageWithImportAttributes(err, command.Attributes)
 		}
 
 		commandGroup := getGroup(command)
@@ -34,46 +34,47 @@ func ValidateCommands(commands []v1alpha2.Command, components []v1alpha2.Compone
 		}
 	}
 
-	groupErrors := ""
+	var groupErrorsList []string
 	for groupKind, commands := range groupKindCommandMap {
 		if err = validateGroup(commands); err != nil {
-			groupErrors += fmt.Sprintf("\ncommand group %s error - %s", groupKind, err.Error())
+			groupErrorsList = append(groupErrorsList, fmt.Sprintf("command group %s error - %s", groupKind, err.Error()))
 		}
 	}
 
-	if len(groupErrors) > 0 {
-		err = fmt.Errorf("%s", groupErrors)
+	if len(groupErrorsList) > 0 {
+		groupErrors := strings.Join(groupErrorsList, "\n")
+		err = fmt.Errorf("\n%s", groupErrors)
 	}
 
 	return err
 }
 
 // validateCommand validates a given devfile command where parentCommands is a map to track all the parent commands when validating
 // the composite command's subcommands recursively and devfileCommands is a map of command id to the devfile command
-func validateCommand(command v1alpha2.Command, parentCommands map[string]string, devfileCommands map[string]v1alpha2.Command, components []v1alpha2.Component) (err error) {
+func validateCommand(command v1alpha2.Command, parentCommands map[string]string, devfileCommands map[string]v1alpha2.Command, components []v1alpha2.Component) error {
 
 	switch {
 	case command.Composite != nil:
 		return validateCompositeCommand(&command, parentCommands, devfileCommands, components)
 	case command.Exec != nil || command.Apply != nil:
 		return validateCommandComponent(command, components)
 	default:
-		err = fmt.Errorf("command %s type is invalid", command.Id)
+		return &InvalidCommandTypeError{commandId: command.Id}
 	}
 
-	return err
 }
 
 // validateGroup validates commands belonging to a specific group kind. If there are multiple commands belonging to the same group:
 // 1. without any default, err out
 // 2. with more than one default, err out
 func validateGroup(commands []v1alpha2.Command) error {
 	defaultCommandCount := 0
-
+	var defaultCommands []v1alpha2.Command
 	if len(commands) > 1 {
 		for _, command := range commands {
 			if getGroup(command).IsDefault {
 				defaultCommandCount++
+				defaultCommands = append(defaultCommands, command)
 			}
 		}
 	} else {
@@ -83,7 +84,15 @@ func validateGroup(commands []v1alpha2.Command) error {
 	if defaultCommandCount == 0 {
 		return fmt.Errorf("there should be exactly one default command, currently there is no default command")
 	} else if defaultCommandCount > 1 {
-		return fmt.Errorf("there should be exactly one default command, currently there is more than one default command")
+		var commandsReferenceList []string
+		for _, command := range defaultCommands {
+			commandsReferenceList = append(commandsReferenceList,
+				resolveErrorMessageWithImportAttributes(fmt.Errorf("command: %s", command.Id), command.Attributes).Error())
+		}
+		commandsReference := strings.Join(commandsReferenceList, "; ")
+		// example: there should be exactly one default command, currently there is more than one default command;
+		// command: <id1>; command: <id2>, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
+		return fmt.Errorf("there should be exactly one default command, currently there is more than one default command; %s", commandsReference)
 	}
 
 	return nil

pkg/validation/commands_test.go

@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"github.com/devfile/api/v2/pkg/attributes"
 	"testing"
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
@@ -30,9 +31,10 @@ func generateDummyExecCommand(name, component string, group *v1alpha2.CommandGro
 }
 
 // generateDummyExecCommand returns a dummy apply command for testing
-func generateDummyApplyCommand(name, component string, group *v1alpha2.CommandGroup) v1alpha2.Command {
+func generateDummyApplyCommand(name, component string, group *v1alpha2.CommandGroup, cmdAttributes attributes.Attributes) v1alpha2.Command {
 	return v1alpha2.Command{
-		Id: name,
+		Attributes: cmdAttributes,
+		Id:         name,
 		CommandUnion: v1alpha2.CommandUnion{
 			Apply: &v1alpha2.ApplyCommand{
 				LabeledCommand: v1alpha2.LabeledCommand{
@@ -76,6 +78,10 @@ func TestValidateCommands(t *testing.T) {
 	multipleDefaultCmdErr := ".*there should be exactly one default command, currently there is more than one default command"
 	invalidCmdErr := ".*command does not map to a container component"
 
+	parentOverridesFromMainDevfile := attributes.Attributes{}.PutString(ImportSourceAttribute,
+		"uri: http://127.0.0.1:8080").PutString(ParentOverrideAttribute, "main devfile")
+	invalidCmdErrWithImportAttributes := ".*command does not map to a container component, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
+
 	tests := []struct {
 		name     string
 		commands []v1alpha2.Command
@@ -131,7 +137,7 @@ func TestValidateCommands(t *testing.T) {
 		{
 			name: "Invalid Apply command with wrong component",
 			commands: []v1alpha2.Command{
-				generateDummyApplyCommand("command", "invalidComponent", nil),
+				generateDummyApplyCommand("command", "invalidComponent", nil, attributes.Attributes{}),
 			},
 			wantErr: &invalidCmdErr,
 		},
@@ -144,6 +150,13 @@ func TestValidateCommands(t *testing.T) {
 				generateDummyCompositeCommand("composite-a", []string{"basic-exec"}, nil),
 			},
 		},
+		{
+			name: "Invalid command with import source attribute",
+			commands: []v1alpha2.Command{
+				generateDummyApplyCommand("command", "invalidComponent", nil, parentOverridesFromMainDevfile),
+			},
+			wantErr: &invalidCmdErrWithImportAttributes,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -193,11 +206,11 @@ func TestValidateCommandComponent(t *testing.T) {
 		},
 		{
 			name:    "Valid Apply Command",
-			command: generateDummyApplyCommand("command", component, nil),
+			command: generateDummyApplyCommand("command", component, nil, attributes.Attributes{}),
 		},
 		{
 			name:    "Invalid Apply Command with wrong component",
-			command: generateDummyApplyCommand("command", invalidComponent, &v1alpha2.CommandGroup{Kind: runGroup}),
+			command: generateDummyApplyCommand("command", invalidComponent, &v1alpha2.CommandGroup{Kind: runGroup}, attributes.Attributes{}),
 			wantErr: &invalidCmdErr,
 		},
 	}
@@ -307,7 +320,13 @@ func TestValidateGroup(t *testing.T) {
 	component := "alias1"
 
 	noDefaultCmdErr := ".*there should be exactly one default command, currently there is no default command"
-	multipleDefaultCmdErr := ".*there should be exactly one default command, currently there is more than one default command"
+	multipleDefaultError := ".*there should be exactly one default command, currently there is more than one default command"
+	multipleDefaultCmdErr := multipleDefaultError + "; command: run command; command: customcommand"
+
+	parentOverridesFromMainDevfile := attributes.Attributes{}.PutString(ImportSourceAttribute,
+		"uri: http://127.0.0.1:8080").PutString(ParentOverrideAttribute, "main devfile")
+	multipleDefaultCmdErrWithImportAttributes := multipleDefaultError +
+		"; command: run command; command: customcommand, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
 
 	tests := []struct {
 		name     string
@@ -322,6 +341,14 @@ func TestValidateGroup(t *testing.T) {
 			},
 			wantErr: &multipleDefaultCmdErr,
 		},
+		{
+			name: "Two default run commands with import source attribute",
+			commands: []v1alpha2.Command{
+				generateDummyExecCommand("run command", component, &v1alpha2.CommandGroup{Kind: runGroup, IsDefault: true}),
+				generateDummyApplyCommand("customcommand", component, &v1alpha2.CommandGroup{Kind: runGroup, IsDefault: true}, parentOverridesFromMainDevfile),
+			},
+			wantErr: &multipleDefaultCmdErrWithImportAttributes,
+		},
 		{
 			name: "No default for more than one build commands",
 			commands: []v1alpha2.Command{

pkg/validation/components.go

@@ -2,6 +2,7 @@ package validation
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -26,6 +27,7 @@ func ValidateComponents(components []v1alpha2.Component) error {
 	processedVolumeMounts := make(map[string][]string)
 	processedEndPointName := make(map[string]bool)
 	processedEndPointPort := make(map[int]bool)
+	processedComponentWithVolumeMounts := make(map[string]v1alpha2.Component)
 
 	err := v1alpha2.CheckDuplicateKeys(components)
 	if err != nil {
@@ -38,6 +40,7 @@ func ValidateComponents(components []v1alpha2.Component) error {
 			// Process all the volume mounts in container components to validate them later
 			for _, volumeMount := range component.Container.VolumeMounts {
 				processedVolumeMounts[component.Name] = append(processedVolumeMounts[component.Name], volumeMount.Name)
+				processedComponentWithVolumeMounts[component.Name] = component
 
 			}
 
@@ -52,7 +55,7 @@ func ValidateComponents(components []v1alpha2.Component) error {
 
 			err := validateEndpoints(component.Container.Endpoints, processedEndPointPort, processedEndPointName)
 			if err != nil {
-				return err
+				return resolveErrorMessageWithImportAttributes(err, component.Attributes)
 			}
 		case component.Volume != nil:
 			processedVolumes[component.Name] = true
@@ -61,54 +64,58 @@ func ValidateComponents(components []v1alpha2.Component) error {
 				// express storage in Kubernetes. For reference, you may check doc
 				// https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
 				if _, err := resource.ParseQuantity(component.Volume.Size); err != nil {
-					return &InvalidVolumeError{name: component.Name, reason: fmt.Sprintf("size %s for volume component is invalid, %v. Example - 2Gi, 1024Mi", component.Volume.Size, err)}
+					invalidVolErr := &InvalidVolumeError{name: component.Name, reason: fmt.Sprintf("size %s for volume component is invalid, %v. Example - 2Gi, 1024Mi", component.Volume.Size, err)}
+					return resolveErrorMessageWithImportAttributes(invalidVolErr, component.Attributes)
 				}
 			}
 		case component.Openshift != nil:
 			if component.Openshift.Uri != "" {
 				err := ValidateURI(component.Openshift.Uri)
 				if err != nil {
-					return err
+					return resolveErrorMessageWithImportAttributes(err, component.Attributes)
 				}
 			}
 
 			err := validateEndpoints(component.Openshift.Endpoints, processedEndPointPort, processedEndPointName)
 			if err != nil {
-				return err
+				return resolveErrorMessageWithImportAttributes(err, component.Attributes)
 			}
 		case component.Kubernetes != nil:
 			if component.Kubernetes.Uri != "" {
 				err := ValidateURI(component.Kubernetes.Uri)
 				if err != nil {
-					return err
+					return resolveErrorMessageWithImportAttributes(err, component.Attributes)
 				}
 			}
 			err := validateEndpoints(component.Kubernetes.Endpoints, processedEndPointPort, processedEndPointName)
 			if err != nil {
-				return err
+				return resolveErrorMessageWithImportAttributes(err, component.Attributes)
 			}
 		case component.Plugin != nil:
 			if component.Plugin.RegistryUrl != "" {
 				err := ValidateURI(component.Plugin.RegistryUrl)
 				if err != nil {
-					return err
+					return resolveErrorMessageWithImportAttributes(err, component.Attributes)
 				}
 			}
 		}
 
 	}
 
 	// Check if the volume mounts mentioned in the containers are referenced by a volume component
-	var invalidVolumeMountsErr string
+	var invalidVolumeMountsErrList []string
 	for componentName, volumeMountNames := range processedVolumeMounts {
 		for _, volumeMountName := range volumeMountNames {
 			if !processedVolumes[volumeMountName] {
-				invalidVolumeMountsErr += fmt.Sprintf("\nvolume mount %s belonging to the container component %s", volumeMountName, componentName)
+				missingVolumeMountErr := fmt.Errorf("volume mount %s belonging to the container component %s", volumeMountName, componentName)
+				newErr := resolveErrorMessageWithImportAttributes(missingVolumeMountErr, processedComponentWithVolumeMounts[componentName].Attributes)
+				invalidVolumeMountsErrList = append(invalidVolumeMountsErrList, newErr.Error())
 			}
 		}
 	}
 
-	if len(invalidVolumeMountsErr) > 0 {
+	if len(invalidVolumeMountsErrList) > 0 {
+		invalidVolumeMountsErr := fmt.Sprintf("\n%s", strings.Join(invalidVolumeMountsErrList, "\n"))
 		return &MissingVolumeMountError{errMsg: invalidVolumeMountsErr}
 	}
 

pkg/validation/components_test.go

@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"github.com/devfile/api/v2/pkg/attributes"
 	"testing"
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
@@ -79,10 +80,11 @@ func generateDummyKubernetesComponent(name string, endpoints []v1alpha2.Endpoint
 }
 
 // generateDummyPluginComponent returns a dummy Plugin component for testing
-func generateDummyPluginComponent(name, url string) v1alpha2.Component {
+func generateDummyPluginComponent(name, url string, compAttribute attributes.Attributes) v1alpha2.Component {
 
 	return v1alpha2.Component{
-		Name: name,
+		Attributes: compAttribute,
+		Name:       name,
 		ComponentUnion: v1alpha2.ComponentUnion{
 			Plugin: &v1alpha2.PluginComponent{
 				ImportReference: v1alpha2.ImportReference{
@@ -137,6 +139,10 @@ func TestValidateComponents(t *testing.T) {
 	sameTargetPortErr := "devfile contains multiple containers with same TargetPort.*"
 	invalidURIErr := ".*invalid URI for request"
 
+	pluginOverridesFromMainDevfile := attributes.Attributes{}.PutString(ImportSourceAttribute,
+		"uri: http://127.0.0.1:8080").PutString(PluginOverrideAttribute, "main devfile")
+	invalidURIErrWithImportAttributes := ".*invalid URI for request, imported from uri: http://127.0.0.1:8080, in plugin overrides from main devfile"
+
 	tests := []struct {
 		name       string
 		components []v1alpha2.Component
@@ -233,10 +239,17 @@ func TestValidateComponents(t *testing.T) {
 		{
 			name: "Invalid plugin registry url",
 			components: []v1alpha2.Component{
-				generateDummyPluginComponent("abc", "http//invalidregistryurl"),
+				generateDummyPluginComponent("abc", "http//invalidregistryurl", attributes.Attributes{}),
 			},
 			wantErr: &invalidURIErr,
 		},
+		{
+			name: "Invalid component due to bad URI with import source attributes",
+			components: []v1alpha2.Component{
+				generateDummyPluginComponent("abc", "http//invalidregistryurl", pluginOverridesFromMainDevfile),
+			},
+			wantErr: &invalidURIErrWithImportAttributes,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/validation/errors.go

@@ -1,6 +1,9 @@
 package validation
 
-import "fmt"
+import (
+	"fmt"
+	attributesAPI "github.com/devfile/api/v2/pkg/attributes"
+)
 
 // InvalidEventError returns an error if the devfile event type has invalid events
 type InvalidEventError struct {
@@ -22,6 +25,15 @@ func (e *InvalidCommandError) Error() string {
 	return fmt.Sprintf("the command %q is invalid - %s", e.commandId, e.reason)
 }
 
+// InvalidCommandError returns an error if the command is invalid
+type InvalidCommandTypeError struct {
+	commandId string
+}
+
+func (e *InvalidCommandTypeError) Error() string {
+	return fmt.Sprintf("command %s has invalid type", e.commandId)
+}
+
 // ReservedEnvError returns an error if the user attempts to customize a reserved ENV in a container
 type ReservedEnvError struct {
 	componentName string
@@ -77,3 +89,35 @@ type InvalidComponentError struct {
 func (e *InvalidComponentError) Error() string {
 	return fmt.Sprintf("the component %q is invalid - %s", e.componentName, e.reason)
 }
+
+// resolveErrorMessageWithImportAttributes returns an updated error message
+// with detailed information on the imported and overriden resource.
+// example:
+// "the component <compName> is invalid - <reason>, imported from Uri: http://example.com/devfile.yaml, in parent overrides from main devfile"
+func resolveErrorMessageWithImportAttributes(validationErr error, attributes attributesAPI.Attributes) error {
+	var findKeyErr error
+	importReference := attributes.Get(ImportSourceAttribute, &findKeyErr)
+
+	// overridden element must contain import resource information
+	// an overridden element can be either parentOverride or pluginOverride
+	// example:
+	// if an element is imported from another devfile, but contains no overrides - ImportSourceAttribute
+	// if an element is from parentOverride - ImportSourceAttribute + ParentOverrideAttribute
+	// if an element is from pluginOverride - ImportSourceAttribute + PluginOverrideAttribute
+	if findKeyErr == nil {
+		validationErr = fmt.Errorf("%s, imported from %s", validationErr.Error(), importReference)
+		parentOverrideReference := attributes.Get(ParentOverrideAttribute, &findKeyErr)
+		if findKeyErr == nil {
+			validationErr = fmt.Errorf("%s, in parent overrides from %s", validationErr.Error(), parentOverrideReference)
+		} else {
+			// reset findKeyErr to nil
+			findKeyErr = nil
+			pluginOverrideReference := attributes.Get(PluginOverrideAttribute, &findKeyErr)
+			if findKeyErr == nil {
+				validationErr = fmt.Errorf("%s, in plugin overrides from %s", validationErr.Error(), pluginOverrideReference)
+			}
+		}
+	}
+
+	return validationErr
+}