Skip to content

Commit a22cea8

Browse files
yangcao77yangcao77
committed
check for attribute and print out all default command and it's import reference if any
Signed-off-by: Stephanie <[email protected]>
1 parent 4ba7837 commit a22cea8

File tree

8 files changed

+76
-95
lines changed

8 files changed

+76
-95
lines changed

‎pkg/validation/commands.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func ValidateCommands(commands []v1alpha2.Command, components []v1alpha2.Compone
2525
parentCommands := make(map[string]string)
2626
err = validateCommand(command, parentCommands, commandMap, components)
2727
if err != nil {
28-
return resolveErrorMessageWithImportArrtibutes(err, command.Attributes)
28+
return resolveErrorMessageWithImportAttributes(err, command.Attributes)
2929
}
3030

3131
commandGroup := getGroup(command)
@@ -68,11 +68,12 @@ func validateCommand(command v1alpha2.Command, parentCommands map[string]string,
6868
// 2. with more than one default, err out
6969
func validateGroup(commands []v1alpha2.Command) error {
7070
defaultCommandCount := 0
71-
71+
var defaultCommands []v1alpha2.Command
7272
if len(commands) > 1 {
7373
for _, command := range commands {
7474
if getGroup(command).IsDefault {
7575
defaultCommandCount++
76+
defaultCommands = append(defaultCommands, command)
7677
}
7778
}
7879
} else {
@@ -82,7 +83,13 @@ func validateGroup(commands []v1alpha2.Command) error {
8283
if defaultCommandCount == 0 {
8384
return fmt.Errorf("there should be exactly one default command, currently there is no default command")
8485
} else if defaultCommandCount > 1 {
85-
return fmt.Errorf("there should be exactly one default command, currently there is more than one default command")
86+
var commandsReference string
87+
for _, command := range defaultCommands {
88+
commandsReference += resolveErrorMessageWithImportAttributes(fmt.Errorf("; command: %s", command.Id), command.Attributes).Error()
89+
}
90+
// example: there should be exactly one default command, currently there is more than one default command;
91+
// command: <id1>; command: <id2>, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
92+
return fmt.Errorf("there should be exactly one default command, currently there is more than one default command%s", commandsReference)
8693
}
8794

8895
return nil

‎pkg/validation/commands_test.go

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ func generateDummyExecCommand(name, component string, group *v1alpha2.CommandGro
3131
}
3232

3333
// generateDummyExecCommand returns a dummy apply command for testing
34-
func generateDummyApplyCommand(name, component string, group *v1alpha2.CommandGroup) v1alpha2.Command {
34+
func generateDummyApplyCommand(name, component string, group *v1alpha2.CommandGroup, cmdAttributes attributes.Attributes) v1alpha2.Command {
3535
return v1alpha2.Command{
36-
Id: name,
36+
Attributes: cmdAttributes,
37+
Id: name,
3738
CommandUnion: v1alpha2.CommandUnion{
3839
Apply: &v1alpha2.ApplyCommand{
3940
LabeledCommand: v1alpha2.LabeledCommand{
@@ -136,7 +137,7 @@ func TestValidateCommands(t *testing.T) {
136137
{
137138
name: "Invalid Apply command with wrong component",
138139
commands: []v1alpha2.Command{
139-
generateDummyApplyCommand("command", "invalidComponent", nil),
140+
generateDummyApplyCommand("command", "invalidComponent", nil, attributes.Attributes{}),
140141
},
141142
wantErr: &invalidCmdErr,
142143
},
@@ -152,20 +153,7 @@ func TestValidateCommands(t *testing.T) {
152153
{
153154
name: "Invalid command with import source attribute",
154155
commands: []v1alpha2.Command{
155-
{
156-
Attributes: parentOverridesFromMainDevfile,
157-
Id: "command",
158-
CommandUnion: v1alpha2.CommandUnion{
159-
Apply: &v1alpha2.ApplyCommand{
160-
LabeledCommand: v1alpha2.LabeledCommand{
161-
BaseCommand: v1alpha2.BaseCommand{
162-
Group: nil,
163-
},
164-
},
165-
Component: "invalidComponent",
166-
},
167-
},
168-
},
156+
generateDummyApplyCommand("command", "invalidComponent", nil, parentOverridesFromMainDevfile),
169157
},
170158
wantErr: &invalidCmdErrWithImportAttributes,
171159
},
@@ -218,11 +206,11 @@ func TestValidateCommandComponent(t *testing.T) {
218206
},
219207
{
220208
name: "Valid Apply Command",
221-
command: generateDummyApplyCommand("command", component, nil),
209+
command: generateDummyApplyCommand("command", component, nil, attributes.Attributes{}),
222210
},
223211
{
224212
name: "Invalid Apply Command with wrong component",
225-
command: generateDummyApplyCommand("command", invalidComponent, &v1alpha2.CommandGroup{Kind: runGroup}),
213+
command: generateDummyApplyCommand("command", invalidComponent, &v1alpha2.CommandGroup{Kind: runGroup}, attributes.Attributes{}),
226214
wantErr: &invalidCmdErr,
227215
},
228216
}
@@ -332,7 +320,13 @@ func TestValidateGroup(t *testing.T) {
332320
component := "alias1"
333321

334322
noDefaultCmdErr := ".*there should be exactly one default command, currently there is no default command"
335-
multipleDefaultCmdErr := ".*there should be exactly one default command, currently there is more than one default command"
323+
multipleDefaultError := ".*there should be exactly one default command, currently there is more than one default command"
324+
multipleDefaultCmdErr := multipleDefaultError + "; command: run command; command: customcommand"
325+
326+
parentOverridesFromMainDevfile := attributes.Attributes{}.PutString(ImportSourceAttribute,
327+
"uri: http://127.0.0.1:8080").PutString(ParentOverrideAttribute, "main devfile")
328+
multipleDefaultCmdErrWithImportAttributes := multipleDefaultError +
329+
"; command: run command; command: customcommand, imported from uri: http://127.0.0.1:8080, in parent overrides from main devfile"
336330

337331
tests := []struct {
338332
name string
@@ -347,6 +341,14 @@ func TestValidateGroup(t *testing.T) {
347341
},
348342
wantErr: &multipleDefaultCmdErr,
349343
},
344+
{
345+
name: "Two default run commands with import source attribute",
346+
commands: []v1alpha2.Command{
347+
generateDummyExecCommand("run command", component, &v1alpha2.CommandGroup{Kind: runGroup, IsDefault: true}),
348+
generateDummyApplyCommand("customcommand", component, &v1alpha2.CommandGroup{Kind: runGroup, IsDefault: true}, parentOverridesFromMainDevfile),
349+
},
350+
wantErr: &multipleDefaultCmdErrWithImportAttributes,
351+
},
350352
{
351353
name: "No default for more than one build commands",
352354
commands: []v1alpha2.Command{

‎pkg/validation/components.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func ValidateComponents(components []v1alpha2.Component) error {
5454

5555
err := validateEndpoints(component.Container.Endpoints, processedEndPointPort, processedEndPointName)
5656
if err != nil {
57-
return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
57+
return resolveErrorMessageWithImportAttributes(err, component.Attributes)
5858
}
5959
case component.Volume != nil:
6060
processedVolumes[component.Name] = true
@@ -64,37 +64,37 @@ func ValidateComponents(components []v1alpha2.Component) error {
6464
// https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
6565
if _, err := resource.ParseQuantity(component.Volume.Size); err != nil {
6666
invalidVolErr := &InvalidVolumeError{name: component.Name, reason: fmt.Sprintf("size %s for volume component is invalid, %v. Example - 2Gi, 1024Mi", component.Volume.Size, err)}
67-
return resolveErrorMessageWithImportArrtibutes(invalidVolErr, component.Attributes)
67+
return resolveErrorMessageWithImportAttributes(invalidVolErr, component.Attributes)
6868
}
6969
}
7070
case component.Openshift != nil:
7171
if component.Openshift.Uri != "" {
7272
err := ValidateURI(component.Openshift.Uri)
7373
if err != nil {
74-
return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
74+
return resolveErrorMessageWithImportAttributes(err, component.Attributes)
7575
}
7676
}
7777

7878
err := validateEndpoints(component.Openshift.Endpoints, processedEndPointPort, processedEndPointName)
7979
if err != nil {
80-
return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
80+
return resolveErrorMessageWithImportAttributes(err, component.Attributes)
8181
}
8282
case component.Kubernetes != nil:
8383
if component.Kubernetes.Uri != "" {
8484
err := ValidateURI(component.Kubernetes.Uri)
8585
if err != nil {
86-
return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
86+
return resolveErrorMessageWithImportAttributes(err, component.Attributes)
8787
}
8888
}
8989
err := validateEndpoints(component.Kubernetes.Endpoints, processedEndPointPort, processedEndPointName)
9090
if err != nil {
91-
return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
91+
return resolveErrorMessageWithImportAttributes(err, component.Attributes)
9292
}
9393
case component.Plugin != nil:
9494
if component.Plugin.RegistryUrl != "" {
9595
err := ValidateURI(component.Plugin.RegistryUrl)
9696
if err != nil {
97-
return resolveErrorMessageWithImportArrtibutes(err, component.Attributes)
97+
return resolveErrorMessageWithImportAttributes(err, component.Attributes)
9898
}
9999
}
100100
}
@@ -107,7 +107,7 @@ func ValidateComponents(components []v1alpha2.Component) error {
107107
for _, volumeMountName := range volumeMountNames {
108108
if !processedVolumes[volumeMountName] {
109109
missingVolumeMountErr := fmt.Errorf("\nvolume mount %s belonging to the container component %s", volumeMountName, componentName)
110-
newErr := resolveErrorMessageWithImportArrtibutes(missingVolumeMountErr, processedComponentWithVolumeMounts[componentName].Attributes)
110+
newErr := resolveErrorMessageWithImportAttributes(missingVolumeMountErr, processedComponentWithVolumeMounts[componentName].Attributes)
111111
invalidVolumeMountsErr += newErr.Error()
112112
}
113113
}

‎pkg/validation/components_test.go

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,11 @@ func generateDummyKubernetesComponent(name string, endpoints []v1alpha2.Endpoint
8080
}
8181

8282
// generateDummyPluginComponent returns a dummy Plugin component for testing
83-
func generateDummyPluginComponent(name, url string) v1alpha2.Component {
83+
func generateDummyPluginComponent(name, url string, compAttribute attributes.Attributes) v1alpha2.Component {
8484

8585
return v1alpha2.Component{
86-
Name: name,
86+
Attributes: compAttribute,
87+
Name: name,
8788
ComponentUnion: v1alpha2.ComponentUnion{
8889
Plugin: &v1alpha2.PluginComponent{
8990
ImportReference: v1alpha2.ImportReference{
@@ -238,24 +239,14 @@ func TestValidateComponents(t *testing.T) {
238239
{
239240
name: "Invalid plugin registry url",
240241
components: []v1alpha2.Component{
241-
generateDummyPluginComponent("abc", "http//invalidregistryurl"),
242+
generateDummyPluginComponent("abc", "http//invalidregistryurl", attributes.Attributes{}),
242243
},
243244
wantErr: &invalidURIErr,
244245
},
245246
{
246247
name: "Invalid component due to bad URI with import source attributes",
247248
components: []v1alpha2.Component{
248-
{
249-
Attributes: pluginOverridesFromMainDevfile,
250-
Name: "name",
251-
ComponentUnion: v1alpha2.ComponentUnion{
252-
Plugin: &v1alpha2.PluginComponent{
253-
ImportReference: v1alpha2.ImportReference{
254-
RegistryUrl: "http//invalidregistryurl",
255-
},
256-
},
257-
},
258-
},
249+
generateDummyPluginComponent("abc", "http//invalidregistryurl", pluginOverridesFromMainDevfile),
259250
},
260251
wantErr: &invalidURIErrWithImportAttributes,
261252
},

‎pkg/validation/errors.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,11 @@ func (e *InvalidComponentError) Error() string {
9090
return fmt.Sprintf("the component %q is invalid - %s", e.componentName, e.reason)
9191
}
9292

93-
// resolveErrorMessageWithImportArrtibutes returns an updated error message
93+
// resolveErrorMessageWithImportAttributes returns an updated error message
9494
// with detailed information on the imported and overriden resource.
9595
// example:
9696
// "the component <compName> is invalid - <reason>, imported from Uri: http://example.com/devfile.yaml, in parent overrides from main devfile"
97-
func resolveErrorMessageWithImportArrtibutes(validationErr error, attributes attributesAPI.Attributes) error {
97+
func resolveErrorMessageWithImportAttributes(validationErr error, attributes attributesAPI.Attributes) error {
9898
var findKeyErr error
9999
importReference := attributes.Get(ImportSourceAttribute, &findKeyErr)
100100

‎pkg/validation/events_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55

66
"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
7+
"github.com/devfile/api/v2/pkg/attributes"
78
"github.com/stretchr/testify/assert"
89
)
910

@@ -12,8 +13,8 @@ func TestValidateEvents(t *testing.T) {
1213
containers := []string{"container1", "container2"}
1314

1415
commands := []v1alpha2.Command{
15-
generateDummyApplyCommand("apply1", containers[0], nil),
16-
generateDummyApplyCommand("apply2", containers[0], nil),
16+
generateDummyApplyCommand("apply1", containers[0], nil, attributes.Attributes{}),
17+
generateDummyApplyCommand("apply2", containers[0], nil, attributes.Attributes{}),
1718
generateDummyExecCommand("exec1", containers[1], nil),
1819
generateDummyExecCommand("exec2", containers[1], nil),
1920
generateDummyCompositeCommand("compositeOnlyApply", []string{"apply1", "apply2"}, nil),
@@ -112,8 +113,8 @@ func TestIsEventValid(t *testing.T) {
112113
containers := []string{"container1", "container2"}
113114

114115
commands := []v1alpha2.Command{
115-
generateDummyApplyCommand("apply1", containers[0], nil),
116-
generateDummyApplyCommand("apply2", containers[0], nil),
116+
generateDummyApplyCommand("apply1", containers[0], nil, attributes.Attributes{}),
117+
generateDummyApplyCommand("apply2", containers[0], nil, attributes.Attributes{}),
117118
generateDummyExecCommand("exec1", containers[1], nil),
118119
generateDummyExecCommand("exec2", containers[1], nil),
119120
generateDummyCompositeCommand("compositeOnlyApply", []string{"apply1", "apply2"}, nil),

‎pkg/validation/projects.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,19 @@ func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error {
2424
switch len(gitSource.Remotes) {
2525
case 0:
2626
starterProjectErr := fmt.Errorf("\nstarterProject %s should have at least one remote", starterProject.Name)
27-
newErr := resolveErrorMessageWithImportArrtibutes(starterProjectErr, starterProject.Attributes)
27+
newErr := resolveErrorMessageWithImportAttributes(starterProjectErr, starterProject.Attributes)
2828
errString += newErr.Error()
2929
case 1:
3030
if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
3131
err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, starterProject.Name)
3232
if err != nil {
33-
newErr := resolveErrorMessageWithImportArrtibutes(err, starterProject.Attributes)
33+
newErr := resolveErrorMessageWithImportAttributes(err, starterProject.Attributes)
3434
errString += fmt.Sprintf("\n%s", newErr.Error())
3535
}
3636
}
3737
default: // len(gitSource.Remotes) >= 2
3838
starterProjectErr := fmt.Errorf("\nstarterProject %s should have one remote only", starterProject.Name)
39-
newErr := resolveErrorMessageWithImportArrtibutes(starterProjectErr, starterProject.Attributes)
39+
newErr := resolveErrorMessageWithImportAttributes(starterProjectErr, starterProject.Attributes)
4040
errString += newErr.Error()
4141
}
4242
}
@@ -67,24 +67,24 @@ func ValidateProjects(projects []v1alpha2.Project) error {
6767
switch len(gitSource.Remotes) {
6868
case 0:
6969
projectErr := fmt.Errorf("\nprojects %s should have at least one remote", project.Name)
70-
newErr := resolveErrorMessageWithImportArrtibutes(projectErr, project.Attributes)
70+
newErr := resolveErrorMessageWithImportAttributes(projectErr, project.Attributes)
7171
errString += newErr.Error()
7272
case 1:
7373
if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
7474
if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
75-
newErr := resolveErrorMessageWithImportArrtibutes(err, project.Attributes)
75+
newErr := resolveErrorMessageWithImportAttributes(err, project.Attributes)
7676
errString += fmt.Sprintf("\n%s", newErr.Error())
7777
}
7878
}
7979
default: // len(gitSource.Remotes) >= 2
8080
if gitSource.CheckoutFrom == nil || gitSource.CheckoutFrom.Remote == "" {
8181
projectErr := fmt.Errorf("\nproject %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name)
82-
newErr := resolveErrorMessageWithImportArrtibutes(projectErr, project.Attributes)
82+
newErr := resolveErrorMessageWithImportAttributes(projectErr, project.Attributes)
8383
errString += newErr.Error()
8484
continue
8585
}
8686
if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
87-
newErr := resolveErrorMessageWithImportArrtibutes(err, project.Attributes)
87+
newErr := resolveErrorMessageWithImportAttributes(err, project.Attributes)
8888
errString += fmt.Sprintf("\n%s", newErr.Error())
8989
}
9090
}

0 commit comments

Comments
 (0)