diff --git a/lib/kitchen/driver/cloudstack.rb b/lib/kitchen/driver/cloudstack.rb index 73d3ebb..d9aed77 100644 --- a/lib/kitchen/driver/cloudstack.rb +++ b/lib/kitchen/driver/cloudstack.rb @@ -18,6 +18,7 @@ require "kitchen" require "kitchen/driver/base" require "time" unless defined?(Time.zone_offset) +require "uri" unless defined?(URI) require_relative "cloudstack_version" require_relative "cloudstack/client" @@ -117,6 +118,23 @@ def status(state) } end + # Checks the configuration for the mistakes that otherwise surface only + # once +create+ is already talking to CloudStack. + # + # This driver declares no +required_config+, so a missing endpoint or + # credential is not caught at finalize time the way it is in most + # drivers; it fails part way through a deploy instead. These checks move + # that discovery to +kitchen doctor+. + # + # @param state [Hash] mutable instance and driver state + # @return [Boolean] true when a problem was reported + def doctor(state) # rubocop:disable Lint/UnusedMethodArgument + problems = missing_settings_problems + endpoint_problems + + problems.each { |problem| warn(problem) } + !problems.empty? + end + # The CloudStack API connection, exposed so it can be substituted. # # @return [Client] @@ -126,6 +144,47 @@ def client private + # Settings the driver cannot deploy without, with the reason each one + # matters, reported together so one `kitchen doctor` names them all + # rather than one per run. + # + # @return [Array] one problem per unset setting + def missing_settings_problems + { + cloudstack_api_url: "the API endpoint to talk to", + cloudstack_api_key: "the API key to authenticate with", + cloudstack_secret_key: "the secret key to sign requests with", + cloudstack_template_id: "the template to build the instance from", + cloudstack_serviceoffering_id: "the service offering to size it with", + cloudstack_zone_id: "the zone to build it in", + }.filter_map do |key, why| + "#{key} is not set: CloudStack needs #{why}." if config[key].to_s.empty? + end + end + + # Confirms the endpoint parses and that CloudStack accepts the keys. + # + # Skipped when the endpoint or credentials are missing, because + # {#missing_settings_problems} has already said so and a second message + # about a failed connection would just be noise. + # + # @return [Array] a connectivity problem, or an empty array + def endpoint_problems + return [] if %i{cloudstack_api_url cloudstack_api_key cloudstack_secret_key} + .any? { |key| config[key].to_s.empty? } + + uri = URI.parse(config[:cloudstack_api_url]) + return ["cloudstack_api_url (#{config[:cloudstack_api_url]}) has no host."] if uri.host.nil? + + client.compute.list_zones + [] + rescue URI::InvalidURIError => e + ["cloudstack_api_url (#{config[:cloudstack_api_url]}) is not a URL: #{e.message}"] + rescue ::StandardError => e + ["CloudStack rejected the configured credentials at " \ + "#{config[:cloudstack_api_url]}: #{e.message}"] + end + # Deploys the instance and waits for CloudStack to finish building it. # # @param state [Hash] mutable instance state; gains +server_id+ diff --git a/spec/kitchen/driver/cloudstack_spec.rb b/spec/kitchen/driver/cloudstack_spec.rb index 9fdbcbc..217a0c7 100644 --- a/spec/kitchen/driver/cloudstack_spec.rb +++ b/spec/kitchen/driver/cloudstack_spec.rb @@ -204,6 +204,81 @@ def build_driver(config = {}) end end + describe "#doctor" do + # doctor reports through warn; capture the messages rather than the log + # format so the assertions are about what it found, not how it printed it. + def doctor_run(config = {}) + driver = build_driver(config) + messages = [] + allow(driver).to receive(:warn) { |m| messages << m } + [driver.doctor({}), messages] + end + + it "reports the credentials the base config leaves unset" do + found, messages = doctor_run + + expect(found).to be(true) + expect(messages.join("\n")).to include("cloudstack_api_key is not set") + expect(messages.join("\n")).to include("cloudstack_secret_key is not set") + end + + it "names every missing setting in one run rather than one per run" do + driver = described_class.new({}) + messages = [] + allow(driver).to receive(:warn) { |m| messages << m } + + driver.doctor({}) + + %w{cloudstack_api_url cloudstack_api_key cloudstack_secret_key + cloudstack_template_id cloudstack_serviceoffering_id + cloudstack_zone_id}.each do |key| + expect(messages.join("\n")).to include("#{key} is not set") + end + end + + it "passes when everything is set and CloudStack answers" do + found, messages = doctor_run( + cloudstack_api_key: "key", cloudstack_secret_key: "secret" + ) + + expect(found).to be(false) + expect(messages).to be_empty + end + + it "reports an api url that is not a URL" do + found, messages = doctor_run( + cloudstack_api_url: "not a url", cloudstack_api_key: "key", + cloudstack_secret_key: "secret" + ) + + expect(found).to be(true) + expect(messages.join("\n")).to match(/is not a URL|has no host/) + end + + it "reports credentials CloudStack rejects" do + driver = build_driver( + cloudstack_api_key: "key", cloudstack_secret_key: "secret" + ) + allow(driver.client.compute).to receive(:list_zones) + .and_raise(StandardError.new("401 unauthorized")) + messages = [] + allow(driver).to receive(:warn) { |m| messages << m } + + expect(driver.doctor({})).to be(true) + expect(messages.join("\n")).to include("rejected the configured credentials") + end + + it "stays quiet about connectivity when the endpoint is missing" do + driver = described_class.new({}) + messages = [] + allow(driver).to receive(:warn) { |m| messages << m } + + driver.doctor({}) + + expect(messages.join("\n")).not_to include("rejected the configured credentials") + end + end + describe "transport awareness" do context "with a WinRM transport" do let(:transport) { Kitchen::Transport::Winrm.new }