feat: Add Key::shortened_to_valid_length helper function - #1260
feat: Add Key::shortened_to_valid_length helper function#1260sbernauer wants to merge 10 commits into
Key::shortened_to_valid_length helper function#1260Conversation
siegfriedweber
moved this from Development: Waiting for Review
to Development: In Review
in Stackable Engineering
sbernauer
changed the title
ResourceNames::ensure_max_length publicKey::shortened_to_valid_length helper function
| prefix: impl Into<String>, | ||
| name: impl Into<String>, | ||
| ) -> Result<Self, KeyError> { | ||
| let prefix = ensure_max_length(prefix, KEY_PREFIX_MAX_LEN, 8); |
There was a problem hiding this comment.
Shortening the prefix is pointless: if it isn't already a valid DNS subdomain name, shortening won't make it one. In particular, a prefix must end in a letters-only TLD, but the appended hash adds a hyphen and probably digits, guaranteeing an invalid result.
A unit test for both the shortened and unshortened case would be good.
There was a problem hiding this comment.
I'm glad that you know this stuff better than I do 😅
Fixed in a9cbd52. Aalso made the prefix optional, so that callers can only pass the name
| /// # Panics | ||
| /// | ||
| /// Panics if `max_length_bytes < 1 /* character */ + 1 /* dash */ + hash_length`. | ||
| pub fn ensure_max_length( |
There was a problem hiding this comment.
nit: utils::length_enforcement::ensure_max_length reads as generic, but it only works on strings. What would the corresponding function for collections like vectors be called?
There was a problem hiding this comment.
I renamed it to ensure_max_string_length in 0c8b4fc. But totally happy to name it to something else if you recommend
2 participants
Description
I need this helper function to fix a bug in commons-operator: stackabletech/commons-operator#443
Definition of Done Checklist
Author
Reviewer
Acceptance