(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

@@ -23,7 +23,7 @@ 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,32 @@ 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'
           }
         }
-        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 +259,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 +303,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 +361,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)

spec/unit/ace/transport_app_spec.rb

@@ -32,7 +32,7 @@ def app
     {
       node: "fw.example.net",
       status: "success",
-      result: {
+      value: {
         "output" => "Hello!"
       }
     }
@@ -171,13 +171,13 @@ def app
 
     context 'when Bolt::Inventory throws' do
       before do
-        allow(Bolt::Inventory).to receive(:new).and_raise Exception
+        allow(Bolt::Inventory).to receive(:empty).and_raise(StandardError.new('yeah right'))
       end
 
       it 'will be caught and handled by ace/processing_exception' do
         post '/run_task', JSON.generate(body), 'CONTENT_TYPE' => 'text/json'
 
-        expect(last_response.body).to match(%r{puppetlabs\/ace\/processing_exception})
+        expect(last_response.body).to match(%r{puppetlabs\/ace\/execution_exception})
         expect(last_response.status).to eq(500)
       end
     end
@@ -191,7 +191,7 @@ def app
         expect(last_response.status).to eq(200)
         result = JSON.parse(last_response.body)
         expect(result).to include('status' => 'success')
-        expect(result['result']['output']).to eq('Hello!')
+        expect(result['value']['output']).to eq('Hello!')
       end
     end
 
@@ -212,7 +212,7 @@ def app
         expect(last_response.status).to eq(200)
         result = JSON.parse(last_response.body)
         expect(result).to include('status' => 'success')
-        expect(result['result']['output']).to eq('Hello!')
+        expect(result['value']['output']).to eq('Hello!')
       end
     end
 
@@ -221,7 +221,7 @@ def app
         {
           node: "fw.example.net",
           status: "failure",
-          result: {
+          value: {
             '_error' => {
               'msg' => 'Failed to open TCP connection to fw.example.net',
               'kind' => 'module/unknown',
@@ -251,7 +251,7 @@ def app
         expect(last_response.status).to eq(200)
         result = JSON.parse(last_response.body)
         expect(result).to include('status' => 'failure')
-        expect(result['result']['_error']).not_to have_key('backtrace')
+        expect(result['value']['_error']).not_to have_key('backtrace')
       end
     end
 
@@ -260,7 +260,7 @@ def app
         {
           node: "fw.example.net",
           status: "failure",
-          result: {
+          value: {
             '_error' => {
               'msg' => 'Failed to open TCP connection to fw.example.net',
               'kind' => 'module/unknown',
@@ -290,7 +290,7 @@ def app
         expect(last_response.status).to eq(200)
         result = JSON.parse(last_response.body)
         expect(result).to include('status' => 'failure')
-        expect(result['result']['_error']).not_to have_key('stack_trace')
+        expect(result['value']['_error']).not_to have_key('stack_trace')
       end
     end
   end