kubectl --kubeconfig cannot merge multiple files

CHANGELOG.md

@@ -1,7 +1,10 @@
 ## next
 
+## 0.26.3
+
 *Bug fixes*
-- Fixes a bug introduced in 0.26.2 where kubernetes-render started adding YAML headers to empty render results
+- Fixes a bug introduced in 0.26.0 where listing multiple files in the $KUBECONFIG environment variable would throw an error ([#468](https://github.com/Shopify/kubernetes-deploy/pull/468))
+- Fixes a bug introduced in 0.26.2 where kubernetes-render started adding YAML headers to empty render results ([#467](https://github.com/Shopify/kubernetes-deploy/pull/467))
 
 ## 0.26.2
 
@@ -10,7 +13,7 @@
 through a yaml parser. ([#454](https://github.com/Shopify/kubernetes-deploy/pull/454))
 
 *Bug fixes*
-- Remove use of deprecated feature preventing use with Kuberentes 1.14 ([#460](https://github.com/Shopify/kubernetes-deploy/pull/460))
+- Remove use of deprecated feature preventing use with Kubernetes 1.14 ([#460](https://github.com/Shopify/kubernetes-deploy/pull/460))
 
 ## 0.26.1
 

lib/kubernetes-deploy/kubeclient_builder.rb

@@ -7,18 +7,14 @@ class KubeclientBuilder
     class ContextMissingError < FatalDeploymentError
       def initialize(context_name, kubeconfig)
         super("`#{context_name}` context must be configured in your " \
-          "KUBECONFIG file(s) (#{kubeconfig}).")
+          "KUBECONFIG file(s) (#{kubeconfig.join(', ')}).")
       end
     end
 
-    def self.kubeconfig
-      new.kubeconfig
-    end
-
-    attr_reader :kubeconfig
-
     def initialize(kubeconfig: ENV["KUBECONFIG"])
-      @kubeconfig = kubeconfig || "#{Dir.home}/.kube/config"
+      files = kubeconfig || "#{Dir.home}/.kube/config"
+      # Split the list by colon for Linux and Mac, and semicolon for Windows.
+      @kubeconfig_files = files.split(/[:;]/).map!(&:strip).reject(&:empty?)
     end
 
     def build_v1_kubeclient(context)
@@ -100,33 +96,32 @@ def build_networking_v1_kubeclient(context)
       )
     end
 
-    def config_files
-      # Split the list by colon for Linux and Mac, and semicolon for Windows.
-      kubeconfig.split(/[:;]/).map!(&:strip).reject(&:empty?)
-    end
-
     def validate_config_files
       errors = []
-      if config_files.empty?
+      if @kubeconfig_files.empty?
         errors << "Kube config file name(s) not set in $KUBECONFIG"
       else
-        config_files.each do |f|
-          unless File.file?(f)
-            errors << "Kube config not found at #{f}"
-          end
+        @kubeconfig_files.each do |f|
+          # If any files in the list are not valid, we can't be sure the merged context list is what the user intended
+          errors << "Kube config not found at #{f}" unless File.file?(f)
         end
       end
       errors
     end
 
+    def validate_config_files!
+      errors = validate_config_files
+      raise TaskConfigurationError, errors.join(', ') if errors.present?
+    end
+
     private
 
     def build_kubeclient(api_version:, context:, endpoint_path: nil)
+      validate_config_files!
+      @kubeclient_configs ||= @kubeconfig_files.map { |f| KubeConfig.read(f) }
       # Find a context defined in kube conf files that matches the input context by name
-      configs = config_files.map { |f| KubeConfig.read(f) }
-      config = configs.find { |c| c.contexts.include?(context) }
-
-      raise ContextMissingError.new(context, kubeconfig) unless config
+      config = @kubeclient_configs.find { |c| c.contexts.include?(context) }
+      raise ContextMissingError.new(context, @kubeconfig_files) unless config
 
       kube_context = config.context(context)
       client = Kubeclient::Client.new(

lib/kubernetes-deploy/kubectl.rb

@@ -9,7 +9,6 @@ class ResourceNotFoundError < StandardError; end
 
     def initialize(namespace:, context:, logger:, log_failure_by_default:, default_timeout: DEFAULT_TIMEOUT,
       output_is_sensitive_default: false)
-      @kubeconfig = KubeclientBuilder.kubeconfig
       @namespace = namespace
       @context = context
       @logger = logger
@@ -27,7 +26,6 @@ def run(*args, log_failure: nil, use_context: true, use_namespace: true, output:
       output_is_sensitive = @output_is_sensitive_default if output_is_sensitive.nil?
 
       args = args.unshift("kubectl")
-      args.push("--kubeconfig=#{@kubeconfig}")
       args.push("--namespace=#{@namespace}") if use_namespace
       args.push("--context=#{@context}")     if use_context
       args.push("--output=#{output}") if output

lib/kubernetes-deploy/version.rb

@@ -1,4 +1,4 @@
 # frozen_string_literal: true
 module KubernetesDeploy
-  VERSION = "0.26.2"
+  VERSION = "0.26.3"
 end

test/integration-serial/serial_deploy_test.rb

@@ -66,6 +66,38 @@ def test_unreachable_context
     end
   end
 
+  def test_multiple_configuration_files
+    old_config = ENV['KUBECONFIG']
+    config_file = File.join(__dir__, '../fixtures/kube-config/unknown_config.yml')
+    ENV['KUBECONFIG'] = config_file
+    result = deploy_fixtures('hello-cloud')
+    assert_deploy_failure(result)
+    assert_logs_match_all([
+      'Result: FAILURE',
+      'Configuration invalid',
+      "Kube config not found at #{config_file}",
+    ], in_order: true)
+    reset_logger
+
+    ENV['KUBECONFIG'] = " : "
+    result = deploy_fixtures('hello-cloud')
+    assert_deploy_failure(result)
+    assert_logs_match_all([
+      'Result: FAILURE',
+      'Configuration invalid',
+      "Kube config file name(s) not set in $KUBECONFIG",
+    ], in_order: true)
+    reset_logger
+
+    default_config = "#{Dir.home}/.kube/config"
+    extra_config = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml')
+    ENV['KUBECONFIG'] = "#{default_config}:#{extra_config}"
+    result = deploy_fixtures('hello-cloud', subset: ["configmap-data.yml"])
+    assert_deploy_success(result)
+  ensure
+    ENV['KUBECONFIG'] = old_config
+  end
+
   def test_cr_merging
     assert_deploy_success(deploy_fixtures("crd", subset: %w(mail.yml)))
     assert_deploy_success(deploy_fixtures("crd", subset: %w(mail_cr.yml)))

test/unit/kubernetes-deploy/kubeclient_builder_test.rb

@@ -5,36 +5,57 @@
 class KubeClientBuilderTest < KubernetesDeploy::TestCase
   def test_config_validation_default_config_file
     builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: nil)
-    assert(builder.validate_config_files, [])
+    assert_equal(builder.validate_config_files, [])
   end
 
   def test_config_validation_missing_file
-    config_file = File.join(__dir__, '../fixtures/kube-config/unknown_config.yml')
+    config_file = File.join(__dir__, '../../fixtures/kube-config/unknown_config.yml')
     builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: config_file)
-    assert(builder.validate_config_files, "Kube config not found at #{config_file}")
+    errors = builder.validate_config_files
+    assert_equal(1, errors.length)
+    assert_equal(errors.first, "Kube config not found at #{config_file}")
   end
 
-  def test_multiple_configuration_files
+  def test_build_runs_config_validation
+    config_file = File.join(__dir__, '../../fixtures/kube-config/unknown_config.yml')
+    kubeclient_builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: config_file)
+
+    expected_err = /Kube config not found at .*unknown_config.yml/
+    assert_raises_message(KubernetesDeploy::TaskConfigurationError, expected_err) do
+      kubeclient_builder.build_v1_kubeclient('test-context')
+    end
+  end
+
+  def test_no_config_files_specified
     builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: " : ")
-    assert(builder.validate_config_files, "Kube config file name(s) not set in $KUBECONFIG")
+    errors = builder.validate_config_files
+    assert_equal(1, errors.length)
+    assert_equal(errors.first, "Kube config file name(s) not set in $KUBECONFIG")
 
+    builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: "")
+    errors = builder.validate_config_files
+    assert_equal(1, errors.length)
+    assert_equal(errors.first, "Kube config file name(s) not set in $KUBECONFIG")
+  end
+
+  def test_multiple_valid_configuration_files
     default_config = "#{Dir.home}/.kube/config"
-    extra_config = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml')
+    extra_config = File.join(__dir__, '../../fixtures/kube-config/dummy_config.yml')
     builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: "#{default_config}:#{extra_config}")
-    assert(builder.validate_config_files, [])
+    assert_empty(builder.validate_config_files)
   end
 
   def test_build_client_from_multiple_config_files
     Kubeclient::Client.any_instance.stubs(:discover)
-    # Set KUBECONFIG to include multiple config files
+    default_config = "#{Dir.home}/.kube/config"
     dummy_config = File.join(__dir__, '../../fixtures/kube-config/dummy_config.yml')
-    kubeconfig = "#{ENV['KUBECONFIG']}:#{dummy_config}"
-    kubeclient_builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: kubeconfig)
+
+    kubeclient_builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: "#{default_config}:#{dummy_config}")
     # Build kubeclient for an unknown context fails
     context_name = "unknown_context"
     assert_raises_message(KubernetesDeploy::KubeclientBuilder::ContextMissingError,
       "`#{context_name}` context must be configured in your KUBECONFIG file(s) " \
-      "(#{kubeconfig}).") do
+      "(#{default_config}, #{dummy_config})") do
       kubeclient_builder.build_v1_kubeclient(context_name)
     end
     # Build kubeclient for a context present in the dummy config succeeds