Skip to content
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

Backport of Hash namespace+proxy ID when creating socket path into release/1.15.x #17271

Conversation

hc-github-team-consul-core
Copy link
Collaborator

Backport

This PR is auto-generated from #17204 to be assessed for backporting due to the inclusion of the label backport/1.15.

🚨

Warning automatic cherry-pick of commits failed. If the first commit failed,
you will see a blank no-op commit below. If at least one commit succeeded, you
will see the cherry-picked commits up to, not including, the commit where
the merge conflict occurred.

The person who merged in the original PR is:
@freddygv
This person should manually cherry-pick the original PR into a new backport PR,
and close this one when the manual backport PR is merged in.

merge conflict error: POST https://api.github.com/repos/hashicorp/consul/merges: 409 Merge conflict []

The below text is copied from the body of the original PR.


Description

UNIX domain socket paths are limited to 104-108 characters depending on the OS. This limit was quite easy to exceed when testing the feature on Kubernetes, due to how proxy IDs encode the Pod ID eg:
metrics-collector-59467bcb9b-fkkzl-hcp-metrics-collector-sidecar-proxy

To ensure we stay under the lower 104 character limit this commit makes a couple changes:

  • Use a b64 encoded SHA1 hash of the namespace + proxy ID to create a short and deterministic socket file name.
  • Add validation to proxy registrations and proxy-defaults to enforce a limit on the socket directory length.

Testing & Reproduction steps

  • Added unit tests
  • Manual testing with long socket path at and above limit

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

Overview of commits

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/cc-4929-cap-socket-path/partially-accepted-mallard branch 2 times, most recently from 500178a to 9b04f1c Compare May 9, 2023 18:25
@hashicorp-cla
Copy link

hashicorp-cla commented May 9, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto approved Consul Bot automated PR

UNIX domain socket paths are limited to 104-108 characters, depending on
the OS. This limit was quite easy to exceed when testing the feature on
Kubernetes, due to how proxy IDs encode the Pod ID eg:
metrics-collector-59467bcb9b-fkkzl-hcp-metrics-collector-sidecar-proxy

To ensure we stay under that character limit this commit makes a
couple changes:
- Use a b64 encoded SHA1 hash of the namespace + proxy ID to create a
  short and deterministic socket file name.
- Add validation to proxy registrations and proxy-defaults to enforce a
  limit on the socket directory length.
@freddygv freddygv force-pushed the backport/cc-4929-cap-socket-path/partially-accepted-mallard branch from 0752fc2 to c0c160d Compare May 9, 2023 19:36
@freddygv freddygv marked this pull request as ready for review May 9, 2023 20:24
@freddygv freddygv merged commit 5feb71e into release/1.15.x May 9, 2023
@freddygv freddygv deleted the backport/cc-4929-cap-socket-path/partially-accepted-mallard branch May 9, 2023 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants