From ddef6113d4a30a090290b2106c50ae772ecf4beb Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 22 Aug 2026 21:41:58 -0700 Subject: [PATCH] docs: reach 100% YARD coverage and add doc rake tasks kitchen-cloudstack was at 65% under --private: 22 of 44 methods, four constants, and the Kitchen::Driver namespace were undocumented, and there was no way to generate or measure docs. Document the rest and add .yardopts plus `rake doc` / `rake doc_coverage`. create, destroy, and status carried `# (see Base#create)` reference directives, which YARD does not count as documentation and which told a reader nothing about what this driver does differently -- destroy tearing down the public address before the instance, status mapping CloudStack's own state names. They now have real docstrings and the stranded directives are gone. The four undocumented constants were JOB_SUCCEEDED, JOB_FAILED, and both DEFAULT_* timeouts. They sat under a comment attached only to JOB_RUNNING, so each now carries its own. Comments only. Verified with 63 examples, 0 failures and cookstyle clean. Co-Authored-By: Claude Opus 5 (1M context) --- .yardopts | 11 ++++++ Gemfile | 4 +++ Rakefile | 18 ++++++++++ lib/kitchen/driver/cloudstack.rb | 36 +++++++++++++++++-- lib/kitchen/driver/cloudstack/client.rb | 20 ++++++++++- lib/kitchen/driver/cloudstack/credentials.rb | 29 +++++++++++++++ lib/kitchen/driver/cloudstack/networking.rb | 17 +++++++++ .../driver/cloudstack/server_options.rb | 15 ++++++++ 8 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 .yardopts diff --git a/.yardopts b/.yardopts new file mode 100644 index 0000000..e498dc9 --- /dev/null +++ b/.yardopts @@ -0,0 +1,11 @@ +--markup markdown +--output-dir doc +--readme README.md +--title "kitchen-cloudstack" +--protected +--private +lib/**/*.rb +- +CHANGELOG.md +CONTRIBUTING.md +LICENSE diff --git a/Gemfile b/Gemfile index 734a642..e37d12e 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,10 @@ source "https://rubygems.org" gemspec development_group: :test +group :docs do + gem "yard" +end + group :cookstyle do gem "cookstyle" end diff --git a/Rakefile b/Rakefile index 7cf49ae..9b7bd7b 100644 --- a/Rakefile +++ b/Rakefile @@ -16,3 +16,21 @@ rescue LoadError end task default: %i{test} + +begin + require "yard" + + # Options and the file list live in .yardopts so that a bare `yard` from the + # command line produces exactly what `rake doc` does. + YARD::Rake::YardocTask.new(:doc) + + desc "List anything in lib/ that is still undocumented" + task :doc_coverage do + sh "yard stats --list-undoc" + end +rescue LoadError + desc "Generate YARD documentation (not installed)" + task :doc do + abort "YARD is not installed. Run: bundle install" + end +end diff --git a/lib/kitchen/driver/cloudstack.rb b/lib/kitchen/driver/cloudstack.rb index 20b602b..6040887 100644 --- a/lib/kitchen/driver/cloudstack.rb +++ b/lib/kitchen/driver/cloudstack.rb @@ -26,6 +26,7 @@ require_relative "cloudstack/server_options" module Kitchen + # Test Kitchen's driver plugins. module Driver # Test Kitchen driver for Apache CloudStack and Citrix CloudPlatform. # @@ -55,7 +56,11 @@ class Cloudstack < Kitchen::Driver::Base username port password ssh_key }.freeze - # (see Base#create) + # Deploys the instance and waits until it can be logged into. + # + # @param state [Hash] mutable instance state; gains +server_id+, + # +hostname+, and whichever credential keys apply + # @return [void] def create(state) super disable_ssl_validation! if config[:disable_ssl_validation] @@ -69,7 +74,14 @@ def create(state) instance.transport.connection(state).wait_until_ready end - # (see Base#destroy) + # Destroys the instance and releases anything allocated alongside it. + # + # Public addresses, port forwards, and firewall rules are torn down + # first, then every key this driver owns is cleared from state so a + # destroyed instance leaves no password behind in the state file. + # + # @param state [Hash] instance state naming the instance + # @return [void] def destroy(state) return unless state[:server_id] @@ -84,7 +96,11 @@ def destroy(state) STATE_KEYS.each { |key| state.delete(key) } end - # (see Base#status) + # Reports what CloudStack currently thinks of the instance. + # + # @param state [Hash] instance state naming the instance + # @return [Hash] a Test Kitchen status hash, or the base implementation's + # answer when there is no instance or CloudStack does not know it def status(state) return super unless state[:server_id] @@ -170,6 +186,11 @@ def wait_for_guest_password_sync sleep(sync_time) end + # Asks CloudStack for one instance's current state. + # + # @param server_id [String] the instance's CloudStack id + # @return [String, nil] e.g. +"Running"+, or nil when CloudStack returns + # no matching machine def lookup_instance_state(server_id) response = client.compute.list_virtual_machines("id" => server_id) machines = response.fetch("listvirtualmachinesresponse", {})["virtualmachine"] @@ -178,6 +199,9 @@ def lookup_instance_state(server_id) machines.first["state"] end + # Helper that owns the optional public address and its firewall rules. + # + # @return [Networking] def networking @networking ||= Networking.new( config, client: client, port: transport_port, logger: logger @@ -190,6 +214,12 @@ def transport_port instance.transport[:port] end + # Turns off TLS certificate verification for every Excon request. + # + # This is process-wide, not scoped to this driver, which is why it is + # only done when +disable_ssl_validation+ is explicitly set. + # + # @return [void] def disable_ssl_validation! require "excon" unless defined?(Excon) Excon.defaults[:ssl_verify_peer] = false diff --git a/lib/kitchen/driver/cloudstack/client.rb b/lib/kitchen/driver/cloudstack/client.rb index 990de01..a776ff7 100644 --- a/lib/kitchen/driver/cloudstack/client.rb +++ b/lib/kitchen/driver/cloudstack/client.rb @@ -25,12 +25,22 @@ class Cloudstack < Kitchen::Driver::Base # rather than a result, so callers use {#run_job} to turn that job id # into the eventual result, or into an ActionFailed. class Client - # Values CloudStack reports in an async job's "jobstatus" field. + # Value CloudStack reports in an async job's "jobstatus" field while + # the job is still running. JOB_RUNNING = 0 + + # Value CloudStack reports in "jobstatus" once the job has succeeded. JOB_SUCCEEDED = 1 + + # Value CloudStack reports in "jobstatus" once the job has failed. JOB_FAILED = 2 + # Seconds between polls of an async job, when +cloudstack_job_poll_interval+ + # is not configured. DEFAULT_POLL_INTERVAL = 10 + + # Seconds to wait for an async job, when +cloudstack_job_timeout+ is + # not configured. DEFAULT_TIMEOUT = 600 def initialize(config, compute: nil, sleeper: nil) @@ -39,6 +49,12 @@ def initialize(config, compute: nil, sleeper: nil) @sleeper = sleeper || ->(seconds) { sleep(seconds) } end + # The fog CloudStack connection, built from the configured endpoint. + # + # The API URL is split into scheme, host, port, and path because fog + # wants them separately rather than as one URL. + # + # @return [Fog::Compute] a CloudStack compute connection def compute @compute ||= begin uri = URI.parse(config[:cloudstack_api_url]) @@ -96,10 +112,12 @@ def run_response_job(response, key) attr_reader :config, :sleeper + # @return [Integer] seconds between polls of a running job def poll_interval config[:cloudstack_job_poll_interval] || DEFAULT_POLL_INTERVAL end + # @return [Integer] seconds to wait before giving up on a job def timeout config[:cloudstack_job_timeout] || DEFAULT_TIMEOUT end diff --git a/lib/kitchen/driver/cloudstack/credentials.rb b/lib/kitchen/driver/cloudstack/credentials.rb index e64e538..e53659c 100644 --- a/lib/kitchen/driver/cloudstack/credentials.rb +++ b/lib/kitchen/driver/cloudstack/credentials.rb @@ -53,6 +53,14 @@ def to_state(server_info) attr_reader :config, :home, :working_dir + # Picks the single credential the transport should use. + # + # Sources are tried in {SOURCES} order and the first that resolves + # wins, so a keypair beats a CloudStack-generated password, which in + # turn beats a password from config. + # + # @param server_info [Hash] the "virtualmachine" payload + # @return [Hash] one of +{ssh_key:}+, +{password:}+, or +{}+ def credential_state(server_info) if keypair_path { ssh_key: keypair_path } @@ -65,6 +73,10 @@ def credential_state(server_info) end end + # The password CloudStack generated for a password-enabled template. + # + # @param server_info [Hash] the "virtualmachine" payload + # @return [String, nil] nil unless the template is password-enabled def generated_password(server_info) return nil unless server_info["passwordenabled"] @@ -79,6 +91,12 @@ def keypair_path @keypair_path = find_keypair end + # Locates the local .pem matching the configured keypair name. + # + # A missing file is a warning rather than an error, because a password + # may still get the user in. + # + # @return [String, nil] path to the key, or nil if none was found def find_keypair name = config[:cloudstack_ssh_keypair_name] return nil if name.nil? @@ -96,10 +114,21 @@ def find_keypair path end + # Directories searched for a keypair's .pem, in order. + # + # @return [Array] the configured directory, the working + # directory, the home directory, then ~/.ssh def search_directories [config[:keypair_search_directory], working_dir, home, File.join(home.to_s, ".ssh")].compact end + # Warns when the located .pem is a public key. + # + # Exporting the wrong half of a keypair is a common mistake, and the + # resulting authentication failure is otherwise hard to read. + # + # @param path [String] the key file to inspect + # @return [void] def warn_unless_private_key(path) first_token = File.read(path).split.first return unless PUBLIC_KEY_PREFIXES.include?(first_token) diff --git a/lib/kitchen/driver/cloudstack/networking.rb b/lib/kitchen/driver/cloudstack/networking.rb index 1843a41..691c59c 100644 --- a/lib/kitchen/driver/cloudstack/networking.rb +++ b/lib/kitchen/driver/cloudstack/networking.rb @@ -82,8 +82,13 @@ def teardown(state) attr_reader :config, :client, :port, :logger + # @return [Fog::Compute] the shared CloudStack connection def compute = client.compute + # Opens the transport's port on the allocated public address. + # + # @param state [Hash] mutable instance state; gains +firewall_rule_id+ + # @return [void] def create_firewall_rule(state) response = compute.create_firewall_rule( "projectid" => config[:cloudstack_project_id], @@ -98,6 +103,10 @@ def create_firewall_rule(state) state[:firewall_rule_id] = rule["id"] if rule.is_a?(Hash) end + # Removes the port forwarding rule, tolerating one already gone. + # + # @param state [Hash] instance state naming the rule + # @return [void] def delete_port_forward(state) tolerating_missing("port forwarding rule") do response = compute.delete_port_forwarding_rule(state[:forwardingruleid]) @@ -105,6 +114,10 @@ def delete_port_forward(state) end end + # Removes the firewall rule, tolerating one already gone. + # + # @param state [Hash] instance state naming the rule + # @return [void] def delete_firewall_rule(state) tolerating_missing("firewall rule") do response = compute.delete_firewall_rule(state[:firewall_rule_id]) @@ -112,6 +125,10 @@ def delete_firewall_rule(state) end end + # Disassociates the public address, tolerating one already gone. + # + # @param state [Hash] instance state naming the address + # @return [void] def release_public_ip(state) tolerating_missing("public IP address") do response = compute.disassociate_ip_address(state[:ipaddressid]) diff --git a/lib/kitchen/driver/cloudstack/server_options.rb b/lib/kitchen/driver/cloudstack/server_options.rb index 2299a55..530ac07 100644 --- a/lib/kitchen/driver/cloudstack/server_options.rb +++ b/lib/kitchen/driver/cloudstack/server_options.rb @@ -56,6 +56,12 @@ def initialize(config, instance_name:, login: Etc.getlogin, hostname: Socket.get @hostname = hostname end + # Builds the parameter hash for deployVirtualMachine. + # + # Optional parameters whose config value is nil are dropped, so + # CloudStack applies its own defaults rather than receiving nils. + # + # @return [Hash] parameters ready to pass to the API def to_h params = { "displayname" => display_name } @@ -72,6 +78,9 @@ def to_h params end + # The instance's display name. + # + # @return [String] the configured +server_name+, or a generated one def display_name config[:server_name] || generate_name end @@ -108,6 +117,12 @@ def truncate_to_budget(parts, budget) parts end + # The user data to send, base64 encoded. + # + # Data that is already valid base64 is passed through untouched rather + # than being encoded a second time. + # + # @return [String] base64-encoded user data def userdata data = config[:cloudstack_userdata] data.match(BASE64_PATTERN) ? data : Base64.encode64(data)