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 resolveErrorMessageWithImportArrtibutes(err, command.Attributes)
 		}
 
 		commandGroup := getGroup(command)
@@ -50,18 +50,17 @@ func ValidateCommands(commands []v1alpha2.Command, components []v1alpha2.Compone
 
 // 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:

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"
@@ -76,6 +77,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
@@ -144,6 +149,26 @@ func TestValidateCommands(t *testing.T) {
 				generateDummyCompositeCommand("composite-a", []string{"basic-exec"}, nil),
 			},
 		},
+		{
+			name: "Invalid command with import source attribute",
+			commands: []v1alpha2.Command{
+				{
+					Attributes: parentOverridesFromMainDevfile,
+					Id:         "command",
+					CommandUnion: v1alpha2.CommandUnion{
+						Apply: &v1alpha2.ApplyCommand{
+							LabeledCommand: v1alpha2.LabeledCommand{
+								BaseCommand: v1alpha2.BaseCommand{
+									Group: nil,
+								},
+							},
+							Component: "invalidComponent",
+						},
+					},
+				},
+			},
+			wantErr: &invalidCmdErrWithImportAttributes,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/validation/components.go

@@ -26,6 +26,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 +39,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 +54,7 @@ func ValidateComponents(components []v1alpha2.Component) error {
 
 			err := validateEndpoints(component.Container.Endpoints, processedEndPointPort, processedEndPointName)
 			if err != nil {
-				return err
+				return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
 			}
 		case component.Volume != nil:
 			processedVolumes[component.Name] = true
@@ -61,37 +63,38 @@ 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 resolveErrorMessageWithImportArrtibutes(invalidVolErr, component.Attributes)
 				}
 			}
 		case component.Openshift != nil:
 			if component.Openshift.Uri != "" {
 				err := ValidateURI(component.Openshift.Uri)
 				if err != nil {
-					return err
+					return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
 				}
 			}
 
 			err := validateEndpoints(component.Openshift.Endpoints, processedEndPointPort, processedEndPointName)
 			if err != nil {
-				return err
+				return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
 			}
 		case component.Kubernetes != nil:
 			if component.Kubernetes.Uri != "" {
 				err := ValidateURI(component.Kubernetes.Uri)
 				if err != nil {
-					return err
+					return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
 				}
 			}
 			err := validateEndpoints(component.Kubernetes.Endpoints, processedEndPointPort, processedEndPointName)
 			if err != nil {
-				return err
+				return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
 			}
 		case component.Plugin != nil:
 			if component.Plugin.RegistryUrl != "" {
 				err := ValidateURI(component.Plugin.RegistryUrl)
 				if err != nil {
-					return err
+					return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
 				}
 			}
 		}
@@ -103,7 +106,9 @@ func ValidateComponents(components []v1alpha2.Component) error {
 	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("\nvolume mount %s belonging to the container component %s", volumeMountName, componentName)
+				newErr := resolveErrorMessageWithImportArrtibutes(missingVolumeMountErr, processedComponentWithVolumeMounts[componentName].Attributes)
+				invalidVolumeMountsErr += newErr.Error()
 			}
 		}
 	}

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"
@@ -137,6 +138,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
@@ -237,6 +242,23 @@ func TestValidateComponents(t *testing.T) {
 			},
 			wantErr: &invalidURIErr,
 		},
+		{
+			name: "Invalid component due to bad URI with import source attributes",
+			components: []v1alpha2.Component{
+				{
+					Attributes: pluginOverridesFromMainDevfile,
+					Name:       "name",
+					ComponentUnion: v1alpha2.ComponentUnion{
+						Plugin: &v1alpha2.PluginComponent{
+							ImportReference: v1alpha2.ImportReference{
+								RegistryUrl: "http//invalidregistryurl",
+							},
+						},
+					},
+				},
+			},
+			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 type is invalid", 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,28 @@ type InvalidComponentError struct {
 func (e *InvalidComponentError) Error() string {
 	return fmt.Sprintf("the component %q is invalid - %s", e.componentName, e.reason)
 }
+
+// resolveErrorMessageWithImportArrtibutes returns an error message with detailed information on the import resource
+// example: "the component <compName> is invalid - <reason>", imported from Uri: http://example.com/devfile.yaml, in parent overrides from main devfile"
+func resolveErrorMessageWithImportArrtibutes(err error, attributes attributesAPI.Attributes) error {
+	var findKeyErr error
+	importReference := attributes.Get(ImportSourceAttribute, &findKeyErr)
+	returnErr := err
+
+	if findKeyErr == nil {
+		returnErr = fmt.Errorf("%s, imported from %s", returnErr.Error(), importReference)
+		parentOverrideReference := attributes.Get(ParentOverrideAttribute, &findKeyErr)
+		if findKeyErr == nil {
+			returnErr = fmt.Errorf("%s, in parent overrides from %s", returnErr.Error(), parentOverrideReference)
+		} else {
+			// reset findKeyErr to nil
+			findKeyErr = nil
+			pluginOverrideReference := attributes.Get(PluginOverrideAttribute, &findKeyErr)
+			if findKeyErr == nil {
+				returnErr = fmt.Errorf("%s, in plugin overrides from %s", returnErr.Error(), pluginOverrideReference)
+			}
+		}
+	}
+
+	return returnErr
+}

pkg/validation/projects.go

@@ -23,16 +23,22 @@ func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error {
 
 		switch len(gitSource.Remotes) {
 		case 0:
-			errString += fmt.Sprintf("\nstarterProject %s should have at least one remote", starterProject.Name)
+			starterProjectErr := fmt.Errorf("\nstarterProject %s should have at least one remote", starterProject.Name)
+			newErr := resolveErrorMessageWithImportArrtibutes(starterProjectErr, starterProject.Attributes)
+			errString += newErr.Error()
 		case 1:
 			if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
 				err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, starterProject.Name)
 				if err != nil {
-					errString += fmt.Sprintf("\n%s", err.Error())
+					newErr := resolveErrorMessageWithImportArrtibutes(err, starterProject.Attributes)
+					errString += newErr.Error()
+					errString += fmt.Sprintf("\n%s", newErr.Error())
 				}
 			}
 		default: // len(gitSource.Remotes) >= 2
-			errString += fmt.Sprintf("\nstarterProject %s should have one remote only", starterProject.Name)
+			starterProjectErr := fmt.Errorf("\nstarterProject %s should have one remote only", starterProject.Name)
+			newErr := resolveErrorMessageWithImportArrtibutes(starterProjectErr, starterProject.Attributes)
+			errString += newErr.Error()
 		}
 	}
 
