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.

Sign up for GitHub

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

Jump to bottom

ability to clear headers on outbound requests #1218

Open
siggy opened this issue Apr 16, 2017 · 4 comments
Open

ability to clear headers on outbound requests #1218

siggy opened this issue Apr 16, 2017 · 4 comments
Labels
enhancement help wanted

Comments

@siggy
Copy link
Member

siggy commented Apr 16, 2017

A user reported not being able to proxy requests via Linkerd to Loggly:
https://stackoverflow.com/questions/43295644/node-global-tunnel-and-loggly-and-linkerd

It turns out Loggly returns a 400 when a request has headers with forward slashes in it:

normal request:

$ curl -H "foo: bar" https://logs-01.loggly.com/
Invalid API key

failed request:

$ curl -H "l5d-dst-logical: /svc/logs-01.loggly.com" https://logs-01.loggly.com/
<html><head><title>Bad Request</title></head><body><h1>400 Bad Request</h1></body></html>

While it's debatable whether refusing this class of requests is valid, it would still be helpful for linkerd to support proxying to Loggly. Additionally, there may be use cases where operators simply do not want to leak information from inside their clusters to the internet.

We note in the linkerd documentation that Operators may opt to remove this header from responses sent to the outside world. This implies setting up an additional service/nginx/proxy/something to strip headers on outbound requests. It would be more convenient to the operator for linkerd to handle this for them.
@adleong
Copy link
Member

adleong commented Apr 17, 2017

Relatedly, we could also strip off informational headers linkerd sends back on the response. I'm not sure if there are any yet but https://github.com/linkerd/linkerd/pull/1185/files would add l5d-success-class to indicate response classification.

@olix0r
Copy link
Member

olix0r commented Apr 17, 2017 via email

@olix0r
Copy link
Member

olix0r commented Apr 18, 2017

@adleong I think it would make sense for what you describe to be rolled into the clearContext feature...

@olix0r
Copy link
Member

olix0r commented Apr 18, 2017

No, slashes are obviously allowed in http headers (e.g. text/html). loggy is silly.

This does seem like a reasonable client feature.

@esbie esbie marked this as a duplicate of #1513 Jul 18, 2017
@esbie esbie mentioned this issue Jul 18, 2017
Closed
Tim-Brooks pushed a commit to Tim-Brooks/linkerd that referenced this issue Dec 20, 2018
@hawkw
proxy: Replace >=100,000 ms latency buckets with 1, 2, 3, 4, and 5 ms (…
06d0209 
…linkerd#1218)

This branch adds buckets for latencies below 10 ms to the proxy's latency
histograms, and removes the buckets for 100, 200, 300, 400, and 500 
seconds, so the largest non-infinity bucket is 50,000 ms. It also removes
comments that claimed that these buckets were the same as those created
by the control plane, as this is no longer true (the metrics are now scraped
by Prometheus from the proxy directly).

Closes linkerd#1208

Signed-off-by: Eliza Weisman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@siggy @olix0r @adleong