Merge pull request #1214 from Magisus/forge-puppetfile

(RK-308) Allow respecting `forge` declaration in puppetfile

CHANGELOG.mkd

@@ -4,6 +4,7 @@ CHANGELOG
 Unreleased
 ----------
 
+- (RK-308) Provide a `forge.allow_puppetfile_override` setting that, when true, causes a `forge` declaration in the Puppetfile to override `forge.baseurl`. [#1214](https://github.com/puppetlabs/r10k/pull/1214)
 - (CODEMGMT-1415) Provide a `--incremental` flag to only sync those modules in a Puppetfile whose definitions have changed since last sync, or those whose versions could change. [#1200](https://github.com/puppetlabs/r10k/pull/1200)
 - (CODEMGMT-1454) Ensure missing repo caches are re-synced [#1210](https://github.com/puppetlabs/r10k/pull/1210)
 - (PF-2437) Allow token authentication to be used with the Forge. [#1192](https://github.com/puppetlabs/r10k/pull/1192)

doc/dynamic-environments/configuration.mkd

@@ -176,6 +176,11 @@ forge:
   authorization_token: 'Bearer mysupersecretauthtoken'
 ```
 
+#### allow_puppetfile_override
+
+The `allow_puppetfile_override` setting causes r10k to respect [`forge` declarations](https://github.com/puppetlabs/r10k/blob/main/doc/puppetfile.mkd#forge)
+in Puppetfiles, overriding the `baseurl` setting and allowing per-environment configuration of the Forge URL.
+
 Deployment options
 ------------------
 

doc/puppetfile.mkd

@@ -51,10 +51,9 @@ handles modules.
 ### forge
 
 The `forge` setting specifies which server that Forge based modules are fetched
-from. This is currently a noop and is provided for compatibility with
-librarian-puppet.
-
-R10k supports setting the Forge to use _globally_ in `r10k.yaml`. see [Configuration](/doc/dynamic-environments/configuration.mkd#baseurl) for details.
+from. This declaration is only respected if [`forge.allow_puppetfile_override`](/dynamic-environments/configuration.mkd#allow_puppetfile_override)
+is set to true in the main `r10k.yaml`. Otherwise, use [`forge.baseurl`](/doc/dynamic-environments/configuration.mkd#baseurl)
+to globally configure where modules should be downloaded from.
 
 ### moduledir
 

lib/r10k/action/deploy/environment.rb

@@ -56,6 +56,9 @@ def initialize(opts, argv, settings = {})
                 purge_allowlist: read_purge_allowlist(settings.dig(:deploy, :purge_whitelist) || [],
                                                       settings.dig(:deploy, :purge_allowlist) || [])
               },
+              forge: {
+                allow_puppetfile_override: settings.dig(:forge, :allow_puppetfile_override) || false
+              },
               output: {}
             }
           })

lib/r10k/action/deploy/module.rb

@@ -44,6 +44,9 @@ def initialize(opts, argv, settings = {})
                 # force here is used to make it easier to reason about
                 force: !@no_force
               },
+              forge: {
+                allow_puppetfile_override: settings.dig(:forge, :allow_puppetfile_override) || false
+              },
               purging: {},
               output: {}
             }

lib/r10k/module_loader/puppetfile.rb

@@ -9,7 +9,6 @@ class Puppetfile
 
       DEFAULT_MODULEDIR = 'modules'
       DEFAULT_PUPPETFILE_NAME = 'Puppetfile'
-      DEFAULT_FORGE_API = 'forgeapi.puppetlabs.com'
 
       attr_accessor :default_branch_override, :environment
       attr_reader :modules, :moduledir, :puppetfile_path,
@@ -29,17 +28,16 @@ class Puppetfile
       def initialize(basedir:,
                      moduledir: DEFAULT_MODULEDIR,
                      puppetfile: DEFAULT_PUPPETFILE_NAME,
-                     forge: DEFAULT_FORGE_API,
                      overrides: {},
                      environment: nil)
 
         @basedir     = cleanpath(basedir)
         @moduledir   = resolve_path(@basedir, moduledir)
         @puppetfile_path  = resolve_path(@basedir, puppetfile)
-        @forge       = forge
         @overrides   = overrides
         @environment = environment
         @default_branch_override = @overrides.dig(:environments, :default_branch_override)
+        @allow_puppetfile_forge = @overrides.dig(:forge, :allow_puppetfile_override)
 
         @existing_module_metadata = []
         @existing_module_versions_by_name = {}
@@ -113,7 +111,12 @@ def add_module_metadata(name, info)
 
       # @param [String] forge
       def set_forge(forge)
-        @forge = forge
+        if @allow_puppetfile_forge
+          logger.debug _("Using Forge from Puppetfile: %{forge}") % { forge: forge }
+          PuppetForge.host = forge
+        else
+          logger.debug _("Ignoring Forge declaration in Puppetfile, using value from settings: %{forge}.") % { forge: PuppetForge.host }
+        end
       end
 
       # @param [String] moduledir

lib/r10k/puppetfile.rb

@@ -78,7 +78,6 @@ def initialize(basedir, options_or_moduledir = nil, deprecated_path_arg = nil, d
       basedir: @basedir,
       moduledir: @moduledir,
       puppetfile: @puppetfile,
-      forge: @forge,
       overrides: @overrides,
       environment: @environment
     )

lib/r10k/settings.rb

@@ -126,8 +126,19 @@ def self.forge_settings
         URIDefinition.new(:baseurl, {
           :desc => "The URL to the Puppet Forge to use for downloading modules."
         }),
+
         Definition.new(:authorization_token, {
           :desc => "The token for Puppet Forge authorization. Leave blank for unauthorized or license-based connections."
+        }),
+
+        Definition.new(:allow_puppetfile_override, {
+          :desc => "Whether to use `forge` declarations in the Puppetfile as an override of `baseurl`.",
+          :default => false,
+          :validate => lambda do |value|
+            unless !!value == value
+              raise ArgumentError, "`allow_puppetfile_override` can only be a boolean value, not '#{value}'"
+            end
+          end
         })
       ])
     end

spec/fixtures/unit/puppetfile/forge-override/Puppetfile

@@ -0,0 +1,8 @@
+forge "my.custom.forge.com"
+
+mod "puppetlabs/stdlib", '4.12.0'
+mod "puppetlabs/concat", '2.1.0'
+
+mod 'apache',
+  :git    => 'https://github.com/puppetlabs/puppetlabs-apache',
+  :branch => 'docs_experiment'

spec/unit/module_loader/puppetfile_spec.rb

@@ -8,7 +8,6 @@
       let(:options) do
         {
           basedir: '/test/basedir/env',
-          forge: 'localforge.internal.corp',
           overrides: { modules: { deploy_modules: true } },
           environment: R10K::Environment::Git.new('env',
                                                   '/test/basedir/',
@@ -48,10 +47,6 @@
         end
       end
 
-      it 'the forge' do
-        expect(subject.instance_variable_get(:@forge)).to eq('localforge.internal.corp')
-      end
-
       it 'the overrides' do
         expect(subject.instance_variable_get(:@overrides)).to eq({ modules: { deploy_modules: true }})
       end
@@ -72,10 +67,6 @@
         expect(subject.instance_variable_get(:@puppetfile_path)).to eq('/test/basedir/Puppetfile')
       end
 
-      it 'uses the public forge' do
-        expect(subject.instance_variable_get(:@forge)).to eq('forgeapi.puppetlabs.com')
-      end
-
       it 'creates an empty overrides' do
         expect(subject.instance_variable_get(:@overrides)).to eq({})
       end
@@ -276,6 +267,27 @@ def expect_wrapped_error(error, pf_path, error_type)
       end
     end
 
+    describe 'forge declaration' do
+      before(:each) do
+        PuppetForge.host = ""
+      end
+
+      it 'is respected if `allow_puppetfile_override` is true' do
+        @path = File.join(PROJECT_ROOT, 'spec', 'fixtures', 'unit', 'puppetfile', 'forge-override')
+        puppetfile = R10K::ModuleLoader::Puppetfile.new(basedir: @path, overrides: { forge: { allow_puppetfile_override: true } })
+        puppetfile.load!
+        expect(PuppetForge.host).to eq("my.custom.forge.com/")
+      end
+
+      it 'is ignored if `allow_puppetfile_override` is false' do
+        @path = File.join(PROJECT_ROOT, 'spec', 'fixtures', 'unit', 'puppetfile', 'forge-override')
+        puppetfile = R10K::ModuleLoader::Puppetfile.new(basedir: @path, overrides: { forge: { allow_puppetfile_override: false } })
+        expect(PuppetForge).not_to receive(:host=).with("my.custom.forge.com")
+        puppetfile.load!
+        expect(PuppetForge.host).to eq("/")
+      end
+    end
+
     it 'rejects Puppetfiles with duplicate module names' do
       @path = File.join(PROJECT_ROOT, 'spec', 'fixtures', 'unit', 'puppetfile', 'duplicate-module-error')
       pf_path = File.join(@path, 'Puppetfile')

spec/unit/settings_spec.rb

@@ -90,6 +90,27 @@
         end
       end
     end
+
+    describe "allow_puppetfile_override" do
+      it 'is false by default' do
+        expect(subject.evaluate({})[:allow_puppetfile_override]).to eq(false)
+      end
+
+      it 'can be set to true' do
+        expect(subject.evaluate({"allow_puppetfile_override" => true})[:allow_puppetfile_override]).to eq(true)
+      end
+
+      it "raises an error for non-boolean values" do
+        expect {
+          subject.evaluate({"allow_puppetfile_override" => 'invalid_string'})
+        }.to raise_error do |err|
+          expect(err.message).to match(/Validation failed for 'forge' settings group/)
+          expect(err.errors.size).to eq 1
+          expect(err.errors[:allow_puppetfile_override]).to be_a_kind_of(ArgumentError)
+          expect(err.errors[:allow_puppetfile_override].message).to match(/`allow_puppetfile_override` can only be a boolean value, not 'invalid_string'/)
+        end
+      end
+    end
   end
 
   describe "deploy settings" do
@@ -270,10 +291,12 @@
       it "passes settings through to the forge settings" do
         output = subject.evaluate("forge" => {"baseurl" => "https://forge.tessier-ashpool.freeside",
                                               "proxy" => "https://proxy.tessier-ashpool.freesize:3128",
-                                              "authorization_token" => "faketoken"})
+                                              "authorization_token" => "faketoken",
+                                              "allow_puppetfile_override" => true})
         expect(output[:forge]).to eq(:baseurl => "https://forge.tessier-ashpool.freeside",
                                      :proxy => "https://proxy.tessier-ashpool.freesize:3128",
-                                     :authorization_token => "faketoken")
+                                     :authorization_token => "faketoken",
+                                     :allow_puppetfile_override => true)
       end
     end
   end