(PUP-3239) Stop compilation if $environment has bad manifest settings

lib/puppet/node/environment.rb

@@ -250,14 +250,25 @@ def full_modulepath
   #   Puppet[:disable_per_environment_manifest] is true, and this environment's
   #   original environment.conf had a manifest setting that is not the
   #   Puppet[:default_manifest].
-  # @api public
+  # @api private 
   def conflicting_manifest_settings?
     return false if Puppet[:environmentpath].empty? || !Puppet[:disable_per_environment_manifest]
     environment_conf = Puppet.lookup(:environments).get_conf(name)
     original_manifest = environment_conf.raw_setting(:manifest)
     !original_manifest.nil? && !original_manifest.empty? && original_manifest != Puppet[:default_manifest]
   end
 
+  # Checks the environment and settings for any conflicts
+  # @return [Array<String>] an array of validation errors
+  # @api public
+  def validation_errors
+    errors = []
+    if conflicting_manifest_settings?
+      errors << "The 'disable_per_environment_manifest' setting is true, and the '#{name}' environment has an environment.conf manifest that conflicts with the 'default_manifest' setting."
+    end
+    errors
+  end
+
   # Return an environment-specific Puppet setting.
   #
   # @api public

lib/puppet/parser/compiler.rb

@@ -20,13 +20,12 @@ def self.compile(node)
     $env_module_directories = nil
     node.environment.check_for_reparse
 
-    if node.environment.conflicting_manifest_settings?
+    errors = node.environment.validation_errors
+    if !errors.empty?
+      errors.each { |e| Puppet.err(e) } if errors.size > 1
       errmsg = [
-        "The 'disable_per_environment_manifest' setting is true, and this '#{node.environment}'",
-        "has an environment.conf manifest that conflicts with the 'default_manifest' setting.",
-        "Compilation has been halted in order to avoid running a catalog which may be using",
-        "unexpected manifests. For more information, see",
-        "http://docs.puppetlabs.com/puppet/latest/reference/environments.html",
+        "Compilation has been halted because: #{errors.first}",
+        "For more information, see http://docs.puppetlabs.com/puppet/latest/reference/environments.html",
       ]
       raise(Puppet::Error, errmsg.join(' '))
     end

spec/unit/node/environment_spec.rb

@@ -187,8 +187,8 @@
       end
     end
 
-    it "does not register conflicting_manifest_settings? when not using directory environments" do
-      expect(Puppet::Node::Environment.create(:directory, [], '/some/non/default/manifest.pp').conflicting_manifest_settings?).to be_false
+    it "does not register validation_errors when not using directory environments" do
+      expect(Puppet::Node::Environment.create(:directory, [], '/some/non/default/manifest.pp').validation_errors).to be_empty
     end
 
     describe "when operating in the context of directory environments" do
@@ -197,8 +197,8 @@
         Puppet[:default_manifest] = "/default/manifests/site.pp"
       end
 
-      it "has no conflicting_manifest_settings? when disable_per_environment_manifest is false" do
-        expect(Puppet::Node::Environment.create(:directory, [], '/some/non/default/manifest.pp').conflicting_manifest_settings?).to be_false
+      it "has no validation errors when disable_per_environment_manifest is false" do
+        expect(Puppet::Node::Environment.create(:directory, [], '/some/non/default/manifest.pp').validation_errors).to be_empty
       end
 
       context "when disable_per_environment_manifest is true" do
@@ -219,7 +219,11 @@ def assert_manifest_conflict(expectation, envconf_manifest_value)
           loader.stubs(:get_conf).returns(envconf)
 
           Puppet.override(:environments => loader) do
-            expect(environment.conflicting_manifest_settings?).to eq(expectation)
+            if expectation
+              expect(environment.validation_errors).to have_matching_element(/The 'disable_per_environment_manifest' setting is true.*and the.*environment.*conflicts/)
+            else
+              expect(environment.validation_errors).to be_empty
+            end
           end
         end
 

spec/unit/parser/compiler_spec.rb

@@ -97,11 +97,11 @@ def resource(type, title)
     @compiler.environment.should equal(@node.environment)
   end
 
-  it "fails if the node's environment has conflicting manifest settings" do
+  it "fails if the node's environment has validation errors" do
     conflicted_environment = Puppet::Node::Environment.create(:testing, [], '/some/environment.conf/manifest.pp')
-    conflicted_environment.stubs(:conflicting_manifest_settings?).returns(true)
+    conflicted_environment.stubs(:validation_errors).returns(['bad environment'])
     @node.environment = conflicted_environment
-    expect { Puppet::Parser::Compiler.compile(@node) }.to raise_error(Puppet::Error, /disable_per_environment_manifest.*true.*environment.conf.*manifest.*conflict/)
+    expect { Puppet::Parser::Compiler.compile(@node) }.to raise_error(Puppet::Error, /Compilation has been halted because.*bad environment/)
   end
 
   it "should include the resource type collection helper" do