feat: replace fog-cloudstack with cloudstack_client

CONTRIBUTING.md

@@ -18,9 +18,9 @@ most useful thing you can do for this project.
 
 Other changes that would be welcome:
 
-- Wrapping CloudStack and Excon errors in `Kitchen::ActionFailed`, so that bad
-  credentials produce a readable message rather than a `Fog::JSON::DecodeError`
-  and a stack trace.
+- Wrapping CloudStack API errors in `Kitchen::ActionFailed`, so that bad
+  credentials produce a readable message rather than a
+  `CloudstackClient::ApiError` and a stack trace.
 - Making `create` idempotent, so that running it against an instance that
   already exists in state does not deploy a second one.
 - Looking up templates, service offerings, zones and networks by name rather
@@ -72,10 +72,12 @@ Specs live under `spec/`:
 - `spec/kitchen/driver/cloudstack/` covers each supporting class in isolation.
   `ServerOptions` and `Credentials` are plain objects and are tested directly.
 - `spec/integration/lifecycle_spec.rb` runs a full create/status/destroy cycle
-  through real Test Kitchen and real fog, stubbing only the HTTP layer, so
-  request signing, response parsing and plugin wiring are all exercised.
+  through real Test Kitchen and a real `cloudstack_client`, stubbing only the
+  HTTP layer, so request signing, response parsing and plugin wiring are all
+  exercised.
 
-Unit specs inject a fake client rather than stubbing `Fog::Compute` globally.
+Unit specs inject a fake client rather than stubbing `CloudstackClient::Client`
+globally.
 If you add behaviour that talks to CloudStack, prefer the same approach: it
 keeps the tests fast and makes it obvious which API calls a change actually
 makes.

Gemfile

@@ -1,6 +1,14 @@
 source "https://rubygems.org"
 
 gemspec development_group: :test
+
+# TEMPORARY: cloudstack_client 1.6.0 cannot be used outside of cloudstack-cli.
+# It calls ActiveSupport's `present?` on every request, and `require "base64"`
+# is a LoadError on Ruby 3.4+. Both are fixed in niwo/cloudstack_client#20.
+# Remove this override and pin the gemspec to the release that carries the fix.
+gem "cloudstack_client", git: "https://github.com/tas50/cloudstack_client.git",
+  branch: "fix-standalone-library-use"
+
 group :docs do
   gem "yard"
 end

README.md

@@ -12,7 +12,7 @@ A [Test Kitchen](https://kitchen.ci/) driver for [Apache CloudStack](https://clo
 - API credentials for it: an API key, a secret key, and the API URL
 - Test Kitchen 3.0 or newer, including the version bundled with current Cinc
   Workstation and Chef Workstation
-- `fog-cloudstack`, installed automatically as a dependency
+- `cloudstack_client`, installed automatically as a dependency
 
 ## Installation
 

kitchen-cloudstack.gemspec

@@ -25,5 +25,5 @@ Gem::Specification.new do |spec|
   }
 
   spec.add_dependency "test-kitchen", ">= 3.0", "< 5"
-  spec.add_dependency "fog-cloudstack", "~> 0.1.0"
+  spec.add_dependency "cloudstack_client", "~> 1.6"
 end

lib/kitchen/driver/cloudstack.rb

@@ -64,7 +64,6 @@ class Cloudstack < Kitchen::Driver::Base
       # @return [void]
       def create(state)
         super
-        disable_ssl_validation! if config[:disable_ssl_validation]
 
         server_info = deploy_instance(state)
 
@@ -88,10 +87,11 @@ def destroy(state)
 
         networking.teardown(state) if config[:associate_public_ip]
 
-        client.compute.destroy_virtual_machine(
+        client.api.destroy_virtual_machine({
           "id" => state[:server_id],
-          "expunge" => !!config[:cloudstack_expunge]
-        )
+          # Stringified because cloudstack_client drops falsey values.
+          "expunge" => (!!config[:cloudstack_expunge]).to_s,
+        }, Client::SYNC)
         info("CloudStack instance <#{state[:server_id]}> destroyed.")
 
         STATE_KEYS.each { |key| state.delete(key) }
