diff --git a/CHANGELOG.mkd b/CHANGELOG.mkd index ceb8cdfa3..15c9a7818 100644 --- a/CHANGELOG.mkd +++ b/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) diff --git a/doc/dynamic-environments/configuration.mkd b/doc/dynamic-environments/configuration.mkd index 2a1bb2f9e..5c06a35eb 100644 --- a/doc/dynamic-environments/configuration.mkd +++ b/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 ------------------ diff --git a/doc/puppetfile.mkd b/doc/puppetfile.mkd index 0a0619e5a..13d3cff1a 100644 --- a/doc/puppetfile.mkd +++ b/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 diff --git a/lib/r10k/action/deploy/environment.rb b/lib/r10k/action/deploy/environment.rb index 09fcffb68..2e68befbe 100644 --- a/lib/r10k/action/deploy/environment.rb +++ b/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: {} } }) diff --git a/lib/r10k/action/deploy/module.rb b/lib/r10k/action/deploy/module.rb index 96bc50795..1c1ffd0cf 100644 --- a/lib/r10k/action/deploy/module.rb +++ b/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: {} } diff --git a/lib/r10k/module_loader/puppetfile.rb b/lib/r10k/module_loader/puppetfile.rb index 50f4814d6..4b61e42ea 100644 --- a/lib/r10k/module_loader/puppetfile.rb +++ b/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 diff --git a/lib/r10k/puppetfile.rb b/lib/r10k/puppetfile.rb index 19527b5af..d4cfd895b 100644 --- a/lib/r10k/puppetfile.rb +++ b/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 ) diff --git a/lib/r10k/settings.rb b/lib/r10k/settings.rb index 3415cee78..e4d694641 100644 --- a/lib/r10k/settings.rb +++ b/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 diff --git a/spec/fixtures/unit/puppetfile/forge-override/Puppetfile b/spec/fixtures/unit/puppetfile/forge-override/Puppetfile new file mode 100644 index 000000000..f4f73d740 --- /dev/null +++ b/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' diff --git a/spec/unit/module_loader/puppetfile_spec.rb b/spec/unit/module_loader/puppetfile_spec.rb index ce3c20063..fcc99c466 100644 --- a/spec/unit/module_loader/puppetfile_spec.rb +++ b/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') diff --git a/spec/unit/settings_spec.rb b/spec/unit/settings_spec.rb index e03786b40..6acc34794 100644 --- a/spec/unit/settings_spec.rb +++ b/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