Skip to content

Commit

Permalink
(PE-28373) Use Bolt 2.x WIP
Browse files Browse the repository at this point in the history 
First stab at adopting bolt 2.x. More to come...
  • Loading branch information
@donoghuc
donoghuc committed Mar 26, 2020
1 parent 508ad1f commit 45b1eaf
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 44 deletions.
2 changes: 1 addition & 1 deletion agentless-catalog-executor.gemspec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
38 changes: 17 additions & 21 deletions lib/ace/transport_app.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,46 +212,47 @@ 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)]
inventory = Bolt::Inventory.empty
target_data = {
'name' => body['target']['host'] || body['target']['name'] || 'remote',
'config' => {
'transport' => 'remote'
}
}
target = [Bolt::Target.from_hash(target_data, inventory)]
rescue ACE::Error => e
request_error = {
"node": target,
"target": target,
"target": target.name,
"action": nil,
"object": nil,
"status": "failure",
"result": {
"value": {
"_error": e.to_h
}
}
return [400, request_error.to_json]
rescue JSON::ParserError => e
request_error = {
"node": target,
"target": target,
"target": target.name,
"action": nil,
"object": nil,
"status": "failure",
"result": {
"value": {
"_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]
rescue StandardError => e
# TODO: for all these can we not construct a Bolt::Result???
request_error = {
"node": target,
"target": target,
"target": target.name,
"action": nil,
"object": nil,
"status": "failure",
"result": {
"value": {
"_error": ACE::Error.to_h(e.message,
'puppetlabs/ace/execution_exception',
class: e.class, backtrace: e.backtrace).to_h
Expand All @@ -261,10 +262,6 @@ def self.trusted_facts(certname)
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)

Expand All @@ -279,12 +276,11 @@ 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": {
"value": {
"_error": ACE::Error.to_h(e.message,
'puppetlabs/ace/processing_exception',
class: e.class, backtrace: e.backtrace).to_h
Expand Down
1 change: 1 addition & 0 deletions spec/acceptance/ace/transport_app_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
44 changes: 22 additions & 22 deletions spec/unit/ace/transport_app_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def app
{
node: "fw.example.net",
status: "success",
result: {
value: {
"output" => "Hello!"
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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',
Expand Down Expand Up @@ -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

Expand All @@ -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',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -453,9 +453,9 @@ def app
expect(last_response.status).to eq(400)
result = JSON.parse(last_response.body)
expect(result['status']).to eq('failure')
expect(result['result']['_error']['msg']).to eq('There was an error validating the request body.')
expect(result['result']['_error']['kind']).to eq('puppetlabs/ace/schema-error')
expect(result['result']['_error']['details']['schema_error']).to match(
expect(result['value']['_error']['msg']).to eq('There was an error validating the request body.')
expect(result['value']['_error']['kind']).to eq('puppetlabs/ace/schema-error')
expect(result['value']['_error']['details']['schema_error']).to match(
%r{The property '#/compiler' did not contain a required property of 'environment' in schema}
)
end
Expand Down Expand Up @@ -485,10 +485,10 @@ def app
expect(last_response.status).to eq(400)
result = JSON.parse(last_response.body)
expect(result['status']).to eq('failure')
expect(result['result']['_error']['msg']).to match(/unexpected token at/)
expect(result['result']['_error']['kind']).to eq('puppetlabs/ace/request_exception')
expect(result['result']['_error']['details']['class']).to eq('JSON::ParserError')
expect(result['result']['_error']['details']).to be_key('backtrace')
expect(result['value']['_error']['msg']).to match(/unexpected token at/)
expect(result['value']['_error']['kind']).to eq('puppetlabs/ace/request_exception')
expect(result['value']['_error']['details']['class']).to eq('JSON::ParserError')
expect(result['value']['_error']['details']).to be_key('backtrace')
end
end

Expand All @@ -506,8 +506,8 @@ def app
expect(last_response.status).to eq(400)
result = JSON.parse(last_response.body)
expect(result).to eq('certname' => 'fw.example.net', 'status' => 'failure',
'result' => { '_error' => { 'msg' => 'something',
'kind' => 'something/darkside', 'details' => {} } })
'value' => { '_error' => { 'msg' => 'something',
'kind' => 'something/darkside', 'details' => {} } })
end
end

Expand All @@ -525,10 +525,10 @@ def app
expect(last_response.status).to eq(500)
result = JSON.parse(last_response.body)
expect(result['status']).to eq('failure')
expect(result['result']['_error']['msg']).to eq('unknown error')
expect(result['result']['_error']['kind']).to eq('puppetlabs/ace/processing_exception')
expect(result['result']['_error']['details']['class']).to eq('RuntimeError')
expect(result['result']['_error']['details']).to be_key('backtrace')
expect(result['value']['_error']['msg']).to eq('unknown error')
expect(result['value']['_error']['kind']).to eq('puppetlabs/ace/processing_exception')
expect(result['value']['_error']['details']['class']).to eq('RuntimeError')
expect(result['value']['_error']['details']).to be_key('backtrace')
end
end
end
Expand Down

0 comments on commit 45b1eaf

Please sign in to comment.