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 multiple problems in K8s::Client.autoconfig #107

Merged
merged 4 commits into from
Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/k8s/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,17 @@ def self.in_cluster_config(namespace: nil, **options)
# @return [K8s::Client]
def self.autoconfig(namespace: nil, **options)
if ENV.values_at('KUBE_TOKEN', 'KUBE_CA', 'KUBE_SERVER').none? { |v| v.nil? || v.empty? }
configuration = K8s::Config.build(server: ENV['KUBE_SERVER'], ca: ENV['KUBE_CA'], auth_token: options[:auth_token] || ENV['KUBE_TOKEN'])
unless Base64.decode64(ENV['KUBE_CA']).match?(/CERTIFICATE/)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this worth anything?

(The KUBE_CA must be base64, it's decoded and ran through various magics of OpenSSL in transport.rb)

It does not strict_decode64 because transport.rb doesn't either, maybe it could?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should validate that KUBE_CA is actually something usable 👍

raise ArgumentError, 'KUBE_CA does not seem to be base64 encoded'
end

begin
token = options[:auth_token] || Base64.strict_decode64(ENV['KUBE_TOKEN'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This forces the token to always be base64 encoded. the previous version tried to figure out if it's base64 or not. but yes, it was not perfect in any way...

This might break existing use cases for people. :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's impossible to detect base64 reliably. Either we should allow only plain or only base64. And some tokens may contain characters that are unsuitable for env / config files, so it might be better to just allow base64.

If the base64 decoding fails, it gives a fairly clear error message:

          raise ArgumentError, 'KUBE_TOKEN does not seem to be base64 encoded'

rescue ArgumentError
raise ArgumentError, 'KUBE_TOKEN does not seem to be base64 encoded'
end

configuration = K8s::Config.build(server: ENV['KUBE_SERVER'], ca: ENV['KUBE_CA'], auth_token: token)
elsif !ENV['KUBECONFIG'].to_s.empty?
configuration = K8s::Config.from_kubeconfig_env(ENV['KUBECONFIG'])
elsif File.exist?(File.join(Dir.home, '.kube', 'config'))
Expand Down
26 changes: 11 additions & 15 deletions lib/k8s/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,19 +103,18 @@ class NamedContext < ConfigStruct
# @param path [String]
# @return [K8s::Config]
def self.load_file(path)
new(YAML.safe_load(File.read(path), [Time, DateTime, Date]))
new(YAML.safe_load(File.read(File.expand_path(path)), [Time, DateTime, Date], [], true))
end

# Loads configuration files listed in KUBE_CONFIG environment variable and
# merged using the configuration merge rules, @see K8s::Config.merge
# merge using the configuration merge rules, @see K8s::Config.merge
#
# @param kubeconfig [String] by default read from ENV['KUBECONFIG']
def self.from_kubeconfig_env(kubeconfig = nil)
kubeconfig ||= ENV.fetch('KUBECONFIG', '')
return if kubeconfig.empty?
raise ArgumentError, "KUBECONFIG not set" if kubeconfig.empty?

paths = kubeconfig.split(/(?!\\):/)
return load_file(paths.first) if paths.size == 1

paths.inject(load_file(paths.shift)) do |memo, other_cfg|
memo.merge(load_file(other_cfg))
Expand All @@ -125,23 +124,17 @@ def self.from_kubeconfig_env(kubeconfig = nil)
# Build a minimal configuration from at least a server address, server certificate authority data and an access token.
#
# @param server [String] kubernetes server address
# @param ca [String] server certificate authority data
# @param token [String] access token (optionally base64 encoded)
# @param ca [String] server certificate authority data (base64 encoded)
# @param token [String] access token
# @param cluster_name [String] cluster name
# @param user [String] user name
# @param context [String] context name
# @param options [Hash] (see #initialize)
def self.build(server:, ca:, auth_token:, cluster_name: 'kubernetes', user: 'k8s-client', context: 'k8s-client', **options)
begin
decoded_token = Base64.strict_decode64(auth_token)
rescue ArgumentError
decoded_token = nil
end

new(
{
clusters: [{ name: cluster_name, cluster: { server: server, certificate_authority_data: ca } }],
users: [{ name: user, user: { token: decoded_token || auth_token } }],
users: [{ name: user, user: { token: auth_token } }],
contexts: [{ name: context, context: { cluster: cluster_name, user: user } }],
current_context: context
}.merge(options)
Expand Down Expand Up @@ -183,10 +176,13 @@ def merge(other)
end

# @param name [String]
# TODO: raise error if not found
# @raise [K8s::Error::Configuration]
# @return [K8s::Config::Context]
def context(name = current_context)
contexts.find{ |context| context.name == name }.context
found = contexts.find{ |context| context.name == name }
raise K8s::Error::Configuration, "context not found: #{name.inspect}" unless found

found.context
end

# @param name [String]
Expand Down
143 changes: 143 additions & 0 deletions spec/k8s/client_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,149 @@
RSpec.describe K8s::Client do
include FixtureHelpers

describe '#autoconfig' do
let(:ca_b64) { "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUN5RENDQWJDZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFWTVJNd0VRWURWUVFERXdwcmRXSmwKY201bGRHVnpNQjRYRFRFNE1USXhPVEE1TXpJek5Wb1hEVEk0TVRJeE5qQTVNekl6TlZvd0ZURVRNQkVHQTFVRQpBeE1LYTNWaVpYSnVaWFJsY3pDQ0FTSXdEUVlKS29aSWh2Y05BUUVCQlFBRGdnRVBBRENDQVFvQ2dnRUJBTUpsCnBHOVZSamFKMEtWTDM5UXdTemNPRmluZjNGb0MwRkh2bG1HUTVIQ202Q2RZT0VyYXFhTlhIaHZ6UjAyT1lreWoKTmFZK2N0NFl6MEc3TlBmQWlWRFNhRW9zdEZZLzVzdXMwOGJKc0g3Z1E4TkZMMk5qcCtlSWtVTktuNkltcldENQpxaFZVVkV6Q2tCSEsvRjJ1N3hoaTV1M1lvUGJOMjJtZHptZ011TjRGMVF4QnlDOU1ML01kWm4wdDdObTNSeVRaCkJndExqQ05Ea0VaRUFRbm81VmtJMVlNQUdwRmZZcUhXbURRczRabjRzdTJlay95bjhhZkk3RG9yN242MEg3aWkKNC84Y0JWT1RZOTRieElWaW1ETHdjNjkvTXJZN09vTmw0RUpwTEVYdTVzMzFxbE80UkFUYjI4VkNOelBUWWd5ZwpUeWt4T2dIeTl4ZXRnOUkrVzU4Q0F3RUFBYU1qTUNFd0RnWURWUjBQQVFIL0JBUURBZ0trTUE4R0ExVWRFd0VCCi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQkFCUFBiQzhDRzZiaXZ5OGJzcmtiUENtRjBTREMKdjRJbm5QZkZWTXRnZzB2aFh3d1BhaHNyVUNHRmw1d2wwVitTQS96TDkzTmhuTEFuTnFPWFNtaHJZTWZ2NU5GaApwUFlsaTlpMVNWZk1Pc3BkbGxhWEEvV1ZMenZqcEt5S3FtdVNGU3hzRzJuTVpackJ0MitzUDAvUW1MWDVnUS8yCk5sUFZ3a2E5alRoVjBIVUN5a3ptNy9GYU1GbjZUd0xKWWszWWFQN1o1QVQ5c01wYTIyRzBWWEhQVllHVWQ1K1QKWTNBUStjTEo2bjBsQnVJSmsydzFsUzdjK3ZsN2dLczYrVm9LajRFM2lQdDkwQy9lR3ZMaStZcGU5bGI0dm5ZYgpmMXNWL0h0RjkwMDF0USthQ044RnpGelhQNGV1aHlZYW0ycGt6M3V5bmVHT21wKzI1UHRMMGc2RGRQOD0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=" }

let(:kubeconfig) do
<<~EOB
---
apiVersion: v1
clusters:
- cluster:
certificate-authority-data: #{ca_b64}
server: https://192.168.100.100:6443
name: kubernetes
contexts:
- context:
cluster: kubernetes
user: kubernetes-admin
name: kubernetes-admin@kubernetes
current-context: kubernetes-admin@kubernetes
kind: Config
preferences: {}
users:
- name: kubernetes-admin
user:
client-certificate-data: #{ca_b64}
client-key-data: #{ca_b64}
EOB
end

let(:default_kubeconfig_path) { File.join(Dir.home, '.kube', 'config') }


subject { described_class }
context 'from KUBE_CA/KUBE_SERVER/KUBE_TOKEN variables' do

let(:server) { "localhost" }
let(:env) { { 'KUBE_TOKEN' => token, 'KUBE_CA' => ca, 'KUBE_SERVER' => server } }

before do
stub_const("ENV", env)
end

context 'KUBE_TOKEN is not base64 encoded' do
let(:ca) { ca_b64 }
let(:token) { 'foobar' }

it 'raises' do
expect{subject.autoconfig}.to raise_error(ArgumentError, /KUBE_TOKEN.*base64/)
end
end

context 'KUBE_CA is not base64 encoded' do
let(:ca) { Base64.decode64(ca_b64) }
let(:token) { Base64.encode64('foobar').chomp }

it 'raises' do
expect{subject.autoconfig}.to raise_error(ArgumentError, /KUBE_CA.*base64/)
end
end

context 'both KUBE_CA and KUBE_TOKEN are base64 encoded' do
let(:ca) { ca_b64 }
let(:token) { Base64.encode64('foobar').chomp }

it 'returns a correctly configured client' do
client = subject.autoconfig
expect(client).to be_a K8s::Client
expect(client.transport.instance_variable_get(:@auth_token)).to eq 'foobar'
end
end
end

context 'from KUBECONFIG variable' do
let(:env) { { 'KUBECONFIG' => kubeconfig_string } }

before do
stub_const("ENV", env)
end

context 'KUBECONFIG contains a single path' do
let(:kubeconfig_string) { '~/.kube/config' }

it 'loads the file and returns a client' do
expect(File).to receive(:read).with(File.join(Dir.home, '.kube', 'config')).and_return(kubeconfig)
expect(subject.autoconfig).to be_a K8s::Client
end
end

context 'KUBECONFIG contains a multiple paths' do
let(:kubeconfig_string) { '~/.kube/config:~/.kube/config2' }
let(:kubeconfig2) do
<<~EOB
---
apiVersion: v1
clusters:
- cluster:
certificate-authority-data: #{ca_b64}
server: https://192.168.100.100:6443
name: kube2
contexts:
- context:
cluster: kube2
user: kubernetes-admin
name: kubernetes-admin@kube2
current-context: kubernetes-admin@kube2
kind: Config
preferences: {}
users:
- name: kubernetes-admin
user:
client-certificate-data: #{ca_b64}
client-key-data: #{ca_b64}
EOB
end

it 'loads all of the files, merges them and returns a client' do
expect(File).to receive(:read).with(File.join(Dir.home, '.kube', 'config')).and_return(kubeconfig)
expect(File).to receive(:read).with(File.join(Dir.home, '.kube', 'config2')).and_return(kubeconfig2)
expect(subject.autoconfig).to be_a K8s::Client
end
end
end

context 'from default ~/.kube/config' do
it 'loads the file' do
expect(File).to receive(:exist?).with(default_kubeconfig_path).and_return(true)
expect(File).to receive(:read).with(default_kubeconfig_path).and_return(kubeconfig)
expect(subject.autoconfig).to be_a K8s::Client
end
end

context 'from in_cluster_config' do
before do
allow(File).to receive(:exist?).with(default_kubeconfig_path).and_return(false)
stub_const("ENV", {})
end

it 'resorts to in_cluster_config finally' do
expect(subject).to receive(:in_cluster_config)
subject.autoconfig
end
end
end

context "for a mocked API" do
let(:transport) { K8s::Transport.new('http://localhost:8080') }

Expand Down
6 changes: 3 additions & 3 deletions spec/k8s/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@
describe '#self.from_kubeconfig_env' do
context 'KUBECONFIG points to a single file' do
it 'reads the file' do
expect(File).to receive(:read).with('kubeconfig_path').and_return(YAML.dump('current_context' => 'foo'))
expect(File).to receive(:read).with(File.expand_path('kubeconfig_path')).and_return(YAML.dump('current_context' => 'foo'))
described_class.from_kubeconfig_env('kubeconfig_path')
end
end

context 'KUBECONFIG points to two files' do
it 'reads all of the files' do
expect(File).to receive(:read).with('kubeconfig_path').and_return(YAML.dump('current_context' => 'foo'))
expect(File).to receive(:read).with('kubeconfig2_path').and_return(YAML.dump('current_context' => 'should not overwrite 1'))
expect(File).to receive(:read).with(File.expand_path('kubeconfig_path')).and_return(YAML.dump('current_context' => 'foo'))
expect(File).to receive(:read).with(File.expand_path('kubeconfig2_path')).and_return(YAML.dump('current_context' => 'should not overwrite 1'))
expect(described_class.from_kubeconfig_env('kubeconfig_path:kubeconfig2_path').current_context).to eq 'foo'
end
end
Expand Down