Merge pull request #1203 from mwaggett/maint/main/fix-puppetfile-path

(maint) Pass the appropriate Puppetfile path to module loader

CHANGELOG.mkd

@@ -9,7 +9,7 @@ Unreleased
 - (CODEMGMT-1421) Add skeleton for deploy_spec option [#1189](https://github.com/puppetlabs/r10k/pull/1189)
 - (RK-369) Make module deploys run the postrun command if any environments were updated. [#982](https://github.com/puppetlabs/r10k/issues/982)
 - Add support for Github App auth token. This allows r10k to authenticate under strict SSO/2FA guidelines that cannot utilize machine users for code deployment. [#1180](https://github.com/puppetlabs/r10k/pull/1180)
-- Restore the ability to load a Puppetfile from a relative `basedir`. [#1202](https://github.com/puppetlabs/r10k/pull/1202)
+- Restore the ability to load a Puppetfile from a relative `basedir`. [#1202](https://github.com/puppetlabs/r10k/pull/1202), [#1203](https://github.com/puppetlabs/r10k/pull/1203)
 
 3.10.0
 ------

integration/tests/user_scenario/basic_workflow/single_env_purge_unmanaged_modules.rb

@@ -5,7 +5,9 @@
 
 #Init
 master_certname = on(master, puppet('config', 'print', 'certname')).stdout.rstrip
-moduledir = on(master, puppet('config', 'print', 'environmentpath')).stdout.strip + '/production/modules'
+environments_path = on(master, puppet('config', 'print', 'environmentpath')).stdout.strip
+moduledir = File.join(environments_path, 'production', 'modules')
+puppetfile_path = File.join(environments_path, 'production', 'Puppetfile')
 git_remote_environments_path = '/root/environments'
 last_commit = git_last_commit(master, git_remote_environments_path)
 r10k_fqp = get_r10k_fqp(master)
@@ -76,7 +78,7 @@ class { 'motd':
 end
 
 step 'Use r10k to Purge Unmanaged Modules'
-on(master, "#{r10k_fqp} puppetfile purge --puppetfile #{remote_puppetfile_path} --moduledir #{moduledir} --verbose debug --trace")
+on(master, "#{r10k_fqp} puppetfile purge --puppetfile #{puppetfile_path} --moduledir #{moduledir} --verbose debug --trace")
 
 #Agent will fail because r10k will purge the "motd" module
 agents.each do |agent|

lib/r10k/module_loader/puppetfile.rb

@@ -1,13 +1,17 @@
+require 'r10k/logging'
+
 module R10K
   module ModuleLoader
     class Puppetfile
 
+      include R10K::Logging
+
       DEFAULT_MODULEDIR = 'modules'
       DEFAULT_PUPPETFILE_NAME = 'Puppetfile'
       DEFAULT_FORGE_API = 'forgeapi.puppetlabs.com'
 
       attr_accessor :default_branch_override, :environment
-      attr_reader :modules, :moduledir,
+      attr_reader :modules, :moduledir, :puppetfile_path,
         :managed_directories, :desired_contents, :purge_exclusions
 
       # @param basedir [String] The path that contains the moduledir &
@@ -30,7 +34,7 @@ def initialize(basedir:,
 
         @basedir     = cleanpath(basedir)
         @moduledir   = resolve_path(@basedir, moduledir)
-        @puppetfile  = resolve_path(@basedir, puppetfile)
+        @puppetfile_path  = resolve_path(@basedir, puppetfile)
         @forge       = forge
         @overrides   = overrides
         @environment = environment
@@ -41,11 +45,14 @@ def initialize(basedir:,
         @managed_directories = []
         @desired_contents = []
         @purge_exclusions = []
+
+        logger.info _("Using Puppetfile '%{puppetfile}'") % {puppetfile: @puppetfile_path}
+        logger.debug _("Using moduledir '%{moduledir}'") % {moduledir: @moduledir}
       end
 
       def load
         dsl = R10K::ModuleLoader::Puppetfile::DSL.new(self)
-        dsl.instance_eval(puppetfile_content(@puppetfile), @puppetfile)
+        dsl.instance_eval(puppetfile_content(@puppetfile_path), @puppetfile_path)
 
         validate_no_duplicate_names(@modules)
         @modules
@@ -64,7 +71,7 @@ def load
         }
 
       rescue SyntaxError, LoadError, ArgumentError, NameError => e
-        raise R10K::Error.wrap(e, _("Failed to evaluate %{path}") % {path: @puppetfile})
+        raise R10K::Error.wrap(e, _("Failed to evaluate %{path}") % {path: @puppetfile_path})
       end
 
 

lib/r10k/puppetfile.rb

@@ -30,10 +30,6 @@ class Puppetfile
   #   @return [String] The base directory that contains the Puppetfile
   attr_reader :basedir
 
-  # @!attrbute [r] puppetfile_path
-  #   @return [String] The path to the Puppetfile
-  attr_reader :puppetfile_path
-
   # @!attribute [r] environment
   #   @return [R10K::Environment] Optional R10K::Environment that this Puppetfile belongs to.
   attr_reader :environment
@@ -68,21 +64,20 @@ def initialize(basedir, options_or_moduledir = nil, deprecated_path_arg = nil, d
 
     @force           = deprecated_force_arg     || options.delete(:force)           || false
     @moduledir       = deprecated_moduledir_arg || options.delete(:moduledir)       || File.join(basedir, 'modules')
-    @puppetfile_name = deprecated_name_arg      || options.delete(:puppetfile_name) || 'Puppetfile'
-    @puppetfile_path = deprecated_path_arg      || options.delete(:puppetfile_path) || File.join(basedir, @puppetfile_name)
+    puppetfile_name = deprecated_name_arg      || options.delete(:puppetfile_name) || 'Puppetfile'
+    puppetfile_path = deprecated_path_arg      || options.delete(:puppetfile_path)
+    @puppetfile = puppetfile_path || puppetfile_name
     @environment     = options.delete(:environment)
 
     @overrides       = options.delete(:overrides) || {}
     @default_branch_override = @overrides.dig(:environments, :default_branch_override)
 
-    logger.info _("Using Puppetfile '%{puppetfile}'") % {puppetfile: @puppetfile_path}
-
     @forge   = 'forgeapi.puppetlabs.com'
 
     @loader = ::R10K::ModuleLoader::Puppetfile.new(
       basedir: @basedir,
       moduledir: @moduledir,
-      puppetfile: @puppetfile_name,
+      puppetfile: @puppetfile,
       forge: @forge,
       overrides: @overrides,
       environment: @environment
@@ -106,8 +101,8 @@ def load(default_branch_override = nil)
     if self.loaded?
       return @loaded_content
     else
-      if !File.readable?(@puppetfile_path)
-        logger.debug _("Puppetfile %{path} missing or unreadable") % {path: @puppetfile_path.inspect}
+      if !File.readable?(puppetfile_path)
+        logger.debug _("Puppetfile %{path} missing or unreadable") % {path: puppetfile_path.inspect}
       else
         self.load!(default_branch_override)
       end
@@ -156,6 +151,10 @@ def moduledir
     @loader.moduledir
   end
 
+  def puppetfile_path
+    @loader.puppetfile_path
+  end
+
   def environment=(env)
     @loader.environment = env
     @environment = env

spec/unit/module_loader/puppetfile_spec.rb

@@ -37,13 +37,13 @@
         it 'respects absolute paths' do
           absolute_options = options.merge({puppetfile: '/opt/puppetlabs/special/Puppetfile'})
           puppetfile = R10K::ModuleLoader::Puppetfile.new(**absolute_options)
-          expect(puppetfile.instance_variable_get(:@puppetfile)).to eq('/opt/puppetlabs/special/Puppetfile')
+          expect(puppetfile.instance_variable_get(:@puppetfile_path)).to eq('/opt/puppetlabs/special/Puppetfile')
         end
 
         it 'roots the Puppetfile in the basepath if a relative path is specified' do
           relative_options = options.merge({puppetfile: 'Puppetfile.global'})
           puppetfile = R10K::ModuleLoader::Puppetfile.new(**relative_options)
-          expect(puppetfile.instance_variable_get(:@puppetfile)).to eq('/test/basedir/env/Puppetfile.global')
+          expect(puppetfile.instance_variable_get(:@puppetfile_path)).to eq('/test/basedir/env/Puppetfile.global')
         end
       end
 
@@ -68,7 +68,7 @@
       end
 
       it 'has a Puppetfile rooted in the basedir' do
-        expect(subject.instance_variable_get(:@puppetfile)).to eq('/test/basedir/Puppetfile')
+        expect(subject.instance_variable_get(:@puppetfile_path)).to eq('/test/basedir/Puppetfile')
       end
 
       it 'uses the public forge' do

spec/unit/puppetfile_spec.rb

@@ -10,14 +10,33 @@
     )
   end
 
-  describe "a custom puppetfile Puppetfile.r10k" do
+  describe "a custom puppetfile_name" do
     it "is the basedir joined with '/Puppetfile.r10k' path" do
       expect(subject.puppetfile_path).to eq '/some/nonexistent/basedir/Puppetfile.r10k'
     end
   end
 
 end
 
+describe R10K::Puppetfile do
+
+  describe "a custom relative puppetfile_path" do
+    it "is the basedir joined with the puppetfile_path" do
+      relative_subject = described_class.new('/some/nonexistent/basedir',
+                                             {puppetfile_path: 'relative/Puppetfile'})
+      expect(relative_subject.puppetfile_path).to eq '/some/nonexistent/basedir/relative/Puppetfile'
+    end
+  end
+
+  describe "a custom absolute puppetfile_path" do
+    it "is the puppetfile_path as given" do
+      absolute_subject = described_class.new('/some/nonexistent/basedir',
+                                             {puppetfile_path: '/some/absolute/custom/Puppetfile'})
+      expect(absolute_subject.puppetfile_path).to eq '/some/absolute/custom/Puppetfile'
+    end
+  end
+end
+
 describe R10K::Puppetfile do
 
   subject do