Instance creation and configuration api should be refactored

[kke]

Oddities / annoyances:

• K8s:Client.new should probably not require transport as an argument.
• K8s::Client.config takes a config and passes it to Transport.config. I think it's a misleading method name.
• K8s::Transport.config same as above, returns an instance of K8s::Transport, I would expect it to return a config instead of accepting one.
• Transport.new takes auth token/username/password, but there's K8s::Config::UserAuthProvider, maybe Transport.new should actually take an instance of some kind of Authentication class
• Transport is a mixture of abstraction for Excon and generic methods.
• The ! methods such as api_groups! and api_resources! are unintuitive. Perhaps api_groups(force_refresh: true) or something would be better.
• Method visibility in general has been ignored completely, there are no private methods. (I didn't find any methods that should be private except maybe the Util mixin should use module_function)
• K8s::Resource.from_files reads a single file or all files in a directory. I would expect it to take an array of paths or IOs.
• There's a million methods for creating a client instance.
• The client is a bit cumbersome to mock/stub because of the long method chains. It should probably have some kind of test mode / mock transport.

Other than that, it's actually pretty great.

[kke]

Also:

Do we actually need the Dry::Struct / Dry::Types for anything? Couldn't everything just be K8s::Resource( which is < RecursiveOpenStruct).

[kke]

Trying to think how it should work.

K8s::Config

This should actually be something like K8s::KubeConfig. It's not a configuration for K8s::Client. It's for parsing kubeconfig files and finding the necessary values for constructing options for a transport.

K8s::Transport

• It's supposed to be an abstraction. It should be generic. The Excon backend should perhaps be K8s::Transport::Excon. While at it, fix #57
• K8s::Transport.config(<K8s::Config>) should be K8s::KubeConfig.new(...).context.transport
• K8s::Transport.token_from_auth_provider(<K8s::Config::AuthProvider>) and K8s::Transport.token_from_exec(<K8s::Config::UserExec>) should come automatically from the above mentioned K8s::KubeConfig.new(...).context.transport
• K8s::Transport#version (and K8s::Client.version) is the same asK8s::Client.new.api('version'). They should be removed, perhaps replaced withK8s::Client#api_version` which would return a string, not a resource.
• K8s::Transport.new options should be something like: (server:, token: nil, ca: nil, username: nil, password: nil, client_cert: nil, client_key: nil, client_key_pass: nil, insecure_tls_skip_verify: false, logger: nil)

K8s::Stack

Should maybe be in another gem, such as k8s-client-stack as it's sort of custom.

K8s::Client

• K8s::Client.config and K8s::Client.in_cluster_config should be dropped
• K8s::Client.new should take (kubeconfig = nil, transport: nil, namespace: nil, insecure_tls_skip_verify: false, logger: nil, **transport_options). If transport is set, non-empty transport_options should raise. If transport is nil and transport_options is empty, configuration autodiscovery (the current K8s::Client.autoconfig) should be attempted with `logger: logger, insecure_skip_tls_verify: insecure_skip_tls_verify). If autoconfig fails, it should raise.

The user should mostly have to do:

client = K8s::Client.new
client.api('version').gitVersion

client = K8s::Client.new('~/.kube/config', context: 'foo@foofoo')

client = K8s::Client.new(server: 'https://localhost:6443', ca: 'AbCFCFGH==', token: 'abcd')

begin 
  client = K8s::Client.new
rescue K8s::Error::SSL
  client = K8s::Client.new(insecure_skip_tls_verify: true)
end

The user should not have to do anything like:

K8s::Client.new(transport: K8s::Transport.config(K8s::Config.load_file('/etc/kubernetes/admin.conf')))

K8s::Client.config(K8s::Config.load_file('/etc/kubernetes/admin.conf'))

The interface to create a client instance should be K8s::Client.new. Optionally, a shortcut for it could be K8s.client:

K8s.client(insecure_skip_tls_verify: true) do |client|
  puts client.api('version').gitVersion
end

TODO: try to split this into bite sized issues

[cben]

Some points to remember About token_from_auth_provider, token_from_exec:

1. In some cases it's bearer token, in some it's a TLS client cert. At least exec provider is generic, you don't know ahead of time which you'll get. So shouldnt have token in name, and return type should admit both. Lately I'm thinking whether auth providers can be modeled cleanly as a KubeConfig -> KubeConfig transform?

2. they might expire and require renewal. Thus user should not be involved with obtaining a token once and passing it to Client, but Client could call a method on each request to obtain fresh credentials, renewing if necessary. This will become more pressing with https://github.com/kubernetes/kubernetes/issues/70679.

3. Worse, some providers don't have expiration dates, so one gets an auth expired error and should renew and retry 😞

4. Is it useful to create multiple Clients with same config, sharing some single object that'll obtain / renew auth? If yes, this might need Client.new(SomethingDoingAuth.nSomethingDoingAuth.new(...)) rather simple Client.new.Client.new. (but it can be optional advanced use case)

See also notes on https://github.com/abonas/kubeclient/issues/393

[kke]

Perhaps the authentication modules should be implemented as some sort of http client library middleware things.

[kke]

Also, adding to https://github.com/kontena/k8s-client/issues/121#issuecomment-519513084

• There's no sane way to use Transport.new that would actually work outside of specs. All of the cert configuration is done in Transport.config which builds the options hash.