Fix cluster naming for really long service names

[kflynn]

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.

I pretty much have tests running on my M1 Pro Mac after this PR.

Fixes #4354. Best reviewed commit-by-commit:

• 1545a2a: Clean up a duplicated entry in docs/releaseNotes.yml
• c5a6bd8: fix a buglet where using query_parameters or regex_query_parameters would alter the internal name of the Mapping object. Should have no end-user-visible effect, but OMG was it making debugging weird.
• 3a5f6ae: fix Envoy cluster creation issue with conflicting service names #4354 by changing the way we do hashing for collision resolution.
• 607f005: big alterations to TestFakeCollision to better test everything.
• cb2351e: more test changes so that my M1 Mac can run tests

Signed-off-by: Flynn emissary@flynn.kodachi.com

• I made sure to update CHANGELOG.md.
• 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.
• I didn't need to update DEVELOPING.md.
• The changes in this PR have been reviewed for security concerns and adherence to security best practices.

[LanceEa]

@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.

[LanceEa]

leaving a comment to make github review like me 😄.

[kflynn]

Hey @LanceEa! I modifiedsubstantially rewrote the TestFakeCollision test to do a better job of testing this; more info in the lead comment above. 🙂

[LanceEa]

@kflynn - In general, I'm good with the changes. I just had a few questions so I make sure my understanding is correct.

[LanceEa]

Just trying to make sure I understand this due to my lack of python skills. Envoy limits us to 60 characters so when it is greater than that we need to shorten and merge clusters.

The first 39 characters + 1 static dash ("-") + 15 character hash so we will shorten it to 55 characters...So did I interpret my python code correctly? If so any reason for only allowing 55 characters rather than full 60?

[kflynn]

name[0:40] is actually the first 40 characters rather than the first 39, and there's a two-character suffix at the end (-0), so it's 40 + "-" + 16 for the hash + -0 == 59 characters, which is close enough that I didn't want to push my luck. 😉

(The -0 is all but certainly unneeded at this point, too, but I still need to finish convincing myself of that.)

[LanceEa]

Ok, that makes sense and understood. Yeah, I would think introducing the hash negates any reason for keeping the suffix around. Do you want to remove that as part of this PR or follow-up when you have had more time to think of the repercussions of that change?

[kflynn]

The latter, yeah. Hopefully soon. 🤞

[LanceEa]

@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.

[LanceEa]

@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.

[Alice-Lilith]

lgtm, thanks @kflynn 🙂

[ddymko]

looks good