Add URL validation #97 #100
Conversation
|
Welcome @u5surf! It looks like this is your first PR to GoogleCloudPlatform/k8s-cluster-bundle 🎉 |
pkg/validation/validate_url.go
Outdated
| ) | ||
|
|
||
| // ValidateURL validates urls in object files follwing Go's net/url parsing rules. | ||
| func ValidateURL(u string) error { |
There was a problem hiding this comment.
Let's make this private; I don't think it needs to be public
|
Thanks for the PR! |
pkg/validation/validate_url_test.go
Outdated
| return | ||
| } | ||
| return // no error, not a problem | ||
| } else { |
There was a problem hiding this comment.
nit: don't need the else here.
|
Can you update the bazel BUILD file? Once you have |
|
/retest |
|
/ok-to-test |
pkg/validation/validate_url_test.go
Outdated
| desc string | ||
| in string | ||
| scheme string | ||
| path string |
There was a problem hiding this comment.
Let's just remove path -- it's not used any longer.
| t.Run(tc.desc, func(t *testing.T) { | ||
| err := validateURL(tc.in) | ||
| if err == nil { | ||
| if tc.errSubstr != "" && !strings.Contains(err.Error(), tc.errSubstr) { |
There was a problem hiding this comment.
this isn't quite right. err == nil means that err.Error() is dereferencing a nil object.
There should be a case where err != nil and you check this. I got this wrong in validate_names, which is probably why this is wrong here. I'll go fix that.
|
/lgtm |
#97
I added url validation following rules of Go’s net/url Parse.