From 5aa36f2551d83cc845d020e6950e0273f9a79aba Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Mon, 20 May 2019 17:29:35 -0700 Subject: Better target mode no-creds errors Changes the target mode error in the case of no creds at all to: ``` FATAL: ArgumentError: Credentials file specified for target mode does not exist: '/Users/lamont/.chef/credentials' ``` From: ``` [2019-05-20T17:30:41-07:00] FATAL: NoMethodError: undefined method `[]' for nil:NilClass ``` Signed-off-by: Lamont Granquist --- lib/chef/train_transport.rb | 24 ++++++++++-------------- spec/unit/train_transport_spec.rb | 23 ++++++++++++++--------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/lib/chef/train_transport.rb b/lib/chef/train_transport.rb index 9db5f8fbf3..95b5018960 100644 --- a/lib/chef/train_transport.rb +++ b/lib/chef/train_transport.rb @@ -20,12 +20,12 @@ require "train" class Chef class TrainTransport + extend ChefConfig::Mixin::Credentials + # # Returns a RFC099 credentials profile as a hash # def self.load_credentials(profile) - extend ChefConfig::Mixin::Credentials - # Tomlrb.load_file returns a hash with keys as strings credentials = parse_credentials_file if contains_split_fqdn?(credentials, profile) @@ -74,22 +74,18 @@ class Chef profile = tm_config.host credentials_file = - if tm_config.credentials_file - if File.exists?(tm_config.credentials_file) - tm_config.credentials_file - else - raise ArgumentError, "Credentials file specified for target mode does not exist: '#{tm_config.credentials_file}'" - end - elsif File.exists?(Chef::Config.platform_specific_path("/etc/chef/#{profile}/credentials")) + if tm_config.credentials_file && File.exist?(tm_config.credentials_file) + tm_config.credentials_file + elsif File.exist?(Chef::Config.platform_specific_path("/etc/chef/#{profile}/credentials")) Chef::Config.platform_specific_path("/etc/chef/#{profile}/credentials") else super end - if credentials_file - Chef::Log.debug("Loading credentials file '#{credentials_file}' for target '#{profile}'") - else - Chef::Log.debug("No credentials file found for target '#{profile}'") - end + + raise ArgumentError, "No credentials file found for target '#{profile}'" unless credentials_file + raise ArgumentError, "Credentials file specified for target mode does not exist: '#{credentials_file}'" unless File.exist?(credentials_file) + + Chef::Log.debug("Loading credentials file '#{credentials_file}' for target '#{profile}'") credentials_file end diff --git a/spec/unit/train_transport_spec.rb b/spec/unit/train_transport_spec.rb index b56c7e1104..cc978d0b94 100644 --- a/spec/unit/train_transport_spec.rb +++ b/spec/unit/train_transport_spec.rb @@ -40,6 +40,7 @@ describe Chef::TrainTransport do # [foo.example.org] => {"foo"=>{"example"=>{"org"=>{}}}} # ['foo.example.org'] => {"foo.example.org"=>{}} it "warns if the host has been split by toml" do + allow(Chef::TrainTransport).to receive(:credentials_file_path).and_return("/Users/scotthourglass/.chef/credentials") allow(Chef::TrainTransport).to receive(:parse_credentials_file).and_return({ "foo" => { "example" => { "org" => {} } } }) expect(Chef::Log).to receive(:warn).with(/as a Hash/) expect(Chef::Log).to receive(:warn).with(/Hostnames must be surrounded by single quotes/) @@ -48,32 +49,36 @@ describe Chef::TrainTransport do end describe "credentials_file_path" do + let(:config_cred_file_path) { "/somewhere/credentials" } + let(:host_cred_file_path) { "/etc/chef/foo.example.org/credentials" } context "when a file path is specified by a config" do - let(:cred_file_path) { "/somewhere/credentials" } - before do - tm_config = double("Config Context", host: "foo.example.org", credentials_file: cred_file_path) + tm_config = double("Config Context", host: "foo.example.org", credentials_file: config_cred_file_path) allow(Chef::Config).to receive(:target_mode).and_return(tm_config) end it "returns the path if it exists" do - allow(File).to receive(:exists?).with(cred_file_path).and_return(true) - expect(Chef::TrainTransport.credentials_file_path).to eq(cred_file_path) + allow(File).to receive(:exist?).with(config_cred_file_path).and_return(true) + expect(Chef::TrainTransport.credentials_file_path).to eq(config_cred_file_path) end it "raises an error if it does not exist" do - allow(File).to receive(:exists?).with(cred_file_path).and_return(false) + allow(File).to receive(:exist?).and_return(false) expect { Chef::TrainTransport.credentials_file_path }.to raise_error(ArgumentError, /does not exist/) end end + it "raises an error if the default creds files do not exist" do + allow(File).to receive(:exist?).and_return(false) + expect { Chef::TrainTransport.credentials_file_path }.to raise_error(ArgumentError, /does not exist/) + end + it "returns the path to the default config file if it exists" do - cred_file_path = Chef::Config.platform_specific_path("/etc/chef/foo.example.org/credentials") tm_config = double("Config Context", host: "foo.example.org", credentials_file: nil) allow(Chef::Config).to receive(:target_mode).and_return(tm_config) - allow(File).to receive(:exists?).with(cred_file_path).and_return(true) - expect(Chef::TrainTransport.credentials_file_path).to eq(cred_file_path) + allow(File).to receive(:exist?).with(host_cred_file_path).and_return(true) + expect(Chef::TrainTransport.credentials_file_path).to eq(host_cred_file_path) end end end -- cgit v1.2.1