Skip to content

Fix issues with certificate based auth #42

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.

Sign up for GitHub

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

Merged
k8s-ci-robot merged 3 commits into from
Apr 7, 2019
+12 −9
Merged

Fix issues with certificate based auth #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64a48d0
Require 'kubernetes/utils'
DavidHuie Apr 2, 2019
b19e4f3
Save Tempfile instance in cache
DavidHuie Apr 2, 2019
7205308
Fix test to check for cached paths
DavidHuie Apr 5, 2019
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
10 changes: 5 additions & 5 deletions kubernetes/spec/utils_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@
end

context '#create_temp_file_with_base64content' do
before do
Kubernetes.clear_temp_files
end

context 'when it is called at first time' do
it 'should return temp file path' do
expected_path = 'tempfile-path'
Expand All @@ -102,12 +106,8 @@

context 'when it is already called' do
it 'should return cached value' do
expected_path = 'tempfile-path'
content = TEST_DATA_BASE64
Kubernetes.cache_temp_file(content, expected_path)
io = double('io')
expect(io).not_to receive(:path)
expect(io).not_to receive(:write).with(TEST_DATA)
expected_path = Kubernetes.create_temp_file_with_base64content(content)

path = Kubernetes.create_temp_file_with_base64content(content)
expect(path).to eq(expected_path)
Expand Down
1 change: 1 addition & 0 deletions kubernetes/src/kubernetes/config/kube_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
require 'kubernetes/api_client'
require 'kubernetes/configuration'
require 'kubernetes/config/error'
require 'kubernetes/utils'
Copy link
Contributor

@drubin drubin Apr 5, 2019

Choose a reason for hiding this comment

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

Does this mean we were missing a test that would have highlighted this?

Copy link
Contributor Author

@DavidHuie DavidHuie Apr 5, 2019

Choose a reason for hiding this comment

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

Yeah, although I think getting the import situation completely fixed is a bit out of the scope of this PR. IMO, all require calls should just be made within kubernetes/lib/kubernetes.rb so things like this aren't missed.


module Kubernetes
# The KubeConfig class represents configuration based on a YAML
Expand Down
10 changes: 6 additions & 4 deletions kubernetes/src/kubernetes/utils.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,17 @@ def create_temp_file_and_set(obj, key)
def create_temp_file_with_base64content(content)
@temp_files[content] ||= Tempfile.open('kube') do |temp|
temp.write(Base64.strict_decode64(content))
temp.path
temp
end

@temp_files[content].path
end

def cache_temp_file(content, path)
@temp_files[content] = path
def clear_temp_files
@temp_files = {}
end

module_function :new_client_from_config, :load_incluster_config,
:load_kube_config, :create_temp_file_and_set,
:create_temp_file_with_base64content, :cache_temp_file
:create_temp_file_with_base64content, :clear_temp_files
end