Skip to content

Commit 33a78ae

Browse files
yangcao77yangcao77
authored
project & starterproject validation bug (#341)
* fix the dereference Signed-off-by: Stephanie <[email protected]> * add more unit tests, increase test coverage for invalid cases Signed-off-by: Stephanie <[email protected]> * fix gofmt Signed-off-by: Stephanie <[email protected]> * update the logic into switch format for better understanding Signed-off-by: Stephanie <[email protected]>
1 parent 019d88e commit 33a78ae

File tree

2 files changed

+67
-48
lines changed

2 files changed

+67
-48
lines changed

‎pkg/validation/projects.go‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,18 @@ func ValidateStarterProjects(starterProjects []v1alpha2.StarterProject) error {
2121
continue
2222
}
2323

24-
if len(gitSource.Remotes) == 0 {
24+
switch len(gitSource.Remotes) {
25+
case 0:
2526
errString += fmt.Sprintf("\nstarterProject %s should have at least one remote", starterProject.Name)
26-
} else if len(gitSource.Remotes) > 1 {
27-
errString += fmt.Sprintf("\nstarterProject %s should have one remote only", starterProject.Name)
28-
} else if gitSource.CheckoutFrom.Remote != "" {
29-
err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, starterProject.Name)
30-
if err != nil {
31-
errString += fmt.Sprintf("\n%s", err.Error())
27+
case 1:
28+
if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
29+
err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, starterProject.Name)
30+
if err != nil {
31+
errString += fmt.Sprintf("\n%s", err.Error())
32+
}
3233
}
34+
default: // len(gitSource.Remotes) >= 2
35+
errString += fmt.Sprintf("\nstarterProject %s should have one remote only", starterProject.Name)
3336
}
3437
}
3538

@@ -56,11 +59,21 @@ func ValidateProjects(projects []v1alpha2.Project) error {
5659
continue
5760
}
5861

59-
if len(gitSource.Remotes) == 0 {
62+
switch len(gitSource.Remotes) {
63+
case 0:
6064
errString += fmt.Sprintf("\nprojects %s should have at least one remote", project.Name)
61-
} else if len(gitSource.Remotes) > 1 || gitSource.CheckoutFrom.Remote != "" {
62-
err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name)
63-
if err != nil {
65+
case 1:
66+
if gitSource.CheckoutFrom != nil && gitSource.CheckoutFrom.Remote != "" {
67+
if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
68+
errString += fmt.Sprintf("\n%s", err.Error())
69+
}
70+
}
71+
default: // len(gitSource.Remotes) >= 2
72+
if gitSource.CheckoutFrom == nil || gitSource.CheckoutFrom.Remote == "" {
73+
errString += fmt.Sprintf("\nproject %s has more than one remote defined, but has no checkoutfrom remote defined", project.Name)
74+
continue
75+
}
76+
if err := validateRemoteMap(gitSource.Remotes, gitSource.CheckoutFrom.Remote, project.Name); err != nil {
6477
errString += fmt.Sprintf("\n%s", err.Error())
6578
}
6679
}

‎pkg/validation/projects_test.go‎

Lines changed: 43 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,64 +7,56 @@ import (
77
"github.com/stretchr/testify/assert"
88
)
99

10-
func generateDummyGitStarterProject(name, checkoutRemote string, remotes map[string]string) v1alpha2.StarterProject {
10+
func generateDummyGitStarterProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string) v1alpha2.StarterProject {
1111
return v1alpha2.StarterProject{
1212
Name: name,
1313
ProjectSource: v1alpha2.ProjectSource{
1414
Git: &v1alpha2.GitProjectSource{
1515
GitLikeProjectSource: v1alpha2.GitLikeProjectSource{
16-
Remotes: remotes,
17-
CheckoutFrom: &v1alpha2.CheckoutFrom{
18-
Remote: checkoutRemote,
19-
},
16+
Remotes: remotes,
17+
CheckoutFrom: checkoutRemote,
2018
},
2119
},
2220
},
2321
}
2422
}
2523

