add DNS name validation, remove TODO #6674
Conversation
xichengliudui
force-pushed
the
DNS-name-validation
branch
from
18d8bb1
to
0cc3232
Compare
proxy-identity/main.go
Outdated
| if id == "" { | ||
| return nil, errors.New("a non-empty identity is required") | ||
| } | ||
|
|
||
| if err := validation.IsFullyQualifiedDomainName(field.NewPath(""), id).ToAggregate(); err != nil { | ||
| return nil, errors.New("a qualified DNS identity is required") |
There was a problem hiding this comment.
let's put the id in the error message to make this error more helpful.
xichengliudui
force-pushed
the
DNS-name-validation
branch
2 times, most recently
from
b6becf3
to
249f887
Compare
There was a problem hiding this comment.
Thanks @xichengliudui. I have a trivial comment around the error message, but otherwise this looks good.
Signed-off-by: liudui <[email protected]>
xichengliudui
force-pushed
the
DNS-name-validation
branch
from
249f887
to
193ebd5
Compare
There was a problem hiding this comment.
Thanks @xichengliudui! As a future note, it'd be great if you can avoid force-pushing to branches. It makes re-reviewing difficult because it removes the ability to see the changes incrementally—from before and after a review.
Aside from that, thanks for the change!
Thank you for reminding. I'll see to it. |
|
Just for reference, these names are typically of the form |
Subject
add DNS name validation, remove TODO
Problem
non-empty identity can pass the verification whether qualified or not
Signed-off-by: liudui [email protected]