feat(v2): Make ResourceNames::ensure_max_length public - #1260
Conversation
| /// Maximum length of annotation names | ||
| pub const MAX_ANNOTATION_NAME_LENGTH: usize = 63; |
There was a problem hiding this comment.
This constant belongs in kvp::key and should be named MAX_KEY_NAME_LENGTH.
Rather than exposing the value publicly, it might be cleaner to offer a sanitize function that takes a key prefix and name and returns a valid kvp::key::Key.
There was a problem hiding this comment.
I removed the constant and added the requested function. However, I didn't call it sanitize, as to me that gives off security vibes, but happy to discuss the name :)
| pub fn ensure_max_length( | ||
| resource_name: String, | ||
| max_length: usize, | ||
| hash_length: usize, | ||
| ) -> String { |
There was a problem hiding this comment.
This function was written for resource names, where the assertions always hold. If it's now used to shorten arbitrary strings (which could also come from the user) then you can expect the operator to crash.
That said, I do see the usefulness. I'd suggest making it safe for arbitrary strings (which isn't trivial) and moving it to a string utility module.
There was a problem hiding this comment.
Moved it into crates/stackable-operator/src/utils/length_enforcement.rs.
which isn't trivial
However, I'm pretty sure I missed something, will test non-asci strings now
Description
I need this helper function to fix a bug in commons-operator: stackabletech/commons-operator#443
Definition of Done Checklist
Author
Reviewer
Acceptance