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

Fix path prefix handling in server address #158

Merged
merged 2 commits into from
Aug 15, 2019
Merged

Fix path prefix handling in server address #158

merged 2 commits into from
Aug 15, 2019

Conversation

kke
Copy link
Contributor

@kke kke commented Aug 15, 2019

Fixes #157
See #110

@kke kke added the bug Something isn't working label Aug 15, 2019
@rbq
Copy link

rbq commented Aug 15, 2019

Yay! I can verify that with this workaround it runs with Rancher!

I assume it breaks using a path (prefix) like /api/api though?

@kke
Copy link
Contributor Author

kke commented Aug 15, 2019

Yep, that's why that check was left out originally. I still don't understand why it gets added twice, and I have not been able to reproduce the problem in the console.

@kke kke merged commit 062c73d into master Aug 15, 2019
@kke kke deleted the fix/prefix_path branch August 15, 2019 12:02
@kke
Copy link
Contributor Author

kke commented Aug 15, 2019

@rbq
Copy link

rbq commented Aug 15, 2019

Yeah, I tried to understand what's going on but quickly gave up. In K8s::Transport everything seemed fine; when sending a request it builds the path with prefix.

But for some reason K8s::APIClient also reaches into K8s::Transport, adding the prefix a second time?

def path(*path)
  @transport.path(self.class.path(@api_version), *path)
end

So I tried to change that to just:

def path(*path)
  File.join self.class.path(@api_version), *path
end

The result seemed to be one request with a correct URL (not sure what's going on there, discovering available APIs or something?)—immediately followed by an exception with a path that was now missing the prefix. 🤯

@rbq
Copy link

rbq commented Aug 15, 2019

Anyway, thanks a lot! I'll now try building a little Sinatra app listing all the Ingresses deployed on our cluster. :)

@kke kke mentioned this pull request Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server addresses with path-prefix do not work
2 participants