@@ -176,7 +176,7 @@ def endpoint_problems
         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
+        client.api.list_zones
         []
       rescue URI::InvalidURIError => e
         ["cloudstack_api_url (#{config[:cloudstack_api_url]}) is not a URL: #{e.message}"]
@@ -193,8 +193,7 @@ def deploy_instance(state)
         options = ServerOptions.new(config, instance_name: instance.name).to_h
         debug("Deploying CloudStack instance with #{options}")
 
-        response = client.compute.deploy_virtual_machine(options)
-          .fetch("deployvirtualmachineresponse")
+        response = client.api.deploy_virtual_machine(options, Client::SYNC)
 
         state[:server_id] = response.fetch("id")
         info("CloudStack instance <#{state[:server_id]}> created.")
@@ -261,8 +260,10 @@ def wait_for_guest_password_sync
       # @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"]
+        machines = client.api.list_virtual_machines(
+          "id" => server_id,
+          "projectid" => config[:cloudstack_project_id]
+        )
         return nil unless machines.is_a?(Array) && !machines.empty?
 
         machines.first["state"]
@@ -282,17 +283,6 @@ def networking
       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
-      end
     end
   end
 end

lib/kitchen/driver/cloudstack/client.rb

@@ -13,8 +13,7 @@
 
 require "kitchen/driver/base"
 require "kitchen/errors"
-require "fog/cloudstack"
-require "uri" unless defined?(URI)
+require "cloudstack_client"
 
 module Kitchen
   module Driver
@@ -24,6 +23,14 @@ class Cloudstack < Kitchen::Driver::Base
       # Almost every CloudStack call that changes something returns a job id
       # rather than a result, so callers use {#run_job} to turn that job id
       # into the eventual result, or into an ActionFailed.
+      #
+      # cloudstack_client can wait for those jobs itself, but doing so hands
+      # back only the finished job result -- the caller never sees the id of
+      # the resource being built. The driver records that id in instance state
+      # before waiting, so that an interrupted +kitchen create+ still leaves
+      # behind enough state to destroy what it started. Every asynchronous
+      # call is therefore made with +sync: true+, which returns CloudStack's
+      # immediate response, and the waiting happens here instead.
       class Client
         # Value CloudStack reports in an async job's "jobstatus" field while
         # the job is still running.
@@ -43,36 +50,37 @@ class Client
         # not configured.
         DEFAULT_TIMEOUT = 600
 
+        # Options passed to every asynchronous API call, asking for
+        # CloudStack's immediate response rather than cloudstack_client's own
+        # job polling. See the class documentation for why.
+        SYNC = { sync: true }.freeze
+
         # @param config [Hash] the driver configuration
-        # @param compute [Fog::Compute, nil] an existing connection, for tests
+        # @param api [CloudstackClient::Client, nil] an existing connection,
+        #   for tests
         # @param sleeper [#call, nil] receives a number of seconds to wait,
         #   for tests that must not actually sleep
-        def initialize(config, compute: nil, sleeper: nil)
+        def initialize(config, api: nil, sleeper: nil)
           @config = config
-          @compute = compute
+          @api = api
           @sleeper = sleeper || ->(seconds) { sleep(seconds) }
         end
 
-        # The fog CloudStack connection, built from the configured endpoint.
+        # The 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.
+        # Certificate verification is on unless +disable_ssl_validation+ asks
+        # for it to be off, and unlike the fog connection this replaced, that
+        # choice is scoped to this connection rather than set process-wide.
         #
