Skip to content

Commit

Permalink
(maint) Fix expect_upload for module relative paths outside files
Browse files Browse the repository at this point in the history
The expect_upload() BoltSpec expectation for upload_file calls in plans
handled two cases:

Given a /my/modules/amodule with

  /my/modules/amodule/files/foo
  /my/modules/amodule/resources/baz
  /my/modules/amodule/plans/bar

expect_upload('/some/thing') would correctly match against an
upload_file('/some/thing') in plan bar, because it is not relative to
the modulepath at all, and against an upload_file('amodule/foo'),
because it is relative to amodule, and foo is relative to files,
which expect_upload elides when the MockExecutor runs the source_path
through BotlSpec::MockExecutor#module_file_id.

The edge case that was peculiar was a call to
upload_file('amodule/files/../resources/baz'), which expect_upload
would end up expecting as 'amodule/baz'.

The hidden behavior here is that Bolt::Util.find_file* functionality
will take 'foo/bar' and look for /modulepath/foo/files/bar, which is
mirroring Puppety behavior to find module files relative to the files/
dir.
  • Loading branch information
jpartlow committed Sep 29, 2023
1 parent 6ff6505 commit 6a424e4
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/bolt_spec/plans/mock_executor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ def module_file_id(file)
path = Pathname.new(file)
relative = path.relative_path_from(Pathname.new(modpath.first))
segments = relative.to_path.split('/')
([segments[0]] + segments[2..-1]).join('/')
segments[1] == 'files' ?
([segments[0]] + segments[2..-1]).join('/') :
segments.join('/')
end

def run_command(targets, command, options = {}, _position = [])
Expand Down
1 change: 1 addition & 0 deletions spec/bolt_spec/plan_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ def expect_action
with_tempfile_containing('prep', '') do |file|
@source = file.path
allow_upload(@source).with_targets(targets)
expect_upload('plans/resources/bar').with_destination('/o')
example.run
end
end
Expand Down
16 changes: 16 additions & 0 deletions spec/bolt_spec/plans/mock_executor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,20 @@

expect(missing_methods.empty?).to be(true), message
end

context '#module_file_id' do
let(:executor) { BoltSpec::Plans::MockExecutor.new('/some/path/to/modules') }

it 'returns nil if path is outside of modulepath' do
expect(executor.module_file_id('/some/other/path')).to be_nil
end

it 'handles module relative paths relative to module/files returning module/path excluding the files dir' do
expect(executor.module_file_id('/some/path/to/modules/amodule/files/dingo')).to eq('amodule/dingo')
end

it 'handles module relative paths outside of module/files' do
expect(executor.module_file_id('/some/path/to/modules/amodule/files/../other/dingo')).to eq('amodule/other/dingo')
end
end
end
1 change: 1 addition & 0 deletions spec/fixtures/bolt_spec/plans/plans/upload.pp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
plan plans::upload(TargetSpec $nodes, String $source) {
upload_file($source, '/b', $nodes)
upload_file('plans/files/../resources/bar', '/o', $nodes)
return upload_file('plans/script', '/d', $nodes)
}
1 change: 1 addition & 0 deletions spec/fixtures/bolt_spec/plans/resources/bar
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Some other file outside of files/.

0 comments on commit 6a424e4

Please sign in to comment.