26-
func generateDummyGithubStarterProject(name, checkoutRemote string, remotes map[string]string) v1alpha2.StarterProject {
24+
func generateDummyGithubStarterProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string) v1alpha2.StarterProject {
2725
return v1alpha2.StarterProject{
2826
Name: name,
2927
ProjectSource: v1alpha2.ProjectSource{
3028
Github: &v1alpha2.GithubProjectSource{
3129
GitLikeProjectSource: v1alpha2.GitLikeProjectSource{
32-
Remotes: remotes,
33-
CheckoutFrom: &v1alpha2.CheckoutFrom{
34-
Remote: checkoutRemote,
35-
},
30+
Remotes: remotes,
31+
CheckoutFrom: checkoutRemote,
3632
},
3733
},
3834
},
3935
}
4036
}
4137

42-
func generateDummyGitProject(name, checkoutRemote string, remotes map[string]string) v1alpha2.Project {
38+
func generateDummyGitProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string) v1alpha2.Project {
4339
return v1alpha2.Project{
4440
Name: name,
4541
ProjectSource: v1alpha2.ProjectSource{
4642
Git: &v1alpha2.GitProjectSource{
4743
GitLikeProjectSource: v1alpha2.GitLikeProjectSource{
48-
Remotes: remotes,
49-
CheckoutFrom: &v1alpha2.CheckoutFrom{
50-
Remote: checkoutRemote,
51-
},
44+
Remotes: remotes,
45+
CheckoutFrom: checkoutRemote,
5246
},
5347
},
5448
},
5549
}
5650
}
5751

