jgnagy commented 5 years ago

Some Kubernetes platforms (like Rancher 2) proxy kubernetes API requests through a reverse-proxy. The kubectl command (as well as other kubernetes tools that use it) work properly with this approach. This PR brings k8s-client closer to the behavior users expect from tools like these.

I experienced this issue while trying to use mortar and tracked the issue down to how this library was handling proxied requests like these.

While the testing that I added is very far from exhaustive, it does at least ensure that when provided a server with a URI path, it is handled correctly from a Transport perspective. I also verified that I can now see resources in my Rancher cluster.

Hopefully this can be reviewed and released so I can pester the mortar maintainers to pull it in at that level.

jgnagy commented 5 years ago

Looks like the build in Travis is only failing because of bundler 2.x vs 1.x differences in 2.5. It certainly isn't caused by anything I did, but if you need me to try and resolve it I'm happy to.

kke commented 5 years ago

I have made a bunch of rubocop / dry-types / travis fixes that were included in #112

It should remove the unrelated changes from this PR.

jnummelin commented 5 years ago

Let's get #112 merged first to make master branch more stable

kke commented 5 years ago

112 merged, please rebase / merge master.

jgnagy commented 5 years ago

It looks like #112 removed rake from the gemspec which will make testing/linting a little more difficult, FYI (I just ran bundle exec rubocop and bundle exec rspec).

jgnagy commented 5 years ago

Merged in from master, retested, and pushed.

jakolehm commented 5 years ago

@kke PTAL

jgnagy commented 5 years ago

@kke, please review my comments and the additional commit. I made the initializer change you mentioned for @server, though this appears to now conflict what a more recent change for auth. Should be fairly easy to resolve but I will defer to you on how to do so.

kke commented 5 years ago

There was some sort of recursion going on

      result = path.join('/')
      result = File.join(path_prefix, path) unless result.start_with?(path_prefix)
                                                     # ^^^ the original array, should've been "result" probably.
      result.start_with?('/') ? result : "/#{result}"

I resolved the conflict and made it a bit prettier now.

kke commented 5 years ago

@jakolehm seems fine to me now, ptala.

rbq commented 5 years ago

@jgnagy How do you configure the client for Rancher? I'm using my existing ~/.kube/config, which creates a client with @path_prefix="/k8s/clusters/c-blahblah/", but then any requests fails with the path in there twice:

GET /k8s/clusters/c-blahblah/k8s/clusters/c-blahblah/api/v1 => HTTP 404 Not Found`

kke commented 5 years ago

@rbq what does your request look like? (client.api('v1').resource('xyz').get) ?)

rbq commented 5 years ago

@kke I tried different things, e.g. the example from the README. Right now my script looks like this:

#!/usr/bin/env ruby
require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'k8s-client'
end

config = K8s::Config.load_file File.expand_path '~/.kube/config'
client = K8s::Client.config config

client.api('v1').resource('ingresses', namespace: 'default').list().each do |i|
  puts "namespace=#{i.metadata.namespace} name=#{i.metadata.name}"
end

rbq commented 5 years ago

Oh, and the rest of the exception, in case that's useful in any way:

Traceback (most recent call last):
    7: from ./index.rb:12:in `<main>'
    6: from […]/gems/k8s-client-0.10.2/lib/k8s/api_client.rb:73:in `resource'
    5: from […]/gems/k8s-client-0.10.2/lib/k8s/api_client.rb:61:in `find_api_resource'
    4: from […]/gems/k8s-client-0.10.2/lib/k8s/api_client.rb:54:in `api_resources'
    3: from […]/gems/k8s-client-0.10.2/lib/k8s/api_client.rb:46:in `api_resources!'
    2: from […]/gems/k8s-client-0.10.2/lib/k8s/transport.rb:362:in `get'
    1: from […]/gems/k8s-client-0.10.2/lib/k8s/transport.rb:286:in `request'
[…]/gems/k8s-client-0.10.2/lib/k8s/transport.rb:264:in `parse_response': ⏎
GET /k8s/clusters/c-blahblah/k8s/clusters/c-blahblah/api/v1 => HTTP 404 ⏎
Not Found: {"paths"=>["/apis", "/apis/", "/apis/apiextensions.k8s.io", ⏎
"/apis/apiextensions.k8s.io/v1beta1", "/healthz", "/healthz/etcd", ⏎
"/healthz/log", "/healthz/ping", "/healthz/poststarthook/crd-informer-synced", ⏎
"/healthz/poststarthook/generic-apiserver-start-informers", ⏎
"/healthz/poststarthook/start-apiextensions-controllers", ⏎
"/healthz/poststarthook/start-apiextensions-informers", "/metrics", ⏎
"/openapi/v2", "/version"]} (K8s::Error::NotFound)

jgnagy commented 5 years ago

@rbq, my original PR for this didn't have this issue, but as I mentioned in this comment, the PR was modified by @kke to change the behavior. I have not been able to use this k8s-client with Rancher given how k8s-client handles this proxying. My team just drops to shell commands for kubectl for most things at this point, though I hope this is resolved eventually.

kke commented 5 years ago

@rbq / @jgnagy can you try with the branch of #158 ?

#!/usr/bin/env ruby
require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'k8s-client', github: 'kontena/k8s-client', branch: 'fix/prefix_path'
end

config = K8s::Config.load_file File.expand_path '~/.kube/config'
client = K8s::Client.config config

client.api('v1').resource('ingresses', namespace: 'default').list().each do |i|
  puts "namespace=#{i.metadata.namespace} name=#{i.metadata.name}"
end

rbq commented 5 years ago

@kke The branch works (but unfortuanately my script doesn't, since Ingress is not actually a v1 resource ;).

kontena / k8s-client

Allowing URI path prefixes in server URLs #110

112 merged, please rebase / merge master.