diff --git a/CHANGELOG.mkd b/CHANGELOG.mkd index 536398b6c..ae00799fd 100644 --- a/CHANGELOG.mkd +++ b/CHANGELOG.mkd @@ -5,6 +5,7 @@ Unreleased ---------- - Fetch modules from the repo's default branch, rather than defaulting to 'master' [#1096](https://github.com/puppetlabs/r10k/issues/1096) +- Experimental feature change: conflicts between environment-defined modules and Puppetfile-defined modules now default to logging a warning and deploying the environment module version, overriding the Puppetfile. Previously, conflicts would result in an error. The behavior is now configurable via the `module_conflicts` environment setting [#1107](https://github.com/puppetlabs/r10k/pull/1107) 3.7.0 ----- diff --git a/doc/dynamic-environments/configuration.mkd b/doc/dynamic-environments/configuration.mkd index 84bc2fb3f..6066e1f7f 100644 --- a/doc/dynamic-environments/configuration.mkd +++ b/doc/dynamic-environments/configuration.mkd @@ -673,6 +673,31 @@ modules: ref: 62d07f2 ``` +#### Puppetfile module conflicts + +When a module is defined in an environment and also in a Puppetfile, the default behavior is for the environment definition of the module to take precedence, a warning to be logged, and the Puppetfile definition to be ignored. The behavior is configurable to optionally skip the warning, or allow a hard failure instead. Use the `module_conflicts` option in an environment definition to control this. + +Available `module_conflicts` options: + +* `override_puppetfile_and_warn` (default): the version of the module defined by the environment will be used, and the version defined in the Puppetfile will be ignored. A warning will be printed. +* `override_puppetfile`: the version of the module defined by the environment will be used, and the version defined in the Puppetfile will be ignored. +* `error`: an error will be raised alerting the user to the conflict. The environment will not be deployed. + +```yaml +# production.yaml +--- +type: git +remote: git@github.com:puppetlabs/control-repo.git +ref: 8820892 +module_conflicts: override_puppetfile_and_warn +modules: + puppetlabs-stdlib: 6.0.0 + puppetlabs-concat: 6.1.0 + reidmv-xampl: + git: https://github.com/reidmv/reidmv-xampl.git + ref: 62d07f2 +``` + ### Bare Environment Type A "control repository" typically contains a hiera.yaml, an environment.conf, a manifests/site.pp file, and a few other things. However, none of these are strictly necessary for an environment to be functional if modules can be deployed to it. diff --git a/lib/r10k/environment/with_modules.rb b/lib/r10k/environment/with_modules.rb index 20558c7c4..be9b504ba 100644 --- a/lib/r10k/environment/with_modules.rb +++ b/lib/r10k/environment/with_modules.rb @@ -49,7 +49,9 @@ def modules return @modules if @puppetfile.nil? @puppetfile.load unless @puppetfile.loaded? - @modules + @puppetfile.modules + + env_mod_names = @modules.map(&:name) + @modules + @puppetfile.modules.select { |mod| !env_mod_names.include?(mod.name) } end def accept(visitor) @@ -101,10 +103,22 @@ def validate_no_module_conflicts .select { |_, v| v.size > 1 } .map(&:first) unless conflicts.empty? - msg = _('Puppetfile cannot contain module names defined by environment %{name}') % {name: self.name} - msg += ' ' - msg += _("Remove the conflicting definitions of the following modules: %{conflicts}" % { conflicts: conflicts.join(' ') }) - raise R10K::Error.new(msg) + conflicts_str = conflicts.join(', ') + log_msg = _('Environment and Puppetfile both define the following modules, Puppetfile ' \ + 'definition will be ignored: %{mods}' % { mods: conflicts_str }) + case conflict_opt = @options[:module_conflicts] + when 'override_puppetfile' + logger.debug log_msg + when 'override_puppetfile_and_warn', nil + logger.warn log_msg + when 'error' + raise R10K::Error, _('Puppetfile cannot contain module names defined by environment ' \ + '%{env}; Remove the conflicting definitions of the following modules: ' \ + '%{mods}' % { env: self.name, mods: conflicts_str }) + else + raise R10K::Error, _('Unexpected value for `module_conflicts` setting in %{env} ' \ + 'environment: %{val}' % {env: self.name, val: conflict_opt}) + end end end diff --git a/spec/unit/environment/git_spec.rb b/spec/unit/environment/git_spec.rb index 1c7ba6bab..f5d1794ed 100644 --- a/spec/unit/environment/git_spec.rb +++ b/spec/unit/environment/git_spec.rb @@ -62,9 +62,10 @@ describe "enumerating modules" do it "loads the Puppetfile and returns modules in that puppetfile" do + mod = double('A module', :name => 'dbl') expect(subject.puppetfile).to receive(:load) - expect(subject.puppetfile).to receive(:modules).and_return [:modules] - expect(subject.modules).to eq([:modules]) + expect(subject.puppetfile).to receive(:modules).and_return [mod] + expect(subject.modules).to eq([mod]) end end diff --git a/spec/unit/environment/with_modules_spec.rb b/spec/unit/environment/with_modules_spec.rb new file mode 100644 index 000000000..7fe3a3ee6 --- /dev/null +++ b/spec/unit/environment/with_modules_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' +require 'r10k/environment' + +describe R10K::Environment::WithModules do + subject do + described_class.new( + 'release42', + '/some/nonexistent/environmentdir', + 'prefix_release42', + { + :type => 'bare', + :modules => { + 'puppetlabs-stdlib' => '6.0.0', + 'puppetlabs-concat' => '6.1.0', + 'puppetlabs-exec' => '0.5.0', + } + }.merge(subject_params) + ) + end + + # Default no additional params + let(:subject_params) { {} } + + describe "dealing with Puppetfile module conflicts" do + context "with no module conflicts" do + it "validates when there are no conflicts" do + mod = double('module', :name => 'nonconflict') + expect(subject.puppetfile).to receive(:load) + expect(subject.puppetfile).to receive(:modules).and_return [mod] + expect { subject.validate_no_module_conflicts }.not_to raise_error + end + end + + context "with module conflicts and default behavior" do + it "does not raise an error" do + mod = double('duplicate-stdlib', :name => 'stdlib') + expect(subject.puppetfile).to receive(:load) + expect(subject.puppetfile).to receive(:modules).and_return [mod] + expect(subject.logger).to receive(:warn).with(/Puppetfile.*both define.*ignored/i) + expect { subject.validate_no_module_conflicts }.not_to raise_error + end + end + + context "with module conflicts and 'error' behavior" do + let(:subject_params) {{ :module_conflicts => 'error' }} + it "raises an error" do + mod = double('duplicate-stdlib', :name => 'stdlib') + expect(subject.puppetfile).to receive(:load) + expect(subject.puppetfile).to receive(:modules).and_return [mod] + expect { subject.validate_no_module_conflicts }.to raise_error(R10K::Error, /Puppetfile.*defined.*conflict/i) + end + end + + context "with module conflicts and 'override_puppetfile' behavior" do + let(:subject_params) {{ :module_conflicts => 'override_puppetfile' }} + it "does not raise an error" do + mod = double('duplicate-stdlib', :name => 'stdlib') + expect(subject.puppetfile).to receive(:load) + expect(subject.puppetfile).to receive(:modules).and_return [mod] + expect(subject.logger).to receive(:debug).with(/Puppetfile.*both define.*ignored/i) + expect { subject.validate_no_module_conflicts }.not_to raise_error + end + end + + context "with module conflicts and invalid configuration" do + let(:subject_params) {{ :module_conflicts => 'batman' }} + it "raises an error" do + mod = double('duplicate-stdlib', :name => 'stdlib') + expect(subject.puppetfile).to receive(:load) + expect(subject.puppetfile).to receive(:modules).and_return [mod] + expect { subject.validate_no_module_conflicts }.to raise_error(R10K::Error, /Unexpected value.*module_conflicts/i) + end + end + end + + describe "modules method" do + it "overrides duplicates, choosing the environment version" do + mod = double('duplicate-stdlib', :name => 'stdlib', :giveaway => :'i-am-a-double') + expect(subject.puppetfile).to receive(:load) + expect(subject.puppetfile).to receive(:modules).and_return [mod] + returned = subject.modules + expect(returned.map(&:name).sort).to eq(%w[concat exec stdlib]) + + # Make sure the module that was picked was the environment one, not the Puppetfile one + stdlib = returned.find { |m| m.name == 'stdlib' } + expect(stdlib.respond_to?(:giveaway)).to eq(false) + end + end +end