diff --git a/pkg/validation/commands.go b/pkg/validation/commands.go index 8447737c8..64d024bb1 100644 --- a/pkg/validation/commands.go +++ b/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,15 +34,16 @@ 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 @@ -50,7 +51,7 @@ 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: @@ -58,10 +59,9 @@ func validateCommand(command v1alpha2.Command, parentCommands map[string]string, 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: @@ -69,11 +69,12 @@ func validateCommand(command v1alpha2.Command, parentCommands map[string]string, // 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: ; command: , 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 diff --git a/pkg/validation/commands_test.go b/pkg/validation/commands_test.go index e901608c4..b05f4bb15 100644 --- a/pkg/validation/commands_test.go +++ b/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{ diff --git a/pkg/validation/components.go b/pkg/validation/components.go index 73d290778..5833abc29 100644 --- a/pkg/validation/components.go +++ b/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,37 +64,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 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) } } } @@ -99,16 +103,19 @@ func ValidateComponents(components []v1alpha2.Component) error { } // 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} } diff --git a/pkg/validation/components_test.go b/pkg/validation/components_test.go index 77f0a302e..81a47eb90 100644 --- a/pkg/validation/components_test.go +++ b/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) { diff --git a/pkg/validation/errors.go b/pkg/validation/errors.go index ecca9ff66..667565e3f 100644 --- a/pkg/validation/errors.go +++ b/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 is invalid - , 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 +} diff --git a/pkg/validation/events.go b/pkg/validation/events.go index 93cb084b9..20d4668b5 100644 --- a/pkg/validation/events.go +++ b/pkg/validation/events.go @@ -17,34 +17,34 @@ const ( // ValidateEvents validates all the devfile events func ValidateEvents(events v1alpha2.Events, commands []v1alpha2.Command) error { - eventErrors := "" - commandMap := getCommandsMap(commands) + var eventErrorsList []string switch { case len(events.PreStart) > 0: if preStartErr := isEventValid(events.PreStart, preStart, commandMap); preStartErr != nil { - eventErrors += fmt.Sprintf("\n%s", preStartErr.Error()) + eventErrorsList = append(eventErrorsList, preStartErr.Error()) } fallthrough case len(events.PostStart) > 0: if postStartErr := isEventValid(events.PostStart, postStart, commandMap); postStartErr != nil { - eventErrors += fmt.Sprintf("\n%s", postStartErr.Error()) + eventErrorsList = append(eventErrorsList, postStartErr.Error()) } fallthrough case len(events.PreStop) > 0: if preStopErr := isEventValid(events.PreStop, preStop, commandMap); preStopErr != nil { - eventErrors += fmt.Sprintf("\n%s", preStopErr.Error()) + eventErrorsList = append(eventErrorsList, preStopErr.Error()) } fallthrough case len(events.PostStop) > 0: if postStopErr := isEventValid(events.PostStop, postStop, commandMap); postStopErr != nil { - eventErrors += fmt.Sprintf("\n%s", postStopErr.Error()) + eventErrorsList = append(eventErrorsList, postStopErr.Error()) } } // if there is any validation error, return it - if len(eventErrors) > 0 { + if len(eventErrorsList) > 0 { + eventErrors := fmt.Sprintf("\n%s", strings.Join(eventErrorsList, "\n")) return fmt.Errorf("devfile events validation error: %s", eventErrors) } @@ -83,22 +83,23 @@ func isEventValid(eventNames []string, eventType string, commandMap map[string]v } } - var eventErrors string var err error + var eventErrorsList []string if len(invalidCommand) > 0 { - eventErrors = fmt.Sprintf("\n%s does not map to a valid devfile command", strings.Join(invalidCommand, ", ")) + eventErrorsList = append(eventErrorsList, fmt.Sprintf("%s does not map to a valid devfile command", strings.Join(invalidCommand, ", "))) } if len(invalidApplyEvents) > 0 { - eventErrors += fmt.Sprintf("\n%s should either map to an apply command or a composite command with apply commands", strings.Join(invalidApplyEvents, ", ")) + eventErrorsList = append(eventErrorsList, fmt.Sprintf("%s should either map to an apply command or a composite command with apply commands", strings.Join(invalidApplyEvents, ", "))) } if len(invalidExecEvents) > 0 { - eventErrors += fmt.Sprintf("\n%s should either map to an exec command or a composite command with exec commands", strings.Join(invalidExecEvents, ", ")) + eventErrorsList = append(eventErrorsList, fmt.Sprintf("%s should either map to an exec command or a composite command with exec commands", strings.Join(invalidExecEvents, ", "))) } - if len(eventErrors) != 0 { + if len(eventErrorsList) != 0 { + eventErrors := fmt.Sprintf("\n%s", strings.Join(eventErrorsList, "\n")) err = &InvalidEventError{eventType: eventType, errorMsg: eventErrors} } diff --git a/pkg/validation/events_test.go b/pkg/validation/events_test.go index d7bf5ed81..4d2db5aa4 100644 --- a/pkg/validation/events_test.go +++ b/pkg/validation/events_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/api/v2/pkg/attributes" "github.com/stretchr/testify/assert" ) @@ -12,8 +13,8 @@ func TestValidateEvents(t *testing.T) { containers := []string{"container1", "container2"} commands := []v1alpha2.Command{ - generateDummyApplyCommand("apply1", containers[0], nil), - generateDummyApplyCommand("apply2", containers[0], nil), + generateDummyApplyCommand("apply1", containers[0], nil, attributes.Attributes{}), + generateDummyApplyCommand("apply2", containers[0], nil, attributes.Attributes{}), generateDummyExecCommand("exec1", containers[1], nil), generateDummyExecCommand("exec2", containers[1], nil), generateDummyCompositeCommand("compositeOnlyApply", []string{"apply1", "apply2"}, nil), @@ -112,8 +113,8 @@ func TestIsEventValid(t *testing.T) { containers := []string{"container1", "container2"} commands := []v1alpha2.Command{ - generateDummyApplyCommand("apply1", containers[0], nil), - generateDummyApplyCommand("apply2", containers[0], nil), + generateDummyApplyCommand("apply1", containers[0], nil, attributes.Attributes{}), + generateDummyApplyCommand("apply2", containers[0], nil, attributes.Attributes{}), generateDummyExecCommand("exec1", containers[1], nil), generateDummyExecCommand("exec2", containers[1], nil), generateDummyCompositeCommand("compositeOnlyApply", []string{"apply1", "apply2"}, nil), diff --git a/pkg/validation/projects.go b/pkg/validation/projects.go index ea4e8880d..0a2bfc2a5 100644 --- a/pkg/validation/projects.go +++ b/pkg/validation/projects.go @@ -2,6 +2,7 @@ package validation import ( "fmt" + "strings" "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" ) @@ -10,7 +11,7 @@ import ( // and if the checkout remote matches the renote configured func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error { - var errString string + var projectErrorsList []string for _, starterProject := range starterProjects { var gitSource v1alpha2.GitLikeProjectSource if starterProject.Git != nil { @@ -23,22 +24,28 @@ 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("starterProject %s should have at least one remote", starterProject.Name) + newErr := resolveErrorMessageWithImportAttributes(starterProjectErr, starterProject.Attributes) + projectErrorsList = append(projectErrorsList, 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 := resolveErrorMessageWithImportAttributes(err, starterProject.Attributes) + projectErrorsList = append(projectErrorsList, newErr.Error()) } } default: // len(gitSource.Remotes) >= 2 - errString += fmt.Sprintf("\nstarterProject %s should have one remote only", starterProject.Name) + starterProjectErr := fmt.Errorf("starterProject %s should have one remote only", starterProject.Name) + newErr := resolveErrorMessageWithImportAttributes(starterProjectErr, starterProject.Attributes) + projectErrorsList = append(projectErrorsList, newErr.Error()) } } var err error - if len(errString) > 0 { - err = fmt.Errorf("error validating starter projects:%s", errString) + if len(projectErrorsList) > 0 { + projectErrors := fmt.Sprintf("\n%s", strings.Join(projectErrorsList, "\n")) + err = fmt.Errorf("error validating starter projects:%s", projectErrors) } return err @@ -48,7 +55,7 @@ func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error { // remote is mandatory and if the checkout remote matches the renote configured func ValidateProjects(projects []v1alpha2.Project) error { - var errString string + var projectErrorsList []string for _, project := range projects { var gitSource v1alpha2.GitLikeProjectSource if project.Git != nil { @@ -58,30 +65,36 @@ func ValidateProjects(projects []v1alpha2.Project) error { } else { continue } - switch len(gitSource.Remotes) { case 0: - errString += fmt.Sprintf("\nprojects %s should have at least one remote", project.Name) + projectErr := fmt.Errorf("projects %s should have at least one remote", project.Name) + newErr := resolveErrorMessageWithImportAttributes(projectErr, project.Attributes) + projectErrorsList = append(projectErrorsList, 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 := resolveErrorMessageWithImportAttributes(err, project.Attributes) + projectErrorsList = append(projectErrorsList, 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("project %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name) + newErr := resolveErrorMessageWithImportAttributes(projectErr, project.Attributes) + projectErrorsList = append(projectErrorsList, newErr.Error()) continue } if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil { - errString += fmt.Sprintf("\n%s", err.Error()) + newErr := resolveErrorMessageWithImportAttributes(err, project.Attributes) + projectErrorsList = append(projectErrorsList, newErr.Error()) } } } var err error - if len(errString) > 0 { - err = fmt.Errorf("error validating projects:%s", errString) + if len(projectErrorsList) > 0 { + projectErrors := fmt.Sprintf("\n%s", strings.Join(projectErrorsList, "\n")) + err = fmt.Errorf("error validating projects:%s", projectErrors) } return err diff --git a/pkg/validation/projects_test.go b/pkg/validation/projects_test.go index c32853e71..fd2549b82 100644 --- a/pkg/validation/projects_test.go +++ b/pkg/validation/projects_test.go @@ -1,15 +1,17 @@ package validation import ( + "github.com/devfile/api/v2/pkg/attributes" "testing" "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/stretchr/testify/assert" ) -func generateDummyGitStarterProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string) v1alpha2.StarterProject { +func generateDummyGitStarterProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string, projectAttribute attributes.Attributes) v1alpha2.StarterProject { return v1alpha2.StarterProject{ - Name: name, + Attributes: projectAttribute, + Name: name, ProjectSource: v1alpha2.ProjectSource{ Git: &v1alpha2.GitProjectSource{ GitLikeProjectSource: v1alpha2.GitLikeProjectSource{ @@ -35,9 +37,10 @@ func generateDummyGithubStarterProject(name string, checkoutRemote *v1alpha2.Che } } -func generateDummyGitProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string) v1alpha2.Project { +func generateDummyGitProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string, projectAttribute attributes.Attributes) v1alpha2.Project { return v1alpha2.Project{ - Name: name, + Attributes: projectAttribute, + Name: name, ProjectSource: v1alpha2.ProjectSource{ Git: &v1alpha2.GitProjectSource{ GitLikeProjectSource: v1alpha2.GitLikeProjectSource{ @@ -69,6 +72,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 @@ -77,8 +84,8 @@ func TestValidateStarterProjects(t *testing.T) { { name: "Valid Starter Project", starterProjects: []v1alpha2.StarterProject{ - generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}), - generateDummyGitStarterProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote2"}), + generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}, attributes.Attributes{}), + generateDummyGitStarterProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote2"}, attributes.Attributes{}), }, }, { @@ -98,13 +105,13 @@ func TestValidateStarterProjects(t *testing.T) { { name: "Valid Starter Project with empty checkout remote", starterProjects: []v1alpha2.StarterProject{ - generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote"}), + generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote"}, attributes.Attributes{}), }, }, { name: "Valid Starter Project with no checkout remote", starterProjects: []v1alpha2.StarterProject{ - generateDummyGitStarterProject("project1", nil, map[string]string{"origin": "originremote"}), + generateDummyGitStarterProject("project1", nil, map[string]string{"origin": "originremote"}, attributes.Attributes{}), }, }, { @@ -115,6 +122,13 @@ func TestValidateStarterProjects(t *testing.T) { }, wantErr: &atleastOneRemoteErr, }, + { + name: "Invalid Starter Project due to wrong checkout with import source attributes", + starterProjects: []v1alpha2.StarterProject{ + generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"test": "testremote"}, parentOverridesFromMainDevfile), + }, + wantErr: &wrongCheckoutErrWithImportAttributes, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -135,6 +149,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 @@ -143,7 +161,7 @@ func TestValidateProjects(t *testing.T) { { name: "Valid Project", projects: []v1alpha2.Project{ - generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}), + generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}, attributes.Attributes{}), generateDummyGithubProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}), }, }, @@ -157,7 +175,7 @@ func TestValidateProjects(t *testing.T) { { name: "Invalid Project with multiple remote and empty checkout remote", projects: []v1alpha2.Project{ - generateDummyGitProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}), + generateDummyGitProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}, attributes.Attributes{}), generateDummyGithubProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote", "test": "testremote"}), }, wantErr: &missingCheckOutFromRemoteErr, @@ -166,24 +184,31 @@ func TestValidateProjects(t *testing.T) { name: "Invalid Project with wrong checkout", projects: []v1alpha2.Project{ generateDummyGithubProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin1"}, map[string]string{"origin": "originremote", "test": "testremote"}), - generateDummyGitProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin1"}, map[string]string{"origin2": "originremote2"}), + generateDummyGitProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin1"}, map[string]string{"origin2": "originremote2"}, attributes.Attributes{}), }, wantErr: &wrongCheckoutErr, }, { name: "Valid Project with empty checkout remote", projects: []v1alpha2.Project{ - generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote"}), + generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote"}, attributes.Attributes{}), }, }, { name: "Invalid Project with empty remotes", projects: []v1alpha2.Project{ - generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{}), + generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{}, attributes.Attributes{}), generateDummyGithubProject("project2", &v1alpha2.CheckoutFrom{Remote: "origins"}, map[string]string{"origin": "originremote", "test": "testremote"}), }, wantErr: &atleastOneRemoteErr, }, + { + name: "Invalid Project due to wrong checkout with import source attributes", + projects: []v1alpha2.Project{ + generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"test": "testremote"}, parentOverridesFromMainDevfile), + }, + wantErr: &wrongCheckoutErrWithImportAttributes, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/validation/utils.go b/pkg/validation/utils.go index e8de8ad38..6f0ec12df 100644 --- a/pkg/validation/utils.go +++ b/pkg/validation/utils.go @@ -7,6 +7,17 @@ import ( "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" ) +// attribute keys for imported and overridden elements +// the value of those keys is the resource information +const ( + // attribute key of the imported element resource information + ImportSourceAttribute = "api.devfile.io/imported-from" + // attribute key of the parent overridden element resource information + ParentOverrideAttribute = "api.devfile.io/parent-override-from" + // attribute key of the plugin overridden element resource information + PluginOverrideAttribute = "api.devfile.io/plugin-override-from" +) + // getCommandsMap iterates through the commands and returns a map of command func getCommandsMap(commands []v1alpha2.Command) map[string]v1alpha2.Command { commandMap := make(map[string]v1alpha2.Command, len(commands))