-        # @return [Fog::Compute] a CloudStack compute connection
-        def compute
-          @compute ||= begin
-            uri = URI.parse(config[:cloudstack_api_url])
-            Fog::Compute.new(
-              provider: :cloudstack,
-              cloudstack_api_key: config[:cloudstack_api_key],
-              cloudstack_secret_access_key: config[:cloudstack_secret_key],
-              cloudstack_host: uri.host,
-              cloudstack_port: uri.port,
-              cloudstack_path: uri.path,
-              cloudstack_project_id: config[:cloudstack_project_id],
-              cloudstack_scheme: uri.scheme
-            )
-          end
+        # @return [CloudstackClient::Client] a CloudStack API connection
+        def api
+          @api ||= CloudstackClient::Client.new(
+            config[:cloudstack_api_url],
+            config[:cloudstack_api_key],
+            config[:cloudstack_secret_key],
+            quiet: true,
+            ssl_verify: !config[:disable_ssl_validation]
+          )
         end
 
         # Waits for an asynchronous CloudStack job to finish.
@@ -84,7 +92,7 @@ def run_job(jobid)
           elapsed = 0
 
           loop do
-            response = compute.query_async_job_result(jobid)["queryasyncjobresultresponse"]
+            response = api.query_async_job_result("jobid" => jobid)
 
             case response.fetch("jobstatus").to_i
             when JOB_SUCCEEDED
@@ -103,15 +111,6 @@ def run_job(jobid)
           end
         end
 
-        # Runs a request that returns an async job id, and waits for it.
-        #
-        # @param response [Hash] the raw response from a fog request
-        # @param key [String] the response envelope key holding the job id
-        # @return [Hash] the job's "jobresult" payload
-        def run_response_job(response, key)
-          run_job(response.fetch(key).fetch("jobid"))
-        end
-
         private
 
         attr_reader :config, :sleeper

lib/kitchen/driver/cloudstack/networking.rb

@@ -12,7 +12,8 @@
 # limitations under the License.
 
 require "kitchen/driver/base"
-require "fog/cloudstack"
+require "cloudstack_client"
+require_relative "client"
 
 module Kitchen
   module Driver
@@ -25,7 +26,7 @@ class Cloudstack < Kitchen::Driver::Base
       # transport connects on, so a WinRM instance gets 5985 rather than SSH's
       # 22 without any extra configuration.
       class Networking
-        # CloudStack reports an already-deleted resource as a BadRequest with
+        # CloudStack reports an already-deleted resource as an API error with
         # this in the message. Teardown treats it as success.
         ALREADY_GONE = /does not exist/
 
@@ -45,12 +46,13 @@ def initialize(config, client:, port:, logger: nil)
         # @param state [Hash] mutable instance state
         # @return [String] the allocated public IP address
         def associate_public_ip(state)
-          response = compute.associate_ip_address(
+          response = api.associate_ip_address({
             "zoneid" => config[:cloudstack_zone_id],
             "vpcid" => vpc_id,
-            "networkid" => config[:cloudstack_network_id]
-          )
-          result = client.run_response_job(response, "associateipaddressresponse")
+            "networkid" => config[:cloudstack_network_id],
+            "projectid" => config[:cloudstack_project_id],
+          }, Client::SYNC)
+          result = client.run_job(response.fetch("jobid"))
           address = result.fetch("ipaddress")
 
           state[:ipaddressid] = address.fetch("id")
@@ -65,17 +67,21 @@ def associate_public_ip(state)
         # @param virtualmachine_id [String] the instance to forward to
         # @return [void]
         def create_port_forward(state, virtualmachine_id)
-          response = compute.create_port_forwarding_rule(
+          response = api.create_port_forwarding_rule({
             "ipaddressid" => state[:ipaddressid],
             "privateport" => port,
             "protocol" => "TCP",
             "publicport" => port,
             "virtualmachineid" => virtualmachine_id,
             "networkid" => config[:cloudstack_network_id],
-            "openfirewall" => false
-          )
-          state[:forwardingruleid] = response.fetch("createportforwardingruleresponse", {})["id"]
-          client.run_response_job(response, "createportforwardingruleresponse")
+            # cloudstack_client drops falsey argument values, and CloudStack
+            # defaults this to true on a non-VPC network, so a boolean false
+            # here would let CloudStack open the firewall itself -- creating a
+            # rule teardown does not know about.
+            "openfirewall" => "false",
+          }, Client::SYNC)
+          state[:forwardingruleid] = response["id"]
+          client.run_job(response.fetch("jobid"))
         end
 
         # Removes everything {#associate_public_ip} and {#create_port_forward}
