emissary
emissary copied to clipboard
Fix cluster naming for really long service names
Without this fix, the collision-handling code for a cluster with a really long service name will fail, because the really really long service name will push all the stuff that's supposed to disambiguate the cluster names off past the point of relevance. 🤦♂️
With this fix, we stick a cryptographic hash of the whole too-long cluster name into the truncated name, so that the stuff trimmed off the end is still correctly accounted for.
Note that I'm using CI to run tests here: I'm still working on getting things going on my M1 Pro Mac.
Fixes #4354.
Signed-off-by: Flynn [email protected]
- [x] I made sure to update
CHANGELOG.md
. - [x] This is unlikely to impact how Emissary performs at scale. It's going to add an SHA1 to every too-long cluster name being generated, but that should be a small fraction of cluster names.
- [ ] My change is adequately tested.
- [x] I didn't need to update
DEVELOPING.md
. - [x] The changes in this PR have been reviewed for security concerns and adherence to security best practices.
@kflynn - looks like you have a test failing.
Also, it make sense, can we just add a corresponding test to ensure it is working as reported in the issue.
Hey @LanceEa! I ~modified~substantially rewrote the TestFakeCollision
test to do a better job of testing this; more info in the lead comment above. 🙂
@kflynn - it looks good to me. Let me know what you think of removing the suffix as part of this PR?
also, I would like to get a second review, mostly for knowledge transfer so the other team members are aware of this naming convention.
@ddymko - Can you take a look at this, I think it would be good to understand this logic and how we consolidate clusters that have name collisions.