(PE-28373) Use Bolt 2.x

Make ACE compatible with Bolt 2.x by updating how we make instantiate inventory/targets and return Results. Specifically every task result should at least have the keys `status` and `value`.

.rubocop.yml

@@ -7,6 +7,7 @@ AllCops:
     - 'acceptance/vendor/**/*'
     - 'modules/**/*'
     - 'spec/volumes/**/*'
+    - 'tmp/**/*'
 
 # Checks for if and unless statements that would fit on one line if written as a
 # modifier if/unless.

agentless-catalog-executor.gemspec

@@ -22,8 +22,8 @@ Gem::Specification.new do |spec|
 
   # Pin concurrent-ruby to 1.1.5 until https://github.com/ruby-concurrency/concurrent-ruby/pull/856 is released
   spec.add_dependency "concurrent-ruby", "1.1.5"
-  # TODO: migrate to bolt 2.x
-  spec.add_dependency "bolt",  "~> 1.31"
+
+  spec.add_dependency "bolt",  "~> 2.0"
 
   # server-side dependencies cargo culted from https://github.com/puppetlabs/bolt/blob/4418da408643aa7eb5ed64f4c9704b941ea878dc/Gemfile#L10-L16
   spec.add_dependency "hocon", ">= 1.2.5"

lib/ace/transport_app.rb

@@ -137,15 +137,22 @@ def self.init_puppet_target(certname, transport, target)
     end
 
     def scrub_stack_trace(result)
-      if result.dig(:result, '_error', 'details', 'stack_trace')
-        result[:result]['_error']['details'].reject! { |k| k == 'stack_trace' }
+      if result.dig(:value, '_error', 'details', 'stack_trace')
+        result[:value]['_error']['details'].reject! { |k| k == 'stack_trace' }
       end
-      if result.dig(:result, '_error', 'details', 'backtrace')
-        result[:result]['_error']['details'].reject! { |k| k == 'backtrace' }
+      if result.dig(:value, '_error', 'details', 'backtrace')
+        result[:value]['_error']['details'].reject! { |k| k == 'backtrace' }
       end
       result
     end
 
+    def error_result(error)
+      {
+        'status' => 'failure',
+        'value' => { '_error' => error.to_h }
+      }
+    end
+
     def validate_schema(schema, body)
       schema_error = JSON::Validator.fully_validate(schema, body)
       if schema_error.any?
@@ -212,59 +219,33 @@ def self.trusted_facts(certname)
       begin
         body = JSON.parse(request.body.read)
         validate_schema(@schemas["run_task"], body)
-        opts = body['target'].merge('protocol' => 'remote')
 
-        # This is a workaround for Bolt due to the way it expects to receive the target info
-        # see: https://github.com/puppetlabs/bolt/pull/915#discussion_r268280535
-        # Systems calling into ACE will need to determine the nodename/certname and pass this as `name`
-        target = [Bolt::Target.new(body['target']['host'] || body['target']['name'], opts)]
-      rescue ACE::Error => e
-        request_error = {
-          "node": target,
-          "target": target,
-          "action": nil,
-          "object": nil,
-          "status": "failure",
-          "result": {
-            "_error": e.to_h
+        inventory = Bolt::Inventory.empty
+        target_data = {
+          'name' => body['target']['host'] || body['target']['name'] || 'remote',
+          'config' => {
+            'transport' => 'remote',
+            'remote' => body['target']
           }
         }
-        return [400, request_error.to_json]
+        target = [Bolt::Target.from_hash(target_data, inventory)]
+      rescue ACE::Error => e
+        return [400, error_result(e).to_json]
       rescue JSON::ParserError => e
-        request_error = {
-          "node": target,
-          "target": target,
-          "action": nil,
-          "object": nil,
-          "status": "failure",
-          "result": {
-            "_error": ACE::Error.to_h(e.message,
-                                      'puppetlabs/ace/request_exception',
-                                      class: e.class, backtrace: e.backtrace).to_h
-          }
-        }
-        return [400, request_error.to_json]
+        request_error = ACE::Error.new(e.message,
+                                       'puppetlabs/ace/request_exception',
+                                       class: e.class, backtrace: e.backtrace).to_h
+
+        return [400, error_result(request_error).to_json]
       rescue StandardError => e
-        request_error = {
-          "node": target,
-          "target": target,
-          "action": nil,
-          "object": nil,
-          "status": "failure",
-          "result": {
-            "_error": ACE::Error.to_h(e.message,
-                                      'puppetlabs/ace/execution_exception',
-                                      class: e.class, backtrace: e.backtrace).to_h
-          }
-        }
-        return [500, request_error.to_json]
+        request_error = ACE::Error.new(e.message,
+                                       'puppetlabs/ace/execution_exception',
+                                       class: e.class, backtrace: e.backtrace).to_h
+
+        return [500, error_result(request_error).to_json]
       end
 
       begin
-        inventory = Bolt::Inventory.new(nil)
-
-        target.first.inventory = inventory
-
         task_data = body['task']
         task = Bolt::Task::PuppetServer.new(task_data['name'], task_data['metadata'], task_data['files'], @file_cache)
 
@@ -279,15 +260,14 @@ def self.trusted_facts(certname)
       rescue Exception => e # rubocop:disable Lint/RescueException
         # handle all the things and make it obvious what happened
         process_error = {
-          "node": target,
-          "target": target,
+          "target": target.name,
           "action": nil,
           "object": nil,
           "status": "failure",
-          "result": {
-            "_error": ACE::Error.to_h(e.message,
-                                      'puppetlabs/ace/processing_exception',
-                                      class: e.class, backtrace: e.backtrace).to_h
+          "value": {
+            "_error": ACE::Error.new(e.message,
+                                     'puppetlabs/ace/processing_exception',
+                                     class: e.class, backtrace: e.backtrace).to_h
           }
         }
         return [500, process_error.to_json]
@@ -324,9 +304,9 @@ def self.trusted_facts(certname)
         request_error = {
           status: 'failure',
           result: {
-            _error: ACE::Error.to_h(e.message,
-                                    'puppetlabs/ace/request_exception',
-                                    class: e.class, backtrace: e.backtrace)
+            _error: ACE::Error.new(e.message,
+                                   'puppetlabs/ace/request_exception',
+                                   class: e.class, backtrace: e.backtrace)
           }
         }
         return [400, request_error.to_json]
@@ -382,9 +362,9 @@ def self.trusted_facts(certname)
           certname: certname,
           status: 'failure',
           result: {
-            _error: ACE::Error.to_h(e.message,
-                                    'puppetlabs/ace/processing_exception',
-                                    class: e.class, backtrace: e.backtrace).to_h
+            _error: ACE::Error.new(e.message,
+                                   'puppetlabs/ace/processing_exception',
+                                   class: e.class, backtrace: e.backtrace).to_h
           }
         }
         return [500, process_error.to_json]

spec/acceptance/ace/transport_app_spec.rb

@@ -160,6 +160,7 @@ def app
       describe 'success' do
         it 'returns 200 with `success` status' do
           post '/run_task', JSON.generate(run_task_body), 'CONTENT_TYPE' => 'text/json'
+
           expect(last_response.errors).to match(/\A\Z/)
           expect(last_response).to be_ok
           expect(last_response.status).to eq(200)