58-
func generateDummyGithubProject(name, checkoutRemote string, remotes map[string]string) v1alpha2.Project {
52+
func generateDummyGithubProject(name string, checkoutRemote *v1alpha2.CheckoutFrom, remotes map[string]string) v1alpha2.Project {
5953
return v1alpha2.Project{
6054
Name: name,
6155
ProjectSource: v1alpha2.ProjectSource{
6256
Github: &v1alpha2.GithubProjectSource{
6357
GitLikeProjectSource: v1alpha2.GitLikeProjectSource{
64-
Remotes: remotes,
65-
CheckoutFrom: &v1alpha2.CheckoutFrom{
66-
Remote: checkoutRemote,
67-
},
58+
Remotes: remotes,
59+
CheckoutFrom: checkoutRemote,
6860
},
6961
},
7062
},
@@ -85,35 +77,41 @@ func TestValidateStarterProjects(t *testing.T) {
8577
{
8678
name: "Valid Starter Project",
8779
starterProjects: []v1alpha2.StarterProject{
88-
generateDummyGitStarterProject("project1", "origin", map[string]string{"origin": "originremote"}),
89-
generateDummyGitStarterProject("project2", "origin", map[string]string{"origin": "originremote2"}),
80+
generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}),
81+
generateDummyGitStarterProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote2"}),
9082
},
9183
},
9284
{
9385
name: "Invalid Starter Project",
9486
starterProjects: []v1alpha2.StarterProject{
95-
generateDummyGithubStarterProject("project1", "origin", map[string]string{"origin": "originremote", "test": "testremote"}),
87+
generateDummyGithubStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote", "test": "testremote"}),
9688
},
9789
wantErr: &oneRemoteErr,
9890
},
9991
{
10092
name: "Invalid Starter Project with wrong checkout",
10193
starterProjects: []v1alpha2.StarterProject{
102-
generateDummyGithubStarterProject("project1", "origin", map[string]string{"test": "testremote"}),
94+
generateDummyGithubStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"test": "testremote"}),
10395
},
10496
wantErr: &wrongCheckoutErr,
10597
},
10698
{
10799
name: "Valid Starter Project with empty checkout remote",
108100
starterProjects: []v1alpha2.StarterProject{
109-
generateDummyGitStarterProject("project1", "", map[string]string{"origin": "originremote"}),
101+
generateDummyGitStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote"}),
102+
},
103+
},
104+
{
105+
name: "Valid Starter Project with no checkout remote",
106+
starterProjects: []v1alpha2.StarterProject{
107+
generateDummyGitStarterProject("project1", nil, map[string]string{"origin": "originremote"}),
110108
},
111109
},
112110
{
113111
name: "Invalid Starter Project with empty remotes",
114112
starterProjects: []v1alpha2.StarterProject{
115-
generateDummyGithubStarterProject("project1", "origin", map[string]string{}),
116-
generateDummyGithubStarterProject("project3", "origin", map[string]string{"origin": "originremote", "test": "testremote"}),
113+
generateDummyGithubStarterProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{}),
114+
generateDummyGithubStarterProject("project3", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote", "test": "testremote"}),
117115
},
118116
wantErr: &atleastOneRemoteErr,
119117
},
@@ -135,6 +133,7 @@ func TestValidateProjects(t *testing.T) {
135133

136134
wrongCheckoutErr := "unable to find the checkout remote .* in the remotes for project.*"
137135
atleastOneRemoteErr := "projects .* should have at least one remote"
136+
missingCheckOutFromRemoteErr := "project .* has more than one remote defined, but has no checkoutfrom remote defined"
138137

139138
tests := []struct {
140139
name string
@@ -144,37 +143,44 @@ func TestValidateProjects(t *testing.T) {
144143
{
145144
name: "Valid Project",
146145
projects: []v1alpha2.Project{
147-
generateDummyGitProject("project1", "origin", map[string]string{"origin": "originremote"}),
148-
generateDummyGithubProject("project2", "origin", map[string]string{"origin": "originremote"}),
146+
generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}),
147+
generateDummyGithubProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}),
149148
},
150149
},
150+
{
151+
name: "Invalid Project with multiple remotes but no checkoutfrom",
152+
projects: []v1alpha2.Project{
153+
generateDummyGithubProject("project2", nil, map[string]string{"origin": "originremote", "test": "testremote"}),
154+
},
155+
wantErr: &missingCheckOutFromRemoteErr,
156+
},
151157
{
152158
name: "Invalid Project with multiple remote and empty checkout remote",
153159
projects: []v1alpha2.Project{
154-
generateDummyGitProject("project2", "origin", map[string]string{"origin": "originremote"}),
155-
generateDummyGithubProject("project1", "", map[string]string{"origin": "originremote", "test": "testremote"}),
160+
generateDummyGitProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{"origin": "originremote"}),
161+
generateDummyGithubProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote", "test": "testremote"}),
156162
},
157-
wantErr: &wrongCheckoutErr,
163+
wantErr: &missingCheckOutFromRemoteErr,
158164
},
159165
{
160166
name: "Invalid Project with wrong checkout",
161167
projects: []v1alpha2.Project{
162-
generateDummyGithubProject("project1", "origin", map[string]string{"origin": "originremote", "test": "testremote"}),
163-
generateDummyGitProject("project2", "origin1", map[string]string{"origin2": "originremote2"}),
168+
generateDummyGithubProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin1"}, map[string]string{"origin": "originremote", "test": "testremote"}),
169+
generateDummyGitProject("project2", &v1alpha2.CheckoutFrom{Remote: "origin1"}, map[string]string{"origin2": "originremote2"}),
164170
},
165171
wantErr: &wrongCheckoutErr,
166172
},
167173
{
168174
name: "Valid Project with empty checkout remote",
169175
projects: []v1alpha2.Project{
170-
generateDummyGitProject("project1", "", map[string]string{"origin": "originremote"}),
176+
generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: ""}, map[string]string{"origin": "originremote"}),
171177
},
172178
},
173179
{
174180
name: "Invalid Project with empty remotes",
175181
projects: []v1alpha2.Project{
176-
generateDummyGitProject("project1", "origin", map[string]string{}),
177-
generateDummyGithubProject("project2", "origins", map[string]string{"origin": "originremote", "test": "testremote"}),
182+
generateDummyGitProject("project1", &v1alpha2.CheckoutFrom{Remote: "origin"}, map[string]string{}),
183+
generateDummyGithubProject("project2", &v1alpha2.CheckoutFrom{Remote: "origins"}, map[string]string{"origin": "originremote", "test": "testremote"}),
178184
},
179185
wantErr: &atleastOneRemoteErr,
180186
},

0 commit comments

Comments
 (0)