lib: add validation package + DNS label validation #12535
Conversation
eculver
requested review from
rboyer,
freddygv,
ndhanushkodi,
kisunji and
a team
github-actions
bot
added
the
theme/internals
|
I left a comment in the mirrored ent PR, I think it could be useful to merge this into the |
|
|
Co-authored-by: Chris S. Kim <ckim@hashicorp.com>
| "foo-": false, | ||
| "-foo-": false, | ||
| "-foo-bar-": false, | ||
| "no spaces allowed": false, |
There was a problem hiding this comment.
can we add cases for names that are at the boundary length-wise? 64 chars invalid, 63 chars valid
There was a problem hiding this comment.
Hah, this is actually really interesting. I pulled the pattern from the existing code in Enterprise and looking at it more closely, it clearly allows 64 characters when it should only allow 63 according to the RFC.
So, we just fixed a very nuanced bug. Good catch!
There was a problem hiding this comment.
Fixed now, but I want to leave this comment exposed for posterity since it's so fun.
There was a problem hiding this comment.
Nice, this is why we test :P
eculver
added
the
pr/no-changelog
eculver
removed
the
pr/no-changelog
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/608309. |
For future work with mesh federation, we want to validate peer names as valid RFC 1123 DNS labels. This validation already exists, but only in Enterprise and we need the functionality in both OSS and Enterprise. So, I pulled it out and put it into a standalone package under
lib.Note: the regex and tests are the same from the original code in Enterprise.
Ask for reviewers: Is this an ok place to put this? I'm also open to renaming things if it makes sense. I'll also link to the PRs in Enterprise that make use of this package and refactor the previous call sites to use this interface.
Notes to reviewers:
IsValidDNSLabeland aRequireValidDNSLabelto a) preserve existingboolreturn + error messages and b) also allow validation rule to be returned to callers if need be -- this is how the mesh federation code does it.