@@ -93,23 +99,22 @@ def teardown(state)
 
         attr_reader :config, :client, :port, :logger
 
-        # @return [Fog::Compute] the shared CloudStack connection
-        def compute = client.compute
+        # @return [CloudstackClient::Client] the shared CloudStack connection
+        def api = client.api
 
         # 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],
+          response = api.create_firewall_rule({
             "cidrlist" => config[:cloudstack_firewall_cidr] || "0.0.0.0/0",
             "protocol" => "tcp",
             "startport" => port,
             "endport" => port,
-            "ipaddressid" => state[:ipaddressid]
-          )
-          result = client.run_response_job(response, "createfirewallruleresponse")
+            "ipaddressid" => state[:ipaddressid],
+          }, Client::SYNC)
+          result = client.run_job(response.fetch("jobid"))
           rule = result["firewallrule"]
           state[:firewall_rule_id] = rule["id"] if rule.is_a?(Hash)
         end
@@ -120,8 +125,8 @@ def create_firewall_rule(state)
         # @return [void]
         def delete_port_forward(state)
           tolerating_missing("port forwarding rule") do
-            response = compute.delete_port_forwarding_rule(state[:forwardingruleid])
-            client.run_response_job(response, "deleteportforwardingruleresponse")
+            response = api.delete_port_forwarding_rule({ "id" => state[:forwardingruleid] }, Client::SYNC)
+            client.run_job(response.fetch("jobid"))
           end
         end
 
@@ -131,8 +136,8 @@ def delete_port_forward(state)
         # @return [void]
         def delete_firewall_rule(state)
           tolerating_missing("firewall rule") do
-            response = compute.delete_firewall_rule(state[:firewall_rule_id])
-            client.run_response_job(response, "deletefirewallruleresponse")
+            response = api.delete_firewall_rule({ "id" => state[:firewall_rule_id] }, Client::SYNC)
+            client.run_job(response.fetch("jobid"))
           end
         end
 
@@ -142,8 +147,8 @@ def delete_firewall_rule(state)
         # @return [void]
         def release_public_ip(state)
           tolerating_missing("public IP address") do
-            response = compute.disassociate_ip_address(state[:ipaddressid])
-            client.run_response_job(response, "disassociateipaddressresponse")
+            response = api.disassociate_ip_address({ "id" => state[:ipaddressid] }, Client::SYNC)
+            client.run_job(response.fetch("jobid"))
           end
         end
 
@@ -153,19 +158,19 @@ def release_public_ip(state)
         # @param description [String] names the resource, for the debug message
         # @yield the deletion call to attempt
         # @return [void]
-        # @raise [Fog::Cloudstack::Compute::BadRequest] for any error other
-        #   than the resource already being gone
+        # @raise [CloudstackClient::ApiError] for any error other than the
+        #   resource already being gone
         def tolerating_missing(description)
           yield
-        rescue Fog::Cloudstack::Compute::BadRequest => e
+        rescue CloudstackClient::ApiError => e
           raise unless e.to_s.match?(ALREADY_GONE)
 
           logger&.debug("CloudStack #{description} was already gone: #{e}")
         end
 
         # A VPC network needs its vpcid passed when allocating an address.
         def vpc_id
-          networks = compute.list_networks.fetch("listnetworksresponse", {})["network"]
+          networks = api.list_networks("projectid" => config[:cloudstack_project_id])
           return nil unless networks.is_a?(Array)
 
           network = networks.find { |n| n["id"] == config[:cloudstack_network_id] }