-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(model): support labels in manifest #254
Conversation
0e93bed
to
b7cd7ba
Compare
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.
Looks good to me!
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.
One blocking comment around things to test for. Others are optional
335f020
to
274aa4b
Compare
Signed-off-by: Brooks Townsend <[email protected]>
b7cd7ba
to
4b838df
Compare
// ends with an alphanumeric character, and contains only alphanumeric characters or hyphens | ||
!part.is_empty() | ||
&& part.len() <= 63 | ||
&& part.starts_with(|c: char| c.is_ascii_alphabetic()) |
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.
Super nit: you could just do part.starts_with(char::is_ascii_alphabetic)
Feature or Problem
This PR adds supports for serializing the labels in a manifest and validating that they are correct according to the kubernetes label specification, which also matches the OAM specification.
Related Issues
Fixes #249
Release Information
Next wadm
Consumer Impact
Testing
Unit Test(s)
Added unit test to sanity check validation rules.
Acceptance or Integration
Manual Verification
With a manifest:
The requested spec comes back as (including the label):