From 19f046952c28bdbb235ea2cb65d1454c88318cae Mon Sep 17 00:00:00 2001
From: Josh Partlow <joshua.partlow@puppetlabs.com>
Date: Fri, 29 Sep 2023 12:46:06 -0700
Subject: [PATCH] (maint) Fix expect_upload for module relative paths outside
 files

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.
---
 lib/bolt_spec/plans/mock_executor.rb          |  6 +++++-
 spec/bolt_spec/plan_spec.rb                   |  1 +
 spec/bolt_spec/plans/mock_executor_spec.rb    | 16 ++++++++++++++++
 spec/fixtures/bolt_spec/plans/plans/upload.pp |  1 +
 spec/fixtures/bolt_spec/plans/resources/bar   |  1 +
 5 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 spec/fixtures/bolt_spec/plans/resources/bar

diff --git a/lib/bolt_spec/plans/mock_executor.rb b/lib/bolt_spec/plans/mock_executor.rb
index 963421f280..fce9ae592e 100644
--- a/lib/bolt_spec/plans/mock_executor.rb
+++ b/lib/bolt_spec/plans/mock_executor.rb
@@ -49,7 +49,11 @@ 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('/')
+        if segments[1] == 'files'
+          ([segments[0]] + segments[2..-1]).join('/')
+        else
+          segments.join('/')
+        end
       end
 
       def run_command(targets, command, options = {}, _position = [])
diff --git a/spec/bolt_spec/plan_spec.rb b/spec/bolt_spec/plan_spec.rb
index 3494a14c4d..347e0a716f 100644
--- a/spec/bolt_spec/plan_spec.rb
+++ b/spec/bolt_spec/plan_spec.rb
@@ -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
diff --git a/spec/bolt_spec/plans/mock_executor_spec.rb b/spec/bolt_spec/plans/mock_executor_spec.rb
index 99e16a2b37..4b72e21a2e 100644
--- a/spec/bolt_spec/plans/mock_executor_spec.rb
+++ b/spec/bolt_spec/plans/mock_executor_spec.rb
@@ -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
diff --git a/spec/fixtures/bolt_spec/plans/plans/upload.pp b/spec/fixtures/bolt_spec/plans/plans/upload.pp
index f894ea0628..687b3813a6 100644
--- a/spec/fixtures/bolt_spec/plans/plans/upload.pp
+++ b/spec/fixtures/bolt_spec/plans/plans/upload.pp
@@ -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)
 }
diff --git a/spec/fixtures/bolt_spec/plans/resources/bar b/spec/fixtures/bolt_spec/plans/resources/bar
new file mode 100644
index 0000000000..bd5c583268
--- /dev/null
+++ b/spec/fixtures/bolt_spec/plans/resources/bar
@@ -0,0 +1 @@
+Some other file outside of files/.