Browse code

Improve consistency of build trigger types

* Add validation against unknown types
* Remove redundant validation
* Fix capitalization of BuildTriggerTypes
* GithubWebHook -> GitHubWebHook
* Ensure v1beta3 and v1 accepts both capitalizations (imageChange and ImageChange,
github and GitHub, generic and Generic)

Rodolfo Carvalho authored on 2015/06/03 00:59:31
Showing 23 changed files
... ...
@@ -485,7 +485,7 @@ oc get buildConfigs
485 485
 oc get bc
486 486
 oc get builds
487 487
 
488
-[[ $(oc describe buildConfigs ruby-sample-build --api-version=v1beta3 | grep --text "Webhook Github"  | grep -F "${API_SCHEME}://${API_HOST}:${API_PORT}/osapi/v1beta3/namespaces/default/buildconfigs/ruby-sample-build/webhooks/secret101/github") ]]
488
+[[ $(oc describe buildConfigs ruby-sample-build --api-version=v1beta3 | grep --text "Webhook GitHub"  | grep -F "${API_SCHEME}://${API_HOST}:${API_PORT}/osapi/v1beta3/namespaces/default/buildconfigs/ruby-sample-build/webhooks/secret101/github") ]]
489 489
 [[ $(oc describe buildConfigs ruby-sample-build --api-version=v1beta3 | grep --text "Webhook Generic" | grep -F "${API_SCHEME}://${API_HOST}:${API_PORT}/osapi/v1beta3/namespaces/default/buildconfigs/ruby-sample-build/webhooks/secret101/generic") ]]
490 490
 oc start-build --list-webhooks='all' ruby-sample-build
491 491
 [[ $(oc start-build --list-webhooks='all' ruby-sample-build | grep --text "generic") ]]
... ...
@@ -922,13 +922,13 @@ func deepCopy_api_BuildStrategy(in buildapi.BuildStrategy, out *buildapi.BuildSt
922 922
 
923 923
 func deepCopy_api_BuildTriggerPolicy(in buildapi.BuildTriggerPolicy, out *buildapi.BuildTriggerPolicy, c *conversion.Cloner) error {
924 924
 	out.Type = in.Type
925
-	if in.GithubWebHook != nil {
926
-		out.GithubWebHook = new(buildapi.WebHookTrigger)
927
-		if err := deepCopy_api_WebHookTrigger(*in.GithubWebHook, out.GithubWebHook, c); err != nil {
925
+	if in.GitHubWebHook != nil {
926
+		out.GitHubWebHook = new(buildapi.WebHookTrigger)
927
+		if err := deepCopy_api_WebHookTrigger(*in.GitHubWebHook, out.GitHubWebHook, c); err != nil {
928 928
 			return err
929 929
 		}
930 930
 	} else {
931
-		out.GithubWebHook = nil
931
+		out.GitHubWebHook = nil
932 932
 	}
933 933
 	if in.GenericWebHook != nil {
934 934
 		out.GenericWebHook = new(buildapi.WebHookTrigger)
... ...
@@ -922,13 +922,13 @@ func deepCopy_v1_BuildStrategy(in buildapiv1.BuildStrategy, out *buildapiv1.Buil
922 922
 
923 923
 func deepCopy_v1_BuildTriggerPolicy(in buildapiv1.BuildTriggerPolicy, out *buildapiv1.BuildTriggerPolicy, c *conversion.Cloner) error {
924 924
 	out.Type = in.Type
925
-	if in.GithubWebHook != nil {
926
-		out.GithubWebHook = new(buildapiv1.WebHookTrigger)
927
-		if err := deepCopy_v1_WebHookTrigger(*in.GithubWebHook, out.GithubWebHook, c); err != nil {
925
+	if in.GitHubWebHook != nil {
926
+		out.GitHubWebHook = new(buildapiv1.WebHookTrigger)
927
+		if err := deepCopy_v1_WebHookTrigger(*in.GitHubWebHook, out.GitHubWebHook, c); err != nil {
928 928
 			return err
929 929
 		}
930 930
 	} else {
931
-		out.GithubWebHook = nil
931
+		out.GitHubWebHook = nil
932 932
 	}
933 933
 	if in.GenericWebHook != nil {
934 934
 		out.GenericWebHook = new(buildapiv1.WebHookTrigger)
... ...
@@ -930,13 +930,13 @@ func deepCopy_v1beta3_BuildStrategy(in buildapiv1beta3.BuildStrategy, out *build
930 930
 
931 931
 func deepCopy_v1beta3_BuildTriggerPolicy(in buildapiv1beta3.BuildTriggerPolicy, out *buildapiv1beta3.BuildTriggerPolicy, c *conversion.Cloner) error {
932 932
 	out.Type = in.Type
933
-	if in.GithubWebHook != nil {
934
-		out.GithubWebHook = new(buildapiv1beta3.WebHookTrigger)
935
-		if err := deepCopy_v1beta3_WebHookTrigger(*in.GithubWebHook, out.GithubWebHook, c); err != nil {
933
+	if in.GitHubWebHook != nil {
934
+		out.GitHubWebHook = new(buildapiv1beta3.WebHookTrigger)
935
+		if err := deepCopy_v1beta3_WebHookTrigger(*in.GitHubWebHook, out.GitHubWebHook, c); err != nil {
936 936
 			return err
937 937
 		}
938 938
 	} else {
939
-		out.GithubWebHook = nil
939
+		out.GitHubWebHook = nil
940 940
 	}
941 941
 	if in.GenericWebHook != nil {
942 942
 		out.GenericWebHook = new(buildapiv1beta3.WebHookTrigger)
... ...
@@ -326,8 +326,8 @@ type BuildTriggerPolicy struct {
326 326
 	// Type is the type of build trigger
327 327
 	Type BuildTriggerType `json:"type,omitempty"`
328 328
 
329
-	// GithubWebHook contains the parameters for a GitHub webhook type of trigger
330
-	GithubWebHook *WebHookTrigger `json:"github,omitempty"`
329
+	// GitHubWebHook contains the parameters for a GitHub webhook type of trigger
330
+	GitHubWebHook *WebHookTrigger `json:"github,omitempty"`
331 331
 
332 332
 	// GenericWebHook contains the parameters for a Generic webhook type of trigger
333 333
 	GenericWebHook *WebHookTrigger `json:"generic,omitempty"`
... ...
@@ -340,17 +340,17 @@ type BuildTriggerPolicy struct {
340 340
 type BuildTriggerType string
341 341
 
342 342
 const (
343
-	// GithubWebHookBuildTriggerType represents a trigger that launches builds on
343
+	// GitHubWebHookBuildTriggerType represents a trigger that launches builds on
344 344
 	// GitHub webhook invocations
345
-	GithubWebHookBuildTriggerType BuildTriggerType = "github"
345
+	GitHubWebHookBuildTriggerType BuildTriggerType = "GitHub"
346 346
 
347 347
 	// GenericWebHookBuildTriggerType represents a trigger that launches builds on
348 348
 	// generic webhook invocations
349
-	GenericWebHookBuildTriggerType BuildTriggerType = "generic"
349
+	GenericWebHookBuildTriggerType BuildTriggerType = "Generic"
350 350
 
351 351
 	// ImageChangeBuildTriggerType represents a trigger that launches builds on
352 352
 	// availability of a new version of an image
353
-	ImageChangeBuildTriggerType BuildTriggerType = "imageChange"
353
+	ImageChangeBuildTriggerType BuildTriggerType = "ImageChange"
354 354
 )
355 355
 
356 356
 // BuildList is a collection of Builds.
... ...
@@ -338,6 +338,21 @@ func convert_v1_ImageChangeTrigger_To_api_ImageChangeTrigger(in *ImageChangeTrig
338 338
 	return nil
339 339
 }
340 340
 
341
+func convert_v1_BuildTriggerPolicy_To_api_BuildTriggerPolicy(in *BuildTriggerPolicy, out *newer.BuildTriggerPolicy, s conversion.Scope) error {
342
+	if err := s.DefaultConvert(in, out, conversion.DestFromSource); err != nil {
343
+		return err
344
+	}
345
+	switch in.Type {
346
+	case ImageChangeBuildTriggerTypeDeprecated:
347
+		out.Type = newer.ImageChangeBuildTriggerType
348
+	case GenericWebHookBuildTriggerTypeDeprecated:
349
+		out.Type = newer.GenericWebHookBuildTriggerType
350
+	case GitHubWebHookBuildTriggerTypeDeprecated:
351
+		out.Type = newer.GitHubWebHookBuildTriggerType
352
+	}
353
+	return nil
354
+}
355
+
341 356
 func init() {
342 357
 	err := kapi.Scheme.AddDefaultingFuncs(
343 358
 		func(obj *SourceBuildStrategy) {
... ...
@@ -388,6 +403,7 @@ func init() {
388 388
 		convert_v1_BuildOutput_To_api_BuildOutput,
389 389
 		convert_api_ImageChangeTrigger_To_v1_ImageChangeTrigger,
390 390
 		convert_v1_ImageChangeTrigger_To_api_ImageChangeTrigger,
391
+		convert_v1_BuildTriggerPolicy_To_api_BuildTriggerPolicy,
391 392
 	)
392 393
 
393 394
 	// Add field conversion funcs.
... ...
@@ -4,7 +4,6 @@ import (
4 4
 	"testing"
5 5
 
6 6
 	knewer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
7
-	/*kolder "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1"*/
8 7
 
9 8
 	newer "github.com/openshift/origin/pkg/build/api"
10 9
 	older "github.com/openshift/origin/pkg/build/api/v1"
... ...
@@ -26,3 +25,48 @@ func TestImageChangeTriggerDefaultValueConversion(t *testing.T) {
26 26
 		t.Errorf("expected %v, actual %v", &newer.ImageChangeTrigger{}, nil)
27 27
 	}
28 28
 }
29
+
30
+func TestBuildTriggerPolicyOldToNewConversion(t *testing.T) {
31
+	testCases := map[string]struct {
32
+		Olds                     []older.BuildTriggerType
33
+		ExpectedBuildTriggerType newer.BuildTriggerType
34
+	}{
35
+		"ImageChange": {
36
+			Olds: []older.BuildTriggerType{
37
+				older.ImageChangeBuildTriggerType,
38
+				older.ImageChangeBuildTriggerTypeDeprecated,
39
+			},
40
+			ExpectedBuildTriggerType: newer.ImageChangeBuildTriggerType,
41
+		},
42
+		"Generic": {
43
+			Olds: []older.BuildTriggerType{
44
+				older.GenericWebHookBuildTriggerType,
45
+				older.GenericWebHookBuildTriggerTypeDeprecated,
46
+			},
47
+			ExpectedBuildTriggerType: newer.GenericWebHookBuildTriggerType,
48
+		},
49
+		"GitHub": {
50
+			Olds: []older.BuildTriggerType{
51
+				older.GitHubWebHookBuildTriggerType,
52
+				older.GitHubWebHookBuildTriggerTypeDeprecated,
53
+			},
54
+			ExpectedBuildTriggerType: newer.GitHubWebHookBuildTriggerType,
55
+		},
56
+	}
57
+	for s, testCase := range testCases {
58
+		expected := testCase.ExpectedBuildTriggerType
59
+		for _, old := range testCase.Olds {
60
+			var actual newer.BuildTriggerPolicy
61
+			oldVersion := older.BuildTriggerPolicy{
62
+				Type: old,
63
+			}
64
+			err := Convert(&oldVersion, &actual)
65
+			if err != nil {
66
+				t.Fatalf("%s (%s -> %s): unexpected error: %v", s, old, expected, err)
67
+			}
68
+			if actual.Type != testCase.ExpectedBuildTriggerType {
69
+				t.Errorf("%s (%s -> %s): expected %v, actual %v", s, old, expected, expected, actual.Type)
70
+			}
71
+		}
72
+	}
73
+}
... ...
@@ -321,8 +321,8 @@ type BuildTriggerPolicy struct {
321 321
 	// Type is the type of build trigger
322 322
 	Type BuildTriggerType `json:"type,omitempty" description:"type of build trigger"`
323 323
 
324
-	// GithubWebHook contains the parameters for a GitHub webhook type of trigger
325
-	GithubWebHook *WebHookTrigger `json:"github,omitempty" description:"parameters for a GitHub webhook type of trigger"`
324
+	// GitHubWebHook contains the parameters for a GitHub webhook type of trigger
325
+	GitHubWebHook *WebHookTrigger `json:"github,omitempty" description:"parameters for a GitHub webhook type of trigger"`
326 326
 
327 327
 	// GenericWebHook contains the parameters for a Generic webhook type of trigger
328 328
 	GenericWebHook *WebHookTrigger `json:"generic,omitempty" description:"parameters for a Generic webhook type of trigger"`
... ...
@@ -335,17 +335,20 @@ type BuildTriggerPolicy struct {
335 335
 type BuildTriggerType string
336 336
 
337 337
 const (
338
-	// GithubWebHookBuildTriggerType represents a trigger that launches builds on
338
+	// GitHubWebHookBuildTriggerType represents a trigger that launches builds on
339 339
 	// GitHub webhook invocations
340
-	GithubWebHookBuildTriggerType BuildTriggerType = "github"
340
+	GitHubWebHookBuildTriggerType           BuildTriggerType = "GitHub"
341
+	GitHubWebHookBuildTriggerTypeDeprecated BuildTriggerType = "github"
341 342
 
342 343
 	// GenericWebHookBuildTriggerType represents a trigger that launches builds on
343 344
 	// generic webhook invocations
344
-	GenericWebHookBuildTriggerType BuildTriggerType = "generic"
345
+	GenericWebHookBuildTriggerType           BuildTriggerType = "Generic"
346
+	GenericWebHookBuildTriggerTypeDeprecated BuildTriggerType = "generic"
345 347
 
346 348
 	// ImageChangeBuildTriggerType represents a trigger that launches builds on
347 349
 	// availability of a new version of an image
348
-	ImageChangeBuildTriggerType BuildTriggerType = "imageChange"
350
+	ImageChangeBuildTriggerType           BuildTriggerType = "ImageChange"
351
+	ImageChangeBuildTriggerTypeDeprecated BuildTriggerType = "imageChange"
349 352
 )
350 353
 
351 354
 // BuildList is a collection of Builds.
... ...
@@ -338,6 +338,36 @@ func convert_v1beta3_ImageChangeTrigger_To_api_ImageChangeTrigger(in *ImageChang
338 338
 	return nil
339 339
 }
340 340
 
341
+func convert_api_BuildTriggerPolicy_To_v1beta3_BuildTriggerPolicy(in *newer.BuildTriggerPolicy, out *BuildTriggerPolicy, s conversion.Scope) error {
342
+	if err := s.DefaultConvert(in, out, conversion.DestFromSource); err != nil {
343
+		return err
344
+	}
345
+	switch in.Type {
346
+	case newer.ImageChangeBuildTriggerType:
347
+		out.Type = ImageChangeBuildTriggerType
348
+	case newer.GenericWebHookBuildTriggerType:
349
+		out.Type = GenericWebHookBuildTriggerType
350
+	case newer.GitHubWebHookBuildTriggerType:
351
+		out.Type = GitHubWebHookBuildTriggerType
352
+	}
353
+	return nil
354
+}
355
+
356
+func convert_v1beta3_BuildTriggerPolicy_To_api_BuildTriggerPolicy(in *BuildTriggerPolicy, out *newer.BuildTriggerPolicy, s conversion.Scope) error {
357
+	if err := s.DefaultConvert(in, out, conversion.DestFromSource); err != nil {
358
+		return err
359
+	}
360
+	switch in.Type {
361
+	case ImageChangeBuildTriggerType:
362
+		out.Type = newer.ImageChangeBuildTriggerType
363
+	case GenericWebHookBuildTriggerType:
364
+		out.Type = newer.GenericWebHookBuildTriggerType
365
+	case GitHubWebHookBuildTriggerType:
366
+		out.Type = newer.GitHubWebHookBuildTriggerType
367
+	}
368
+	return nil
369
+}
370
+
341 371
 func init() {
342 372
 	err := kapi.Scheme.AddDefaultingFuncs(
343 373
 		func(obj *SourceBuildStrategy) {
... ...
@@ -383,6 +413,8 @@ func init() {
383 383
 		convert_v1beta3_BuildOutput_To_api_BuildOutput,
384 384
 		convert_api_ImageChangeTrigger_To_v1beta3_ImageChangeTrigger,
385 385
 		convert_v1beta3_ImageChangeTrigger_To_api_ImageChangeTrigger,
386
+		convert_api_BuildTriggerPolicy_To_v1beta3_BuildTriggerPolicy,
387
+		convert_v1beta3_BuildTriggerPolicy_To_api_BuildTriggerPolicy,
386 388
 	)
387 389
 
388 390
 	// Add field conversion funcs.
... ...
@@ -1,11 +1,90 @@
1 1
 package v1beta3_test
2 2
 
3 3
 import (
4
-/*	"testing"
4
+	"testing"
5 5
 
6 6
 	knewer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
7
-	kolder "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3"
8 7
 
9 8
 	newer "github.com/openshift/origin/pkg/build/api"
10
-	older "github.com/openshift/origin/pkg/build/api/v1beta3"*/
9
+	older "github.com/openshift/origin/pkg/build/api/v1beta3"
11 10
 )
11
+
12
+var Convert = knewer.Scheme.Convert
13
+
14
+func TestBuildTriggerPolicyOldToNewConversion(t *testing.T) {
15
+	testCases := map[string]struct {
16
+		Olds                     []older.BuildTriggerType
17
+		ExpectedBuildTriggerType newer.BuildTriggerType
18
+	}{
19
+		"ImageChange": {
20
+			Olds: []older.BuildTriggerType{
21
+				older.ImageChangeBuildTriggerType,
22
+				older.BuildTriggerType(newer.ImageChangeBuildTriggerType),
23
+			},
24
+			ExpectedBuildTriggerType: newer.ImageChangeBuildTriggerType,
25
+		},
26
+		"Generic": {
27
+			Olds: []older.BuildTriggerType{
28
+				older.GenericWebHookBuildTriggerType,
29
+				older.BuildTriggerType(newer.GenericWebHookBuildTriggerType),
30
+			},
31
+			ExpectedBuildTriggerType: newer.GenericWebHookBuildTriggerType,
32
+		},
33
+		"GitHub": {
34
+			Olds: []older.BuildTriggerType{
35
+				older.GitHubWebHookBuildTriggerType,
36
+				older.BuildTriggerType(newer.GitHubWebHookBuildTriggerType),
37
+			},
38
+			ExpectedBuildTriggerType: newer.GitHubWebHookBuildTriggerType,
39
+		},
40
+	}
41
+	for s, testCase := range testCases {
42
+		expected := testCase.ExpectedBuildTriggerType
43
+		for _, old := range testCase.Olds {
44
+			var actual newer.BuildTriggerPolicy
45
+			oldVersion := older.BuildTriggerPolicy{
46
+				Type: old,
47
+			}
48
+			err := Convert(&oldVersion, &actual)
49
+			if err != nil {
50
+				t.Fatalf("%s (%s -> %s): unexpected error: %v", s, old, expected, err)
51
+			}
52
+			if actual.Type != testCase.ExpectedBuildTriggerType {
53
+				t.Errorf("%s (%s -> %s): expected %v, actual %v", s, old, expected, expected, actual.Type)
54
+			}
55
+		}
56
+	}
57
+}
58
+
59
+func TestBuildTriggerPolicyNewToOldConversion(t *testing.T) {
60
+	testCases := map[string]struct {
61
+		New                      newer.BuildTriggerType
62
+		ExpectedBuildTriggerType older.BuildTriggerType
63
+	}{
64
+		"ImageChange": {
65
+			New: newer.ImageChangeBuildTriggerType,
66
+			ExpectedBuildTriggerType: older.ImageChangeBuildTriggerType,
67
+		},
68
+		"Generic": {
69
+			New: newer.GenericWebHookBuildTriggerType,
70
+			ExpectedBuildTriggerType: older.GenericWebHookBuildTriggerType,
71
+		},
72
+		"GitHub": {
73
+			New: newer.GitHubWebHookBuildTriggerType,
74
+			ExpectedBuildTriggerType: older.GitHubWebHookBuildTriggerType,
75
+		},
76
+	}
77
+	for s, testCase := range testCases {
78
+		var actual older.BuildTriggerPolicy
79
+		newVersion := newer.BuildTriggerPolicy{
80
+			Type: testCase.New,
81
+		}
82
+		err := Convert(&newVersion, &actual)
83
+		if err != nil {
84
+			t.Fatalf("%s: unexpected error: %v", s, err)
85
+		}
86
+		if actual.Type != testCase.ExpectedBuildTriggerType {
87
+			t.Errorf("%s: expected %v, actual %v", s, testCase.ExpectedBuildTriggerType, actual.Type)
88
+		}
89
+	}
90
+}
... ...
@@ -321,8 +321,8 @@ type BuildTriggerPolicy struct {
321 321
 	// Type is the type of build trigger
322 322
 	Type BuildTriggerType `json:"type,omitempty"`
323 323
 
324
-	// GithubWebHook contains the parameters for a GitHub webhook type of trigger
325
-	GithubWebHook *WebHookTrigger `json:"github,omitempty"`
324
+	// GitHubWebHook contains the parameters for a GitHub webhook type of trigger
325
+	GitHubWebHook *WebHookTrigger `json:"github,omitempty"`
326 326
 
327 327
 	// GenericWebHook contains the parameters for a Generic webhook type of trigger
328 328
 	GenericWebHook *WebHookTrigger `json:"generic,omitempty"`
... ...
@@ -335,9 +335,9 @@ type BuildTriggerPolicy struct {
335 335
 type BuildTriggerType string
336 336
 
337 337
 const (
338
-	// GithubWebHookBuildTriggerType represents a trigger that launches builds on
338
+	// GitHubWebHookBuildTriggerType represents a trigger that launches builds on
339 339
 	// GitHub webhook invocations
340
-	GithubWebHookBuildTriggerType BuildTriggerType = "github"
340
+	GitHubWebHookBuildTriggerType BuildTriggerType = "github"
341 341
 
342 342
 	// GenericWebHookBuildTriggerType represents a trigger that launches builds on
343 343
 	// generic webhook invocations
... ...
@@ -235,20 +235,13 @@ func validateTrigger(trigger *buildapi.BuildTriggerPolicy) fielderrors.Validatio
235 235
 		return allErrs
236 236
 	}
237 237
 
238
-	// Ensure that only parameters for the trigger's type are present
239
-	triggerPresence := map[buildapi.BuildTriggerType]bool{
240
-		buildapi.GithubWebHookBuildTriggerType:  trigger.GithubWebHook != nil,
241
-		buildapi.GenericWebHookBuildTriggerType: trigger.GenericWebHook != nil,
242
-	}
243
-	allErrs = append(allErrs, validateTriggerPresence(triggerPresence, trigger.Type)...)
244
-
245 238
 	// Validate each trigger type
246 239
 	switch trigger.Type {
247
-	case buildapi.GithubWebHookBuildTriggerType:
248
-		if trigger.GithubWebHook == nil {
240
+	case buildapi.GitHubWebHookBuildTriggerType:
241
+		if trigger.GitHubWebHook == nil {
249 242
 			allErrs = append(allErrs, fielderrors.NewFieldRequired("github"))
250 243
 		} else {
251
-			allErrs = append(allErrs, validateWebHook(trigger.GithubWebHook).Prefix("github")...)
244
+			allErrs = append(allErrs, validateWebHook(trigger.GitHubWebHook).Prefix("github")...)
252 245
 		}
253 246
 	case buildapi.GenericWebHookBuildTriggerType:
254 247
 		if trigger.GenericWebHook == nil {
... ...
@@ -260,16 +253,8 @@ func validateTrigger(trigger *buildapi.BuildTriggerPolicy) fielderrors.Validatio
260 260
 		if trigger.ImageChange == nil {
261 261
 			allErrs = append(allErrs, fielderrors.NewFieldRequired("imageChange"))
262 262
 		}
263
-	}
264
-	return allErrs
265
-}
266
-
267
-func validateTriggerPresence(params map[buildapi.BuildTriggerType]bool, t buildapi.BuildTriggerType) fielderrors.ValidationErrorList {
268
-	allErrs := fielderrors.ValidationErrorList{}
269
-	for triggerType, present := range params {
270
-		if triggerType != t && present {
271
-			allErrs = append(allErrs, fielderrors.NewFieldInvalid(string(triggerType), "", "triggerType wasn't found"))
272
-		}
263
+	default:
264
+		allErrs = append(allErrs, fielderrors.NewFieldInvalid("type", trigger.Type, "invalid trigger type"))
273 265
 	}
274 266
 	return allErrs
275 267
 }
... ...
@@ -544,61 +544,67 @@ func TestValidateTrigger(t *testing.T) {
544 544
 			trigger:  buildapi.BuildTriggerPolicy{},
545 545
 			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("type")},
546 546
 		},
547
-		"github type with no github webhook": {
548
-			trigger:  buildapi.BuildTriggerPolicy{Type: buildapi.GithubWebHookBuildTriggerType},
547
+		"trigger with unknown type": {
548
+			trigger: buildapi.BuildTriggerPolicy{
549
+				Type: "UnknownTriggerType",
550
+			},
551
+			expected: []*fielderrors.ValidationError{fielderrors.NewFieldInvalid("type", "", "")},
552
+		},
553
+		"GitHub type with no github webhook": {
554
+			trigger:  buildapi.BuildTriggerPolicy{Type: buildapi.GitHubWebHookBuildTriggerType},
549 555
 			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("github")},
550 556
 		},
551
-		"github trigger with no secret": {
557
+		"GitHub trigger with no secret": {
552 558
 			trigger: buildapi.BuildTriggerPolicy{
553
-				Type:          buildapi.GithubWebHookBuildTriggerType,
554
-				GithubWebHook: &buildapi.WebHookTrigger{},
559
+				Type:          buildapi.GitHubWebHookBuildTriggerType,
560
+				GitHubWebHook: &buildapi.WebHookTrigger{},
555 561
 			},
556 562
 			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("github.secret")},
557 563
 		},
558
-		"github trigger with generic webhook": {
564
+		"GitHub trigger with generic webhook": {
559 565
 			trigger: buildapi.BuildTriggerPolicy{
560
-				Type: buildapi.GithubWebHookBuildTriggerType,
566
+				Type: buildapi.GitHubWebHookBuildTriggerType,
561 567
 				GenericWebHook: &buildapi.WebHookTrigger{
562 568
 					Secret: "secret101",
563 569
 				},
564 570
 			},
565
-			expected: []*fielderrors.ValidationError{fielderrors.NewFieldInvalid("generic", "", "long description")},
571
+			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("github")},
566 572
 		},
567
-		"generic trigger with no generic webhook": {
573
+		"Generic trigger with no generic webhook": {
568 574
 			trigger:  buildapi.BuildTriggerPolicy{Type: buildapi.GenericWebHookBuildTriggerType},
569 575
 			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("generic")},
570 576
 		},
571
-		"generic trigger with no secret": {
577
+		"Generic trigger with no secret": {
572 578
 			trigger: buildapi.BuildTriggerPolicy{
573 579
 				Type:           buildapi.GenericWebHookBuildTriggerType,
574 580
 				GenericWebHook: &buildapi.WebHookTrigger{},
575 581
 			},
576 582
 			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("generic.secret")},
577 583
 		},
578
-		"generic trigger with github webhook": {
584
+		"Generic trigger with github webhook": {
579 585
 			trigger: buildapi.BuildTriggerPolicy{
580 586
 				Type: buildapi.GenericWebHookBuildTriggerType,
581
-				GithubWebHook: &buildapi.WebHookTrigger{
587
+				GitHubWebHook: &buildapi.WebHookTrigger{
582 588
 					Secret: "secret101",
583 589
 				},
584 590
 			},
585
-			expected: []*fielderrors.ValidationError{fielderrors.NewFieldInvalid("github", "", "long github description")},
591
+			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("generic")},
586 592
 		},
587
-		"imageChange trigger without params": {
593
+		"ImageChange trigger without params": {
588 594
 			trigger: buildapi.BuildTriggerPolicy{
589 595
 				Type: buildapi.ImageChangeBuildTriggerType,
590 596
 			},
591 597
 			expected: []*fielderrors.ValidationError{fielderrors.NewFieldRequired("imageChange")},
592 598
 		},
593
-		"valid github trigger": {
599
+		"valid GitHub trigger": {
594 600
 			trigger: buildapi.BuildTriggerPolicy{
595
-				Type: buildapi.GithubWebHookBuildTriggerType,
596
-				GithubWebHook: &buildapi.WebHookTrigger{
601
+				Type: buildapi.GitHubWebHookBuildTriggerType,
602
+				GitHubWebHook: &buildapi.WebHookTrigger{
597 603
 					Secret: "secret101",
598 604
 				},
599 605
 			},
600 606
 		},
601
-		"valid generic trigger": {
607
+		"valid Generic trigger": {
602 608
 			trigger: buildapi.BuildTriggerPolicy{
603 609
 				Type: buildapi.GenericWebHookBuildTriggerType,
604 610
 				GenericWebHook: &buildapi.WebHookTrigger{
... ...
@@ -606,7 +612,7 @@ func TestValidateTrigger(t *testing.T) {
606 606
 				},
607 607
 			},
608 608
 		},
609
-		"valid imageChange trigger": {
609
+		"valid ImageChange trigger": {
610 610
 			trigger: buildapi.BuildTriggerPolicy{
611 611
 				Type: buildapi.ImageChangeBuildTriggerType,
612 612
 				ImageChange: &buildapi.ImageChangeTrigger{
... ...
@@ -614,7 +620,7 @@ func TestValidateTrigger(t *testing.T) {
614 614
 				},
615 615
 			},
616 616
 		},
617
-		"valid imageChange trigger with empty fields": {
617
+		"valid ImageChange trigger with empty fields": {
618 618
 			trigger: buildapi.BuildTriggerPolicy{
619 619
 				Type:        buildapi.ImageChangeBuildTriggerType,
620 620
 				ImageChange: &buildapi.ImageChangeTrigger{},
... ...
@@ -629,6 +635,14 @@ func TestValidateTrigger(t *testing.T) {
629 629
 			}
630 630
 			continue
631 631
 		}
632
+		if len(errors) != 1 {
633
+			t.Errorf("%s: Expected one validation error, got %d", desc, len(errors))
634
+			for i, err := range errors {
635
+				validationError := err.(*fielderrors.ValidationError)
636
+				t.Errorf("  %d. %v", i+1, validationError)
637
+			}
638
+			continue
639
+		}
632 640
 		err := errors[0]
633 641
 		validationError := err.(*fielderrors.ValidationError)
634 642
 		if validationError.Type != test.expected[0].Type {
... ...
@@ -30,8 +30,8 @@ func (*okBuildConfigGetter) Get(namespace, name string) (*api.BuildConfig, error
30 30
 		},
31 31
 		Triggers: []api.BuildTriggerPolicy{
32 32
 			{
33
-				Type: api.GithubWebHookBuildTriggerType,
34
-				GithubWebHook: &api.WebHookTrigger{
33
+				Type: api.GitHubWebHookBuildTriggerType,
34
+				GitHubWebHook: &api.WebHookTrigger{
35 35
 					Secret: "secret101",
36 36
 				},
37 37
 			},
... ...
@@ -35,13 +35,13 @@ type pushEvent struct {
35 35
 
36 36
 // Extract services webhooks from github.com
37 37
 func (p *WebHook) Extract(buildCfg *api.BuildConfig, secret, path string, req *http.Request) (revision *api.SourceRevision, proceed bool, err error) {
38
-	trigger, ok := webhook.FindTriggerPolicy(api.GithubWebHookBuildTriggerType, buildCfg)
38
+	trigger, ok := webhook.FindTriggerPolicy(api.GitHubWebHookBuildTriggerType, buildCfg)
39 39
 	if !ok {
40 40
 		err = webhook.ErrHookNotEnabled
41 41
 		return
42 42
 	}
43 43
 	glog.V(4).Infof("Checking if the provided secret for BuildConfig %s/%s matches", buildCfg.Namespace, buildCfg.Name)
44
-	if trigger.GithubWebHook.Secret != secret {
44
+	if trigger.GitHubWebHook.Secret != secret {
45 45
 		err = webhook.ErrSecretMismatch
46 46
 		return
47 47
 	}
... ...
@@ -20,8 +20,8 @@ func (c *okBuildConfigGetter) Get(namespace, name string) (*api.BuildConfig, err
20 20
 	return &api.BuildConfig{
21 21
 		Triggers: []api.BuildTriggerPolicy{
22 22
 			{
23
-				Type: api.GithubWebHookBuildTriggerType,
24
-				GithubWebHook: &api.WebHookTrigger{
23
+				Type: api.GitHubWebHookBuildTriggerType,
24
+				GitHubWebHook: &api.WebHookTrigger{
25 25
 					Secret: "secret101",
26 26
 				},
27 27
 			},
... ...
@@ -210,8 +210,8 @@ func setup(t *testing.T, filename, eventType string) *testContext {
210 210
 		buildCfg: &api.BuildConfig{
211 211
 			Triggers: []api.BuildTriggerPolicy{
212 212
 				{
213
-					Type: api.GithubWebHookBuildTriggerType,
214
-					GithubWebHook: &api.WebHookTrigger{
213
+					Type: api.GitHubWebHookBuildTriggerType,
214
+					GitHubWebHook: &api.WebHookTrigger{
215 215
 						Secret: "secret101",
216 216
 					},
217 217
 				},
... ...
@@ -74,8 +74,8 @@ func (c *buildConfigs) WebHookURL(name string, trigger *buildapi.BuildTriggerPol
74 74
 		switch {
75 75
 		case trigger.GenericWebHook != nil:
76 76
 			return c.r.Get().Namespace(c.ns).Resource("buildConfigHooks").Name(name).Suffix(trigger.GenericWebHook.Secret, "generic").URL(), nil
77
-		case trigger.GithubWebHook != nil:
78
-			return c.r.Get().Namespace(c.ns).Resource("buildConfigHooks").Name(name).Suffix(trigger.GithubWebHook.Secret, "github").URL(), nil
77
+		case trigger.GitHubWebHook != nil:
78
+			return c.r.Get().Namespace(c.ns).Resource("buildConfigHooks").Name(name).Suffix(trigger.GitHubWebHook.Secret, "github").URL(), nil
79 79
 		default:
80 80
 			return nil, ErrTriggerIsNotAWebHook
81 81
 		}
... ...
@@ -83,8 +83,8 @@ func (c *buildConfigs) WebHookURL(name string, trigger *buildapi.BuildTriggerPol
83 83
 	switch {
84 84
 	case trigger.GenericWebHook != nil:
85 85
 		return c.r.Get().Namespace(c.ns).Resource("buildConfigs").Name(name).SubResource("webhooks").Suffix(trigger.GenericWebHook.Secret, "generic").URL(), nil
86
-	case trigger.GithubWebHook != nil:
87
-		return c.r.Get().Namespace(c.ns).Resource("buildConfigs").Name(name).SubResource("webhooks").Suffix(trigger.GithubWebHook.Secret, "github").URL(), nil
86
+	case trigger.GitHubWebHook != nil:
87
+		return c.r.Get().Namespace(c.ns).Resource("buildConfigs").Name(name).SubResource("webhooks").Suffix(trigger.GitHubWebHook.Secret, "github").URL(), nil
88 88
 	default:
89 89
 		return nil, ErrTriggerIsNotAWebHook
90 90
 	}
... ...
@@ -33,8 +33,8 @@ func (c *FakeBuildConfigs) WebHookURL(name string, trigger *buildapi.BuildTrigge
33 33
 	switch {
34 34
 	case trigger.GenericWebHook != nil:
35 35
 		return url.Parse(fmt.Sprintf("http://localhost/buildConfigHooks/%s/%s/generic", name, trigger.GenericWebHook.Secret))
36
-	case trigger.GithubWebHook != nil:
37
-		return url.Parse(fmt.Sprintf("http://localhost/buildConfigHooks/%s/%s/github", name, trigger.GithubWebHook.Secret))
36
+	case trigger.GitHubWebHook != nil:
37
+		return url.Parse(fmt.Sprintf("http://localhost/buildConfigHooks/%s/%s/github", name, trigger.GitHubWebHook.Secret))
38 38
 	default:
39 39
 		return nil, client.ErrTriggerIsNotAWebHook
40 40
 	}
... ...
@@ -189,7 +189,7 @@ func RunListBuildWebHooks(f *clientcmd.Factory, out, errOut io.Writer, name stri
189 189
 			if prefix {
190 190
 				hookType = "generic "
191 191
 			}
192
-		case t.GithubWebHook != nil && github:
192
+		case t.GitHubWebHook != nil && github:
193 193
 			if prefix {
194 194
 				hookType = "github "
195 195
 			}
... ...
@@ -129,9 +129,9 @@ func webhookURL(c *buildapi.BuildConfig, cli client.BuildConfigsNamespacer) map[
129 129
 	for _, trigger := range c.Triggers {
130 130
 		whTrigger := ""
131 131
 		switch trigger.Type {
132
-		case "github":
133
-			whTrigger = trigger.GithubWebHook.Secret
134
-		case "generic":
132
+		case buildapi.GitHubWebHookBuildTriggerType:
133
+			whTrigger = trigger.GitHubWebHook.Secret
134
+		case buildapi.GenericWebHookBuildTriggerType:
135 135
 			whTrigger = trigger.GenericWebHook.Secret
136 136
 		}
137 137
 		if len(whTrigger) == 0 {
... ...
@@ -118,8 +118,8 @@ func (r *SourceRef) BuildSource() (*buildapi.BuildSource, []buildapi.BuildTrigge
118 118
 			ContextDir: r.ContextDir,
119 119
 		}, []buildapi.BuildTriggerPolicy{
120 120
 			{
121
-				Type: buildapi.GithubWebHookBuildTriggerType,
122
-				GithubWebHook: &buildapi.WebHookTrigger{
121
+				Type: buildapi.GitHubWebHookBuildTriggerType,
122
+				GitHubWebHook: &buildapi.WebHookTrigger{
123 123
 					Secret: generateSecret(20),
124 124
 				},
125 125
 			},
... ...
@@ -124,8 +124,8 @@ func mockBuildConfig() *buildapi.BuildConfig {
124 124
 		},
125 125
 		Triggers: []buildapi.BuildTriggerPolicy{
126 126
 			{
127
-				Type: buildapi.GithubWebHookBuildTriggerType,
128
-				GithubWebHook: &buildapi.WebHookTrigger{
127
+				Type: buildapi.GitHubWebHookBuildTriggerType,
128
+				GitHubWebHook: &buildapi.WebHookTrigger{
129 129
 					Secret: "secret101",
130 130
 				},
131 131
 			},
... ...
@@ -288,8 +288,8 @@ func mockBuildConfigImageParms(imageName, imageStream, imageTag string) *buildap
288 288
 		},
289 289
 		Triggers: []buildapi.BuildTriggerPolicy{
290 290
 			{
291
-				Type: buildapi.GithubWebHookBuildTriggerType,
292
-				GithubWebHook: &buildapi.WebHookTrigger{
291
+				Type: buildapi.GitHubWebHookBuildTriggerType,
292
+				GitHubWebHook: &buildapi.WebHookTrigger{
293 293
 					Secret: "secret101",
294 294
 				},
295 295
 			},
... ...
@@ -329,8 +329,8 @@ func mockBuildConfigImageStreamParms(imageName, imageStream, imageTag string) *b
329 329
 		},
330 330
 		Triggers: []buildapi.BuildTriggerPolicy{
331 331
 			{
332
-				Type: buildapi.GithubWebHookBuildTriggerType,
333
-				GithubWebHook: &buildapi.WebHookTrigger{
332
+				Type: buildapi.GitHubWebHookBuildTriggerType,
333
+				GitHubWebHook: &buildapi.WebHookTrigger{
334 334
 					Secret: "secret101",
335 335
 				},
336 336
 			},