Merge pull request #7083 from nicklewis/PUP-9135-wrong-error-when-module-not-found

(PUP-9135) Treat task metadata referring to missing module as invalid

Author: nicklewis

lib/puppet/module/task.rb

@@ -63,11 +63,6 @@ def self.is_tasks_filename?(path)
     end
 
     def self.get_file_details(path, mod)
-      unless File.absolute_path(path) == File.path(path)
-        msg = _("File pathnames cannot include relative paths")
-        raise InvalidMetadata.new(msg, 'puppet.tasks/invalid-metadata')
-      end
-
       # This gets the path from the starting point onward
       # For files this should be the file subpath from the metadata
       # For directories it should be the directory subpath plus whatever we globbed
@@ -90,17 +85,23 @@ def self.find_files(files, mod)
 
         pup_module = Puppet::Module.find(module_name, env)
         if pup_module.nil?
-          raise Puppet::Module::MissingModule, _("Module %{module_name} not found in environment %{environment_name}.") %
-            {module_name: pup_module.name, environment_name: env}
+          msg = _("Could not find module %{module_name} containing task file %{filename}" %
+                  {module_name: module_name, filename: endpath})
+          raise InvalidMetadata.new(msg, 'puppet.tasks/invalid-metadata')
         end
 
         unless MOUNTS.include? mount
-          msg = _("Files must be saved in module directories that Puppet makes available via mount points: %{mounts}" % 
+          msg = _("Files must be saved in module directories that Puppet makes available via mount points: %{mounts}" %
                   {mounts: MOUNTS.join(', ')})
           raise InvalidMetadata.new(msg, 'puppet.tasks/invalid-metadata')
         end
 
         path = File.join(pup_module.path, mount, endpath)
+        unless File.absolute_path(path) == File.path(path).chomp('/')
+          msg = _("File pathnames cannot include relative paths")
+          raise InvalidMetadata.new(msg, 'puppet.tasks/invalid-metadata')
+        end
+
         unless File.exist?(path)
           msg = _("Could not find %{path} on disk" % { path: path })
           raise InvalidFile.new(msg)

spec/unit/task_spec.rb

@@ -92,6 +92,18 @@
     expect(tasks.map{|t| t.files.map{ |f| f["path"] } }).to eq([["#{tasks_path}/task1.sh"] + long_files])
   end
 
+  it "fails to load a task if its metadata specifies a non-existent file" do
+    og_files = %w{task1.sh task1.json}.map { |bn| "#{tasks_path}/#{bn}" }
+    Dir.stubs(:glob).with(tasks_glob).returns(og_files)
+    File.stubs(:exist?).with(any_parameters).returns(true)
+
+    Puppet::Module.expects(:find).with(othermod.name, "production").returns(nil).at_least(1)
+    tasks = Puppet::Module::Task.tasks_in_module(mymod)
+    Puppet::Module::Task.any_instance.stubs(:metadata).returns({'files' => ["#{othermod.name}/files/test"]})
+
+    expect { tasks.first.files }.to raise_error(Puppet::Module::Task::InvalidMetadata, /Could not find module #{othermod.name} containing task file test/)
+  end
+
   it "finds files whose names (besides extensions) are valid task names" do
     Dir.expects(:glob).with(tasks_glob).returns(%w{task task_1 xx_t_a_s_k_2_xx})
     tasks = Puppet::Module::Task.tasks_in_module(mymod)