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

Apiserver proxy requires a trailing slash #4958

Closed
satnam6502 opened this issue Mar 2, 2015 · 15 comments · Fixed by #6048 or #6175
Closed

Apiserver proxy requires a trailing slash #4958

satnam6502 opened this issue Mar 2, 2015 · 15 comments · Fixed by #6048 or #6175
Assignees
Labels
area/usability priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@satnam6502
Copy link
Contributor

This works:

 https://130.211.129.169/api/v1beta1/proxy/services/monitoring-grafana/

but this does not:

 https://130.211.129.169/api/v1beta1/proxy/services/monitoring-grafana

We should somehow emphasize the need for the trailing slash for now, and then work out what is going wrong (client side scripting?) to fix this (our fault in the proxy or Grafana's?)

@vishh

@satnam6502
Copy link
Contributor Author

Likewise for Kibana. #4959

@satnam6502
Copy link
Contributor Author

For the Kibana case, I observe a sequence of correctly proxied GETs e.g. to:

fontawesome-webfont...
/api/v1beta1/proxy/services/kibana-logging/font

but then I see:

app.js
/api/v1beta1/proxy/services/app

which fails because I assume what was really needed is:

app.js
/api/v1beta1/proxy/services/kibana-logging/app

and a curl against that gives a pile of plausible Javascript.

@lavalamp
Copy link
Member

lavalamp commented Mar 2, 2015

After a little thought, I think what's going on here is that web clients can't tell that kibana-logging is a directory without the trailing slash. One thing we could potentially do to help is to redirect (301) from kibana-logging to kibana-logging/, since I don't think the former is useful for anyone.

@bgrant0607 bgrant0607 added team/master kind/support Categorizes issue or PR as a support question. labels Mar 4, 2015
@vishh vishh removed kind/support Categorizes issue or PR as a support question. priority/support labels Mar 24, 2015
@vishh vishh changed the title Grafana URL does not work if trailing slash omitted Apiserver proxy requires a trailing slash Mar 24, 2015
@vishh
Copy link
Contributor

vishh commented Mar 24, 2015

cc @nikhiljindal

@vishh vishh assigned nikhiljindal and unassigned lavalamp Mar 24, 2015
@vishh vishh added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Mar 24, 2015
@nikhiljindal
Copy link
Contributor

How are we generating that URL? Can we append a "/" at the end while generating the URL?
Doing a redirect or auto-adding "/" in the proxy for all requests could break other requests.

@vishh
Copy link
Contributor

vishh commented Mar 25, 2015

The URL is not generated anywhere. It is assumed that
'/proxy/service/' would work.

On Tue, Mar 24, 2015 at 7:04 PM, Nikhil Jindal notifications@github.com
wrote:

How are we generating that URL? Can we append a "/" at the end while
generating the URL?
Doing a redirect or auto-adding "/" in the proxy for all requests could
break other requests.


Reply to this email directly or view it on GitHub
#4958 (comment)
.

@nikhiljindal
Copy link
Contributor

I discussed this with @vishh and @lavalamp.
To clarify, proxy server does not require a trailing slash. It works as expected (gives the same response with or without a trailing slash)
The client code, on the other hand, expects it to be rendered on a path with a trailing slash and breaks when that is not the case.
Ideally, we should fix the client code.
For now, we decided to update proxy to send a 301 redirect from a path without a trailing slash to a path with a trailing slash.
I will send a PR for this.

@satnam6502
Copy link
Contributor Author

By "client code" do you mean things like Elasticsearch and Kibana?

@nikhiljindal
Copy link
Contributor

Yes. The code being rendered on those paths.

@lavalamp
Copy link
Member

I agree with what @nikhiljindal said, I just want to clarify that it's not necessarily a "broken" client, just one with different expectations. Adding the 301 redirect seems like an easy way to satisfy such clients and reduce user pain.

lavalamp added a commit that referenced this issue Mar 27, 2015
Updating proxy to return 301 to add a "/" at the end for #4958
lavalamp added a commit that referenced this issue Mar 27, 2015
Revert "Updating proxy to return 301 to add a "/" at the end for #4958"
@piosz
Copy link
Member

piosz commented Mar 30, 2015

Not fixed because of #6114

lavalamp added a commit that referenced this issue Mar 30, 2015
Updating proxy to return 301 to add a "/" at the end for #4958 (attempt 2)
@nikhiljindal
Copy link
Contributor

On GCE, the redirection is still not happening because the request is upgraded before we get a chance to redirect #6264

On localhost, the redirection works fine but we are unable to load the css and js files because of #5339 (comment)

@ddysher
Copy link
Contributor

ddysher commented May 12, 2015

@nikhiljindal Is proxy still supported in v1beta3? I vaguely remember that we wanted to remove it.

I tried this: https://ip/api/v1beta3/proxy/namespace/default/services/monitoring-grafana/ but got NotFound error.

@satnam6502
Copy link
Contributor Author

Access to cluster-level logging components depends on the proxy so avoid making ELBs.
Elasticsearch and Kibana are accessed via the v1beta3 service proxy.

@satnam6502
Copy link
Contributor Author

Although the trailing slashes are no longer required.

Kubernetes master is running at https://104.197.12.87
elasticsearch-logging is running at https://104.197.12.87/api/v1beta3/proxy/namespaces/default/services/elasticsearch-logging
kibana-logging is running at https://104.197.12.87/api/v1beta3/proxy/namespaces/default/services/kibana-logging
kube-dns is running at https://104.197.12.87/api/v1beta3/proxy/namespaces/default/services/kube-dns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/usability priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
7 participants