@@ -61,20 +67,26 @@ func ValidateProjects(projects []v1alpha2.Project) error {
 
 		switch len(gitSource.Remotes) {
 		case 0:
-			errString += fmt.Sprintf("\nprojects %s should have at least one remote", project.Name)
+			projectErr := fmt.Errorf("\nprojects %s should have at least one remote", project.Name)
+			newErr := resolveErrorMessageWithImportArrtibutes(projectErr, project.Attributes)
+			errString += newErr.Error()
 		case 1:
 			if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
 				if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
-					errString += fmt.Sprintf("\n%s", err.Error())
+					newErr := resolveErrorMessageWithImportArrtibutes(err, project.Attributes)
+					errString += fmt.Sprintf("\n%s", newErr.Error())
 				}
 			}
 		default: // len(gitSource.Remotes) >= 2
 			if gitSource.CheckoutFrom == nil || gitSource.CheckoutFrom.Remote == "" {
-				errString += fmt.Sprintf("\nproject %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name)
+				projectErr := fmt.Errorf("\nproject %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name)
+				newErr := resolveErrorMessageWithImportArrtibutes(projectErr, project.Attributes)
+				errString += newErr.Error()
 				continue
 			}
 			if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
-				errString += fmt.Sprintf("\n%s", err.Error())
+				newErr := resolveErrorMessageWithImportArrtibutes(err, project.Attributes)
+				errString += fmt.Sprintf("\n%s", newErr.Error())
 			}
 		}
 	}

pkg/validation/projects_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"
@@ -69,6 +70,10 @@ func TestValidateStarterProjects(t *testing.T) {
 	wrongCheckoutErr := "unable to find the checkout remote .* in the remotes for project.*"
 	atleastOneRemoteErr := "starterProject .* should have at least one remote"
 
+	parentOverridesFromMainDevfile := attributes.Attributes{}.PutString(ImportSourceAttribute,
+		"uri: http://127.0.0.1:8080").PutString(ParentOverrideAttribute, "main devfile")
+	wrongCheckoutErrWithImportAttributes := "unable to find the checkout remote .* in the remotes for project.*, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
+
 	tests := []struct {
 		name            string
 		starterProjects []v1alpha2.StarterProject
@@ -115,6 +120,24 @@ func TestValidateStarterProjects(t *testing.T) {
 			},
 			wantErr: &atleastOneRemoteErr,
 		},
+		{
+			name: "Invalid Starter Project due to wrong checkout with import source attributes",
+			starterProjects: []v1alpha2.StarterProject{
+				{
+					Attributes: parentOverridesFromMainDevfile,
+					Name:       "starterproject1",
+					ProjectSource: v1alpha2.ProjectSource{
+						Github: &v1alpha2.GithubProjectSource{
+							GitLikeProjectSource: v1alpha2.GitLikeProjectSource{
+								Remotes:      map[string]string{"test": "testremote"},
+								CheckoutFrom: &v1alpha2.CheckoutFrom{Remote: "origin"},
+							},
+						},
+					},
+				},
+			},
+			wantErr: &wrongCheckoutErrWithImportAttributes,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -135,6 +158,10 @@ func TestValidateProjects(t *testing.T) {
 	atleastOneRemoteErr := "projects .* should have at least one remote"
 	missingCheckOutFromRemoteErr := "project .* has more than one remote defined, but has no checkoutfrom remote defined"
 
+	parentOverridesFromMainDevfile := attributes.Attributes{}.PutString(ImportSourceAttribute,
+		"uri: http://127.0.0.1:8080").PutString(ParentOverrideAttribute, "main devfile")
+	wrongCheckoutErrWithImportAttributes := "unable to find the checkout remote .* in the remotes for project.*, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
+
 	tests := []struct {
 		name     string
 		projects []v1alpha2.Project
@@ -184,6 +211,24 @@ func TestValidateProjects(t *testing.T) {
 			},
 			wantErr: &atleastOneRemoteErr,
 		},
+		{
+			name: "Invalid Project due to wrong checkout with import source attributes",
+			projects: []v1alpha2.Project{
+				{
+					Attributes: parentOverridesFromMainDevfile,
+					Name:       "project1",
+					ProjectSource: v1alpha2.ProjectSource{
+						Github: &v1alpha2.GithubProjectSource{
+							GitLikeProjectSource: v1alpha2.GitLikeProjectSource{
+								Remotes:      map[string]string{"test": "testremote"},
+								CheckoutFrom: &v1alpha2.CheckoutFrom{Remote: "origin"},
+							},
+						},
+					},
+				},
+			},
+			wantErr: &wrongCheckoutErrWithImportAttributes,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {