Validation cleanup part 7 #18694
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,15 +65,15 @@ func TestValidateObjectMetaCustomName(t *testing.T) { | |
| func TestValidateObjectMetaNamespaces(t *testing.T) { | ||
| errs := ValidateObjectMeta( | ||
| &api.ObjectMeta{Name: "test", Namespace: "foo.bar"}, | ||
| true, | ||
| func(s string, prefix bool) (bool, string) { | ||
| return true, "" | ||
| }, | ||
| field.NewPath("field")) | ||
| if len(errs) != 1 { | ||
| t.Fatalf("unexpected errors: %v", errs) | ||
| } | ||
| if !strings.Contains(errs[0].Error(), `Invalid value: "foo.bar"`) { | ||
| t.Errorf("unexpected error message: %v", errs) | ||
| } | ||
| maxLength := 63 | ||
|
|
@@ -84,15 +84,15 @@ func TestValidateObjectMetaNamespaces(t *testing.T) { | |
| } | ||
| errs = ValidateObjectMeta( | ||
| &api.ObjectMeta{Name: "test", Namespace: string(b)}, | ||
| true, | ||
| func(s string, prefix bool) (bool, string) { | ||
| return true, "" | ||
| }, | ||
| field.NewPath("field")) | ||
| if len(errs) != 1 { | ||
| t.Fatalf("unexpected errors: %v", errs) | ||
| } | ||
| if !strings.Contains(errs[0].Error(), "Invalid value") { | ||
| t.Errorf("unexpected error message: %v", errs) | ||
| } | ||
| } | ||
|
|
@@ -629,23 +629,23 @@ func TestValidateVolumes(t *testing.T) { | |
| }, | ||
| "absolute path": { | ||
| []api.Volume{{Name: "absolutepath", VolumeSource: absolutePathName}}, | ||
| field.ErrorTypeInvalid, | ||
| "downwardAPI.path", "", | ||
| }, | ||
| "dot dot path": { | ||
| []api.Volume{{Name: "dotdotpath", VolumeSource: dotDotInPath}}, | ||
| field.ErrorTypeInvalid, | ||
| "downwardAPI.path", `must not contain '..'`, | ||
| }, | ||
| "dot dot file name": { | ||
| []api.Volume{{Name: "dotdotfilename", VolumeSource: dotDotPathName}}, | ||
| field.ErrorTypeInvalid, | ||
| "downwardAPI.path", `must not start with '..'`, | ||
| }, | ||
| "dot dot first level dirent": { | ||
| []api.Volume{{Name: "dotdotdirfilename", VolumeSource: dotDotFirstLevelDirent}}, | ||
| field.ErrorTypeInvalid, | ||
| "downwardAPI.path", `must not start with '..'`, | ||
| }, | ||
| "empty wwn": { | ||
| []api.Volume{{Name: "badimage", VolumeSource: zeroWWN}}, | ||
|
|
@@ -665,16 +665,16 @@ func TestValidateVolumes(t *testing.T) { | |
| "starts with '..'": { | ||
| []api.Volume{{Name: "badprefix", VolumeSource: startsWithDots}}, | ||
| field.ErrorTypeInvalid, | ||
| "gitRepo.directory", `must not start with '..'`, | ||
| }, | ||
| "contains '..'": { | ||
| []api.Volume{{Name: "containsdots", VolumeSource: containsDots}}, | ||
| field.ErrorTypeInvalid, | ||
| "gitRepo.directory", `must not contain '..'`, | ||
| }, | ||
| "absolute target": { | ||
| []api.Volume{{Name: "absolutetarget", VolumeSource: absPath}}, | ||
| field.ErrorTypeInvalid, | ||
| "gitRepo.directory", "", | ||
| }, | ||
| } | ||
|
|
@@ -824,12 +824,12 @@ func TestValidateEnv(t *testing.T) { | |
| { | ||
| name: "zero-length name", | ||
| envs: []api.EnvVar{{Name: ""}}, | ||
| expectedError: "[0].name: Required value", | ||
| }, | ||
| { | ||
| name: "name not a C identifier", | ||
| envs: []api.EnvVar{{Name: "a.b.c"}}, | ||
| expectedError: `[0].name: Invalid value: "a.b.c": must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName"`, | ||
| }, | ||
| { | ||
| name: "value and valueFrom specified", | ||
|
|
@@ -843,7 +843,7 @@ func TestValidateEnv(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| expectedError: "[0].valueFrom: Invalid value: \"\": may not be specified when `value` is not empty", | ||
| }, | ||
| { | ||
| name: "missing FieldPath on ObjectFieldSelector", | ||
|
|
@@ -855,7 +855,7 @@ func TestValidateEnv(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| expectedError: `[0].valueFrom.fieldRef.fieldPath: Required value`, | ||
| }, | ||
| { | ||
| name: "missing APIVersion on ObjectFieldSelector", | ||
|
|
@@ -867,7 +867,7 @@ func TestValidateEnv(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| expectedError: `[0].valueFrom.fieldRef.apiVersion: Required value`, | ||
| }, | ||
| { | ||
| name: "invalid fieldPath", | ||
|
|
@@ -880,7 +880,7 @@ func TestValidateEnv(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| expectedError: `[0].valueFrom.fieldRef.fieldPath: Invalid value: "metadata.whoops": error converting fieldPath`, | ||
| }, | ||
| { | ||
| name: "invalid fieldPath labels", | ||
|
|
@@ -893,7 +893,7 @@ func TestValidateEnv(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| expectedError: `[0].valueFrom.fieldRef.fieldPath: Unsupported value: "metadata.labels": supported values: metadata.name, metadata.namespace, status.podIP`, | ||
| }, | ||
| { | ||
| name: "invalid fieldPath annotations", | ||
|
|
@@ -906,7 +906,7 @@ func TestValidateEnv(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| expectedError: `[0].valueFrom.fieldRef.fieldPath: Unsupported value: "metadata.annotations": supported values: metadata.name, metadata.namespace, status.podIP`, | ||
| }, | ||
| { | ||
| name: "unsupported fieldPath", | ||
|
|
@@ -919,7 +919,7 @@ func TestValidateEnv(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| expectedError: `valueFrom.fieldRef.fieldPath: Unsupported value: "status.phase": supported values: metadata.name, metadata.namespace, status.podIP`, | ||
|
There was a problem hiding this comment. The multiple colons in a single sentence still makes me frown. Is this following some tradition? There was a problem hiding this comment. It's pretty go-style - add details: at the front. On Tue, Dec 15, 2015 at 11:42 PM, Chao Xu notifications@github.com wrote:
|
||
| }, | ||
| } | ||
| for _, tc := range errorCases { | ||
|
|
@@ -3314,7 +3314,7 @@ func TestValidateLimitRange(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| "may not be specified when `type` is 'Pod'", | ||
| }, | ||
| "default-request-limit-type-pod": { | ||
| api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: api.LimitRangeSpec{ | ||
|
|
@@ -3327,7 +3327,7 @@ func TestValidateLimitRange(t *testing.T) { | |
| }, | ||
| }, | ||
| }}, | ||
| "may not be specified when `type` is 'Pod'", | ||
| }, | ||
| "min value 100m is greater than max value 10m": { | ||
| api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: api.LimitRangeSpec{ | ||
|
|
@@ -4002,7 +4002,7 @@ func TestValidateEndpoints(t *testing.T) { | |
| }, | ||
| }, | ||
| errorType: "FieldValueInvalid", | ||
| errorDetail: "must be a valid IPv4 address", | ||
| }, | ||
| "Multiple ports, one without name": { | ||
| endpoints: api.Endpoints{ | ||
|
|
@@ -4052,7 +4052,7 @@ func TestValidateEndpoints(t *testing.T) { | |
| }, | ||
| }, | ||
| errorType: "FieldValueInvalid", | ||
| errorDetail: "must be a valid IPv4 address", | ||
| }, | ||
| "Port missing number": { | ||
| endpoints: api.Endpoints{ | ||
|
|
@@ -4122,7 +4122,7 @@ func TestValidateEndpoints(t *testing.T) { | |
|
|
||
| for k, v := range errorCases { | ||
| if errs := ValidateEndpoints(&v.endpoints); len(errs) == 0 || errs[0].Type != v.errorType || !strings.Contains(errs[0].Detail, v.errorDetail) { | ||
| t.Errorf("[%s] Expected error type %s with detail %q, got %v", k, v.errorType, v.errorDetail, errs) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -4172,7 +4172,7 @@ func TestValidateSecurityContext(t *testing.T) { | |
| } | ||
| for k, v := range successCases { | ||
| if errs := ValidateSecurityContext(v.sc, field.NewPath("field")); len(errs) != 0 { | ||
| t.Errorf("[%s] Expected success, got %v", k, errs) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -4192,17 +4192,17 @@ func TestValidateSecurityContext(t *testing.T) { | |
| "request privileged when capabilities forbids": { | ||
| sc: privRequestWithGlobalDeny, | ||
| errorType: "FieldValueForbidden", | ||
| errorDetail: "disallowed by policy", | ||
| }, | ||
| "negative RunAsUser": { | ||
| sc: negativeRunAsUser, | ||
| errorType: "FieldValueInvalid", | ||
| errorDetail: isNegativeErrorMsg, | ||
| }, | ||
| } | ||
| for k, v := range errorCases { | ||
| if errs := ValidateSecurityContext(v.sc, field.NewPath("field")); len(errs) == 0 || errs[0].Type != v.errorType || !strings.Contains(errs[0].Detail, v.errorDetail) { | ||
| t.Errorf("[%s] Expected error type %q with detail %q, got %v", k, v.errorType, v.errorDetail, errs) | ||
| } | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A
formatting requirement? It should be consistent with the next point.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I specifically split the negative case into formatting and behavioral, but in the positive sense there is no difference in words.