diff --git a/pkg/validation/commands.go b/pkg/validation/commands.go index eaea7e125..e48fbf6a0 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 resolveErrorMessageWithImportArrtibutes(err, command.Attributes) + return resolveErrorMessageWithImportAttributes(err, command.Attributes) } commandGroup := getGroup(command) @@ -68,11 +68,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 { @@ -82,7 +83,13 @@ 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 commandsReference string + for _, command := range defaultCommands { + commandsReference += resolveErrorMessageWithImportAttributes(fmt.Errorf("; command: %s", command.Id), command.Attributes).Error() + } + // 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 3dfa59817..b05f4bb15 100644 --- a/pkg/validation/commands_test.go +++ b/pkg/validation/commands_test.go @@ -31,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{ @@ -136,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, }, @@ -152,20 +153,7 @@ func TestValidateCommands(t *testing.T) { { 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", - }, - }, - }, + generateDummyApplyCommand("command", "invalidComponent", nil, parentOverridesFromMainDevfile), }, wantErr: &invalidCmdErrWithImportAttributes, }, @@ -218,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, }, } @@ -332,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 @@ -347,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 c07f3e821..5c4b40470 100644 --- a/pkg/validation/components.go +++ b/pkg/validation/components.go @@ -54,7 +54,7 @@ func ValidateComponents(components []v1alpha2.Component) error { err := validateEndpoints(component.Container.Endpoints, processedEndPointPort, processedEndPointName) if err != nil { - return resolveErrorMessageWithImportArrtibutes(err, component.Attributes) + return resolveErrorMessageWithImportAttributes(err, component.Attributes) } case component.Volume != nil: processedVolumes[component.Name] = true @@ -64,37 +64,37 @@ func ValidateComponents(components []v1alpha2.Component) error { // https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ if _, err := resource.ParseQuantity(component.Volume.Size); err != nil { 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) + return resolveErrorMessageWithImportAttributes(invalidVolErr, component.Attributes) } } case component.Openshift != nil: if component.Openshift.Uri != "" { err := ValidateURI(component.Openshift.Uri) if err != nil { - return resolveErrorMessageWithImportArrtibutes(err, component.Attributes) + return resolveErrorMessageWithImportAttributes(err, component.Attributes) } } err := validateEndpoints(component.Openshift.Endpoints, processedEndPointPort, processedEndPointName) if err != nil { - return resolveErrorMessageWithImportArrtibutes(err, component.Attributes) + return resolveErrorMessageWithImportAttributes(err, component.Attributes) } case component.Kubernetes != nil: if component.Kubernetes.Uri != "" { err := ValidateURI(component.Kubernetes.Uri) if err != nil { - return resolveErrorMessageWithImportArrtibutes(err, component.Attributes) + return resolveErrorMessageWithImportAttributes(err, component.Attributes) } } err := validateEndpoints(component.Kubernetes.Endpoints, processedEndPointPort, processedEndPointName) if err != nil { - return resolveErrorMessageWithImportArrtibutes(err, component.Attributes) + return resolveErrorMessageWithImportAttributes(err, component.Attributes) } case component.Plugin != nil: if component.Plugin.RegistryUrl != "" { err := ValidateURI(component.Plugin.RegistryUrl) if err != nil { - return resolveErrorMessageWithImportArrtibutes(err, component.Attributes) + return resolveErrorMessageWithImportAttributes(err, component.Attributes) } } } @@ -107,7 +107,7 @@ func ValidateComponents(components []v1alpha2.Component) error { for _, volumeMountName := range volumeMountNames { if !processedVolumes[volumeMountName] { missingVolumeMountErr := fmt.Errorf("\nvolume mount %s belonging to the container component %s", volumeMountName, componentName) - newErr := resolveErrorMessageWithImportArrtibutes(missingVolumeMountErr, processedComponentWithVolumeMounts[componentName].Attributes) + newErr := resolveErrorMessageWithImportAttributes(missingVolumeMountErr, processedComponentWithVolumeMounts[componentName].Attributes) invalidVolumeMountsErr += newErr.Error() } } diff --git a/pkg/validation/components_test.go b/pkg/validation/components_test.go index 9aa955504..81a47eb90 100644 --- a/pkg/validation/components_test.go +++ b/pkg/validation/components_test.go @@ -80,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{ @@ -238,24 +239,14 @@ 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{ - { - Attributes: pluginOverridesFromMainDevfile, - Name: "name", - ComponentUnion: v1alpha2.ComponentUnion{ - Plugin: &v1alpha2.PluginComponent{ - ImportReference: v1alpha2.ImportReference{ - RegistryUrl: "http//invalidregistryurl", - }, - }, - }, - }, + generateDummyPluginComponent("abc", "http//invalidregistryurl", pluginOverridesFromMainDevfile), }, wantErr: &invalidURIErrWithImportAttributes, }, diff --git a/pkg/validation/errors.go b/pkg/validation/errors.go index 80efe5b7f..a87b8d723 100644 --- a/pkg/validation/errors.go +++ b/pkg/validation/errors.go @@ -90,11 +90,11 @@ func (e *InvalidComponentError) Error() string { return fmt.Sprintf("the component %q is invalid - %s", e.componentName, e.reason) } -// resolveErrorMessageWithImportArrtibutes returns an updated error message +// 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 resolveErrorMessageWithImportArrtibutes(validationErr error, attributes attributesAPI.Attributes) error { +func resolveErrorMessageWithImportAttributes(validationErr error, attributes attributesAPI.Attributes) error { var findKeyErr error importReference := attributes.Get(ImportSourceAttribute, &findKeyErr) 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 a62ec6c7c..b520674cd 100644 --- a/pkg/validation/projects.go +++ b/pkg/validation/projects.go @@ -24,19 +24,19 @@ func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error { switch len(gitSource.Remotes) { case 0: starterProjectErr := fmt.Errorf("\nstarterProject %s should have at least one remote", starterProject.Name) - newErr := resolveErrorMessageWithImportArrtibutes(starterProjectErr, starterProject.Attributes) + newErr := resolveErrorMessageWithImportAttributes(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 { - newErr := resolveErrorMessageWithImportArrtibutes(err, starterProject.Attributes) + newErr := resolveErrorMessageWithImportAttributes(err, starterProject.Attributes) errString += fmt.Sprintf("\n%s", newErr.Error()) } } default: // len(gitSource.Remotes) >= 2 starterProjectErr := fmt.Errorf("\nstarterProject %s should have one remote only", starterProject.Name) - newErr := resolveErrorMessageWithImportArrtibutes(starterProjectErr, starterProject.Attributes) + newErr := resolveErrorMessageWithImportAttributes(starterProjectErr, starterProject.Attributes) errString += newErr.Error() } } @@ -67,24 +67,24 @@ func ValidateProjects(projects []v1alpha2.Project) error { switch len(gitSource.Remotes) { case 0: projectErr := fmt.Errorf("\nprojects %s should have at least one remote", project.Name) - newErr := resolveErrorMessageWithImportArrtibutes(projectErr, project.Attributes) + newErr := resolveErrorMessageWithImportAttributes(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 { - newErr := resolveErrorMessageWithImportArrtibutes(err, project.Attributes) + newErr := resolveErrorMessageWithImportAttributes(err, project.Attributes) errString += fmt.Sprintf("\n%s", newErr.Error()) } } default: // len(gitSource.Remotes) >= 2 if gitSource.CheckoutFrom == nil || gitSource.CheckoutFrom.Remote == "" { projectErr := fmt.Errorf("\nproject %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name) - newErr := resolveErrorMessageWithImportArrtibutes(projectErr, project.Attributes) + newErr := resolveErrorMessageWithImportAttributes(projectErr, project.Attributes) errString += newErr.Error() continue } if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil { - newErr := resolveErrorMessageWithImportArrtibutes(err, project.Attributes) + newErr := resolveErrorMessageWithImportAttributes(err, project.Attributes) errString += fmt.Sprintf("\n%s", newErr.Error()) } } diff --git a/pkg/validation/projects_test.go b/pkg/validation/projects_test.go index 6acc879ec..fd2549b82 100644 --- a/pkg/validation/projects_test.go +++ b/pkg/validation/projects_test.go @@ -8,9 +8,10 @@ import ( "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{ @@ -36,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{ @@ -82,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{}), }, }, { @@ -103,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{}), }, }, { @@ -123,18 +125,7 @@ func TestValidateStarterProjects(t *testing.T) { { 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"}, - }, - }, - }, - }, + generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"test": "testremote"}, parentOverridesFromMainDevfile), }, wantErr: &wrongCheckoutErrWithImportAttributes, }, @@ -170,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"}), }, }, @@ -184,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, @@ -193,20 +184,20 @@ 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, @@ -214,18 +205,7 @@ func TestValidateProjects(t *testing.T) { { 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"}, - }, - }, - }, - }, + generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"test": "testremote"}, parentOverridesFromMainDevfile), }, wantErr: &wrongCheckoutErrWithImportAttributes, },