Merge pull request #3032 from jpartlow/feature/master/pup-3069-add-defaultmanifest

Feature/master/pup 3069 add defaultmanifest

13 files changed, 556 insertions, 27 deletions

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 4d85f7de4..b2cb92975 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -1042,6 +1042,44 @@ EOT
         http://docs.puppetlabs.com/puppet/latest/reference/environments.html",
       :deprecated => :allowed_on_commandline,
     },
+    :default_manifest => {
+      :default    => "./manifests",
+      :type       => :string,
+      :desc       => "The default main manifest for directory environments. Any environment that
+        doesn't set the `manifest` setting in its `environment.conf` file will use
+        this manifest.
+
+        This setting's value can be an absolute or relative path. An absolute path
+        will make all environments default to the same main manifest; a relative
+        path will allow each environment to use its own manifest, and Puppet will
+        resolve the path relative to each environment's main directory.
+
+        In either case, the path can point to a single file or to a directory of
+        manifests to be evaluated in alphabetical order.",
+      :hook       => proc do |value|
+        uninterpolated_value = self.value(true)
+        if uninterpolated_value =~ /\$environment/ || value =~ /\$environment/ then
+          raise(Puppet::Settings::ValidationError,
+                "You cannot interpolate '$environment' within the 'default_manifest' setting.")
+        end
+      end
+    },
+    :disable_per_environment_manifest => {
+      :default    => false,
+      :type       => :boolean,
+      :desc       => "Whether to disallow an environment-specific main manifest. When set
+        to `true`, Puppet will use the manifest specified in the `default_manifest` setting
+        for all environments. If an environment specifies a different main manifest in its
+        `environment.conf` file, catalog requests for that environment will fail with an error.
+
+        This setting requires `default_manifest` to be set to an absolute path.",
+      :hook       => proc do |value|
+        if value && !Pathname.new(Puppet[:default_manifest]).absolute?
+          raise(Puppet::Settings::ValidationError,
+                "The 'default_manifest' setting must be set to an absolute path when 'disable_per_environment_manifest' is true")
+        end
+      end,
+    },
     :code => {
       :default    => "",
       :desc       => "Code to parse directly.  This is essentially only used
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index c8b01acee..5a0eed5e1 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -237,6 +237,27 @@ class Puppet::Node::Environment
   #     (optional)
   attr_reader :config_version
 
+  # Checks to make sure that this environment did not have a manifest set in
+  # its original environment.conf if Puppet is configured with
+  # +disable_per_environment_manifest+ set true.  If it did, the environment's
+  # modules may not function as intended by the original authors, and we may
+  # seek to halt a puppet compilation for a node in this environment.
+  #
+  # The only exception to this would be if the environment.conf manifest is an exact,
+  # uninterpolated match for the current +default_manifest+ setting.
+  #
+  # @return [Boolean] true if using directory environments, and
+  #   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
+  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
+
   # Return an environment-specific Puppet setting.
   #
   # @api public
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index ed30655f6..5df2916fc 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -20,6 +20,17 @@ class Puppet::Parser::Compiler
     $env_module_directories = nil
     node.environment.check_for_reparse
 
+    if node.environment.conflicting_manifest_settings?
+      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",
+      ]
+      raise(Puppet::Error, errmsg.join(' '))
+    end
+
     new(node).compile.to_resource
   rescue => detail
     message = "#{detail} on node #{node.name}"
diff --git a/lib/puppet/settings.rb b/lib/puppet/settings.rb
index b333c0e14..3201b125f 100644
--- a/lib/puppet/settings.rb
+++ b/lib/puppet/settings.rb
@@ -199,6 +199,7 @@ class Puppet::Settings
     @value_sets[:memory] = Values.new(:memory, @config)
     @value_sets[:overridden_defaults] = Values.new(:overridden_defaults, @config)
 
+    @deprecated_settings_that_have_been_configured.clear
     @values.clear
     @cache.clear
   end
@@ -541,12 +542,12 @@ class Puppet::Settings
     # If we get here and don't have any data, we just return and don't muck with the current state of the world.
     return if data.nil?
 
-    record_deprecations_from_puppet_conf(data)
-
-    # If we get here then we have some data, so we need to clear out any previous settings that may have come from
-    #  config files.
+    # If we get here then we have some data, so we need to clear out any
+    # previous settings that may have come from config files.
     unsafe_clear(false, false)
 
+    record_deprecations_from_puppet_conf(data)
+
     # And now we can repopulate with the values from our last parsing of the config files.
     @configuration_file = data
 
@@ -1196,6 +1197,7 @@ Generated on #{Time.now}.
   # @return nil
   def clear_everything_for_tests()
     unsafe_clear(true, true)
+    @configuration_file = nil
     @global_defaults_initialized = false
     @app_defaults_initialized = false
   end
diff --git a/lib/puppet/settings/base_setting.rb b/lib/puppet/settings/base_setting.rb
index d18217646..e7c8cf17f 100644
--- a/lib/puppet/settings/base_setting.rb
+++ b/lib/puppet/settings/base_setting.rb
@@ -155,9 +155,12 @@ class Puppet::Settings::BaseSetting
     str.gsub(/^/, "    ")
   end
 
-  # Retrieves the value, or if it's not set, retrieves the default.
-  def value
-    @settings.value(self.name)
+  # @param bypass_interpolation [Boolean] Set this true to skip the
+  #   interpolation step, returning the raw setting value.  Defaults to false.
+  # @return [String] Retrieves the value, or if it's not set, retrieves the default.
+  # @api public
+  def value(bypass_interpolation = false)
+    @settings.value(self.name, nil, bypass_interpolation)
   end
 
   # Modify the value when it is first evaluated
diff --git a/lib/puppet/settings/environment_conf.rb b/lib/puppet/settings/environment_conf.rb
index 187f963d7..c6fe42c66 100644
--- a/lib/puppet/settings/environment_conf.rb
+++ b/lib/puppet/settings/environment_conf.rb
@@ -51,8 +51,32 @@ class Puppet::Settings::EnvironmentConf
   end
 
   def manifest
-    get_setting(:manifest, File.join(@path_to_env, "manifests")) do |manifest|
-      absolute(manifest)
+    puppet_conf_manifest = Pathname.new(Puppet.settings.value(:default_manifest))
+    disable_per_environment_manifest = Puppet.settings.value(:disable_per_environment_manifest)
+
+    fallback_manifest_directory =
+    if puppet_conf_manifest.absolute?
+      puppet_conf_manifest.to_s
+    else
+      File.join(@path_to_env, puppet_conf_manifest.to_s)
+    end
+
+    if disable_per_environment_manifest
+      environment_conf_manifest = absolute(raw_setting(:manifest))
+      if environment_conf_manifest && fallback_manifest_directory != environment_conf_manifest
+        errmsg = ["The 'disable_per_environment_manifest' setting is true, but the",
+        "environment located at #{@path_to_env} has a manifest setting in its",
+        "environment.conf of '#{environment_conf_manifest}' which does not match",
+        "the default_manifest setting '#{puppet_conf_manifest}'. If this",
+        "environment is expecting to find modules in",
+        "'#{environment_conf_manifest}', they will not be available!"]
+        Puppet.err(errmsg.join(' '))
+      end
+      fallback_manifest_directory.to_s
+    else
+      get_setting(:manifest, fallback_manifest_directory) do |manifest|
+        absolute(manifest)
+      end
     end
   end
 
@@ -82,6 +106,11 @@ class Puppet::Settings::EnvironmentConf
     end
   end
 
+  def raw_setting(setting_name)
+    setting = section.setting(setting_name) if section
+    setting.value if setting
+  end
+
   private
 
   def self.validate(path_to_conf_file, config)
@@ -103,8 +132,7 @@ class Puppet::Settings::EnvironmentConf
   end
 
   def get_setting(setting_name, default = nil)
-    setting = section.setting(setting_name) if section
-    value = setting.value if setting
+    value = raw_setting(setting_name)
     value ||= default
     yield value
   end
diff --git a/spec/integration/defaults_spec.rb b/spec/integration/defaults_spec.rb
index 8c8432b6f..734785230 100755
--- a/spec/integration/defaults_spec.rb
+++ b/spec/integration/defaults_spec.rb
@@ -5,6 +5,32 @@ require 'puppet/defaults'
 require 'puppet/rails'
 
 describe "Puppet defaults" do
+
+  describe "when default_manifest is set" do
+    it "returns ./manifests by default" do
+      expect(Puppet[:default_manifest]).to eq('./manifests')
+    end
+
+    it "errors when $environment is part of the value" do
+      expect {
+        Puppet[:default_manifest] = '/$environment/manifest.pp'
+      }.to raise_error Puppet::Settings::ValidationError, /cannot interpolate.*\$environment/
+    end
+  end
+
+  describe "when disable_per_environment_manifest is set" do
+    it "returns false by default" do
+      expect(Puppet[:disable_per_environment_manifest]).to eq(false)
+    end
+
+    it "errors when set to true and default_manifest is not an absolute path" do
+      expect {
+        Puppet[:default_manifest] = './some/relative/manifest.pp'
+        Puppet[:disable_per_environment_manifest] = true
+      }.to raise_error Puppet::Settings::ValidationError, /'default_manifest' setting must be.*absolute/
+    end
+  end
+
   describe "when setting the :factpath" do
     it "should add the :factpath to Facter's search paths" do
       Facter.expects(:search).with("/my/fact/path")
diff --git a/spec/integration/environments/default_manifest_spec.rb b/spec/integration/environments/default_manifest_spec.rb
new file mode 100644
index 000000000..e6d10f3b2
--- /dev/null
+++ b/spec/integration/environments/default_manifest_spec.rb
@@ -0,0 +1,272 @@
+require 'spec_helper'
+
+describe "default manifests" do
+  shared_examples_for "puppet with default_manifest settings" do
+    let(:confdir) { Puppet[:confdir] }
+    let(:environmentpath) { File.expand_path("envdir", confdir) }
+
+    FS = Puppet::FileSystem
+
+    context "relative default" do
+      let(:testingdir) { File.join(environmentpath, "testing") }
+
+      before(:each) do
+        FileUtils.mkdir_p(testingdir)
+      end
+
+      it "reads manifest from ./manifest of a basic directory environment" do
+        manifestsdir = File.join(testingdir, "manifests")
+        FileUtils.mkdir_p(manifestsdir)
+
+        File.open(File.join(manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromRelativeDefault': }")
+        end
+
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts("environmentpath=#{environmentpath}")
+        end
+
+        expect(a_catalog_compiled_for_environment('testing')).to(
+          include_resource('Notify[ManifestFromRelativeDefault]')
+        )
+      end
+    end
+
+    context "set absolute" do
+      let(:testingdir) { File.join(environmentpath, "testing") }
+
+      before(:each) do
+        FileUtils.mkdir_p(testingdir)
+      end
+
+      it "reads manifest from an absolute default_manifest" do
+        manifestsdir = File.expand_path("manifests", confdir)
+        FileUtils.mkdir_p(manifestsdir)
+
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts(<<-EOF)
+  environmentpath=#{environmentpath}
+  default_manifest=#{manifestsdir}
+          EOF
+        end
+
+        File.open(File.join(manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromAbsoluteDefaultManifest': }")
+        end
+
+        expect(a_catalog_compiled_for_environment('testing')).to(
+          include_resource('Notify[ManifestFromAbsoluteDefaultManifest]')
+        )
+      end
+
+      it "reads manifest from directory environment manifest when environment.conf manifest set" do
+        default_manifestsdir = File.expand_path("manifests", confdir)
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts(<<-EOF)
+  environmentpath=#{environmentpath}
+  default_manifest=#{default_manifestsdir}
+          EOF
+        end
+
+        manifestsdir = File.join(testingdir, "special_manifests")
+        FileUtils.mkdir_p(manifestsdir)
+
+        File.open(File.join(manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromEnvironmentConfManifest': }")
+        end
+
+        File.open(File.join(testingdir, "environment.conf"), "w") do |f|
+          f.puts("manifest=./special_manifests")
+        end
+
+        expect(a_catalog_compiled_for_environment('testing')).to(
+          include_resource('Notify[ManifestFromEnvironmentConfManifest]')
+        )
+        expect(Puppet[:default_manifest]).to eq(default_manifestsdir)
+      end
+
+      it "ignores manifests in the local ./manifests if default_manifest specifies another directory" do
+        default_manifestsdir = File.expand_path("manifests", confdir)
+        FileUtils.mkdir_p(default_manifestsdir)
+
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts(<<-EOF)
+  environmentpath=#{environmentpath}
+  default_manifest=#{default_manifestsdir}
+          EOF
+        end
+
+        File.open(File.join(default_manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromAbsoluteDefaultManifest': }")
+        end
+
+        implicit_manifestsdir = File.join(testingdir, "manifests")
+        FileUtils.mkdir_p(implicit_manifestsdir)
+
+        File.open(File.join(implicit_manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromImplicitRelativeEnvironmentManifestDirectory': }")
+        end
+
+        expect(a_catalog_compiled_for_environment('testing')).to(
+          include_resource('Notify[ManifestFromAbsoluteDefaultManifest]')
+        )
+      end
+
+      it "raises an exception if default_manifest has $environment in it" do
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts(<<-EOF)
+  environmentpath=#{environmentpath}
+  default_manifest=/foo/$environment
+          EOF
+        end
+
+        expect { Puppet.initialize_settings }.to raise_error(Puppet::Settings::ValidationError, /cannot interpolate.*\$environment.*in.*default_manifest/)
+      end
+    end
+
+    context "with disable_per_environment_manifest true" do
+      let(:manifestsdir) { File.expand_path("manifests", confdir) }
+      let(:testingdir) { File.join(environmentpath, "testing") }
+
+      before(:each) do
+        FileUtils.mkdir_p(testingdir)
+      end
+
+      before(:each) do
+        FileUtils.mkdir_p(manifestsdir)
+
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts(<<-EOF)
+  environmentpath=#{environmentpath}
+  default_manifest=#{manifestsdir}
+  disable_per_environment_manifest=true
+          EOF
+        end
+
+        File.open(File.join(manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromAbsoluteDefaultManifest': }")
+        end
+      end
+
+      it "reads manifest from the default manifest setting" do
+        expect(a_catalog_compiled_for_environment('testing')).to(
+          include_resource('Notify[ManifestFromAbsoluteDefaultManifest]')
+        )
+      end
+
+      it "refuses to compile if environment.conf specifies a different manifest" do
+        File.open(File.join(testingdir, "environment.conf"), "w") do |f|
+          f.puts("manifest=./special_manifests")
+        end
+
+        expect { a_catalog_compiled_for_environment('testing') }.to(
+          raise_error(Puppet::Error, /disable_per_environment_manifest.*environment.conf.*manifest.*conflict/)
+        )
+      end
+
+      it "reads manifest from default_manifest setting when environment.conf has manifest set if setting equals default_manifest setting" do
+        File.open(File.join(testingdir, "environment.conf"), "w") do |f|
+          f.puts("manifest=#{manifestsdir}")
+        end
+
+        expect(a_catalog_compiled_for_environment('testing')).to(
+          include_resource('Notify[ManifestFromAbsoluteDefaultManifest]')
+        )
+      end
+
+      it "logs errors if environment.conf specifies a different manifest" do
+        File.open(File.join(testingdir, "environment.conf"), "w") do |f|
+          f.puts("manifest=./special_manifests")
+        end
+
+        Puppet.initialize_settings
+        expect(Puppet[:environmentpath]).to eq(environmentpath)
+        environment = Puppet.lookup(:environments).get('testing')
+        expect(environment.manifest).to eq(manifestsdir)
+        expect(@logs.first.to_s).to match(%r{disable_per_environment_manifest.*is true, but.*environment.*at #{testingdir}.*has.*environment.conf.*manifest.*#{testingdir}/special_manifests})
+      end
+
+      it "raises an error if default_manifest is not absolute" do
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts(<<-EOF)
+  environmentpath=#{environmentpath}
+  default_manifest=./relative
+  disable_per_environment_manifest=true
+          EOF
+        end
+
+        expect { Puppet.initialize_settings }.to raise_error(Puppet::Settings::ValidationError, /default_manifest.*must be.*absolute.*when.*disable_per_environment_manifest.*true/)
+      end
+    end
+
+    context "in legacy environments" do
+      let(:environmentpath) { '' }
+      let(:manifestsdir) { File.expand_path("default_manifests", confdir) }
+      let(:legacy_manifestsdir) { File.expand_path('manifests', confdir) }
+
+      before(:each) do
+        FileUtils.mkdir_p(manifestsdir)
+
+        File.open(File.join(confdir, "puppet.conf"), "w") do |f|
+          f.puts(<<-EOF)
+  default_manifest=#{manifestsdir}
+  disable_per_environment_manifest=true
+  manifest=#{legacy_manifestsdir}
+          EOF
+        end
+
+        File.open(File.join(manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromAbsoluteDefaultManifest': }")
+        end
+      end
+
+      it "has no effect on compilation" do
+        FileUtils.mkdir_p(legacy_manifestsdir)
+
+        File.open(File.join(legacy_manifestsdir, "site.pp"), "w") do |f|
+          f.puts("notify { 'ManifestFromLegacy': }")
+        end
+
+        expect(a_catalog_compiled_for_environment('testing')).to(
+          include_resource('Notify[ManifestFromLegacy]')
+        )
+      end
+    end
+  end
+
+  describe 'using future parser' do
+    before :each do
+      Puppet[:parser] = 'future'
+    end
+    it_behaves_like 'puppet with default_manifest settings'
+  end
+
+  describe 'using current parser' do
+    before :each do
+      Puppet[:parser] = 'current'
+    end
+    it_behaves_like 'puppet with default_manifest settings'
+  end
+
+  RSpec::Matchers.define :include_resource do |expected|
+    match do |actual|
+      actual.resources.map(&:ref).include?(expected)
+    end
+
+    def failure_message_for_should
+      "expected #{@actual.resources.map(&:ref)} to include #{expected}"
+    end
+
+    def failure_message_for_should_not
+      "expected #{@actual.resources.map(&:ref)} not to include #{expected}"
+    end
+  end
+
+  def a_catalog_compiled_for_environment(envname)
+    Puppet.initialize_settings
+    expect(Puppet[:environmentpath]).to eq(environmentpath)
+    node = Puppet::Node.new('testnode', :environment => 'testing')
+    expect(node.environment).to eq(Puppet.lookup(:environments).get('testing'))
+    Puppet::Parser::Compiler.compile(node)
+  end
+end
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index a0688559e..3df5d329c 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -187,6 +187,60 @@ describe Puppet::Node::Environment do
       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
+    end
+
+    describe "when operating in the context of directory environments" do
+      before(:each) do
+        Puppet[:environmentpath] = "$confdir/environments"
+        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
+      end
+
+      context "when disable_per_environment_manifest is true" do
+        let(:config) { mock('config') }
+        let(:global_modulepath) { ["/global/modulepath"] }
+        let(:envconf) { Puppet::Settings::EnvironmentConf.new("/some/direnv", config, global_modulepath) }
+
+        before(:each) do
+          Puppet[:disable_per_environment_manifest] = true
+        end
+
+        def assert_manifest_conflict(expectation, envconf_manifest_value)
+          config.expects(:setting).with(:manifest).returns(
+            mock('setting', :value => envconf_manifest_value)
+          )
+          environment = Puppet::Node::Environment.create(:directory, [], '/default/manifests/site.pp')
+          loader = Puppet::Environments::Static.new(environment)
+          loader.stubs(:get_conf).returns(envconf)
+
+          Puppet.override(:environments => loader) do
+            expect(environment.conflicting_manifest_settings?).to eq(expectation)
+          end
+        end
+
+        it "has conflicting_manifest_settings when environment.conf manifest was set" do
+          assert_manifest_conflict(true, '/some/envconf/manifest/site.pp')
+        end
+
+        it "does not have conflicting_manifest_settings when environment.conf manifest is empty" do
+          assert_manifest_conflict(false, '')
+        end
+
+        it "does not have conflicting_manifest_settings when environment.conf manifest is nil" do
+          assert_manifest_conflict(false, nil)
+        end
+
+        it "does not have conflicting_manifest_settings when environment.conf manifest is an exact, uninterpolated match of default_manifest" do
+          assert_manifest_conflict(false, '/default/manifests/site.pp')
+        end
+      end
+    end
+
     describe "when modeling a specific environment" do
       it "should have a method for returning the environment name" do
         Puppet::Node::Environment.new("testing").name.should == :testing
diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 4698479d3..0ca01f521 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -97,6 +97,13 @@ describe Puppet::Parser::Compiler do
     @compiler.environment.should equal(@node.environment)
   end
 
+  it "fails if the node's environment has conflicting manifest settings" do
+    conflicted_environment = Puppet::Node::Environment.create(:testing, [], '/some/environment.conf/manifest.pp')
+    conflicted_environment.stubs(:conflicting_manifest_settings?).returns(true)
+    @node.environment = conflicted_environment
+    expect { Puppet::Parser::Compiler.compile(@node) }.to raise_error(Puppet::Error, /disable_per_environment_manifest.*true.*environment.conf.*manifest.*conflict/)
+  end
+
   it "should include the resource type collection helper" do
     Puppet::Parser::Compiler.ancestors.should be_include(Puppet::Resource::TypeCollectionHelper)
   end
diff --git a/spec/unit/settings/autosign_setting_spec.rb b/spec/unit/settings/autosign_setting_spec.rb
index 0c8184c8a..0dbfe4ecb 100644
--- a/spec/unit/settings/autosign_setting_spec.rb
+++ b/spec/unit/settings/autosign_setting_spec.rb
@@ -73,7 +73,7 @@ describe Puppet::Settings::AutosignSetting do
   describe "converting the setting to a resource" do
     it "converts the file path to a file resource" do
       path = File.expand_path('/path/to/autosign.conf')
-      settings.stubs(:value).with('autosign').returns(path)
+      settings.stubs(:value).with('autosign', nil, false).returns(path)
       Puppet::FileSystem.stubs(:exist?).with(path).returns true
       Puppet.stubs(:features).returns(stub(:root? => true, :microsoft_windows? => false))
 
@@ -91,7 +91,7 @@ describe Puppet::Settings::AutosignSetting do
     end
 
     it "returns nil when the setting is a boolean" do
-      settings.stubs(:value).with('autosign').returns 'true'
+      settings.stubs(:value).with('autosign', nil, false).returns 'true'
 
       setting.mode = '0664'
       setting.owner = 'service'
diff --git a/spec/unit/settings/environment_conf_spec.rb b/spec/unit/settings/environment_conf_spec.rb
index 6a8a6689e..206244236 100644
--- a/spec/unit/settings/environment_conf_spec.rb
+++ b/spec/unit/settings/environment_conf_spec.rb
@@ -3,31 +3,47 @@ require 'puppet/settings/environment_conf.rb'
 
 describe Puppet::Settings::EnvironmentConf do
 
+  def setup_environment_conf(config, conf_hash)
+    conf_hash.each do |setting,value|
+      config.expects(:setting).with(setting).returns(
+        mock('setting', :value => value)
+      )
+    end
+  end
+
   context "with config" do
-    let(:config) { stub(:config) }
+    let(:config) { stub('config') }
     let(:envconf) { Puppet::Settings::EnvironmentConf.new("/some/direnv", config, ["/global/modulepath"]) }
 
     it "reads a modulepath from config and does not include global_module_path" do
-      config.expects(:setting).with(:modulepath).returns(
-        mock('setting', :value => '/some/modulepath')
-      )
+      setup_environment_conf(config, :modulepath => '/some/modulepath')
+
       expect(envconf.modulepath).to eq(File.expand_path('/some/modulepath'))
     end
 
     it "reads a manifest from config" do
-      config.expects(:setting).with(:manifest).returns(
-        mock('setting', :value => '/some/manifest')
-      )
+      setup_environment_conf(config, :manifest => '/some/manifest')
+
       expect(envconf.manifest).to eq(File.expand_path('/some/manifest'))
     end
 
     it "reads a config_version from config" do
-      config.expects(:setting).with(:config_version).returns(
-        mock('setting', :value => '/some/version.sh')
-      )
+      setup_environment_conf(config, :config_version => '/some/version.sh')
+
       expect(envconf.config_version).to eq(File.expand_path('/some/version.sh'))
     end
 
+    it "read an environment_timeout from config" do
+      setup_environment_conf(config, :environment_timeout => '3m')
+
+      expect(envconf.environment_timeout).to eq(180)
+    end
+
+    it "can retrieve raw settings" do
+      setup_environment_conf(config, :manifest => 'manifest.pp')
+
+      expect(envconf.raw_setting(:manifest)).to eq('manifest.pp')
+    end
   end
 
   context "without config" do
@@ -47,5 +63,56 @@ describe Puppet::Settings::EnvironmentConf do
     it "returns nothing for config_version when config has none" do
       expect(envconf.config_version).to be_nil
     end
+
+    it "returns a defult of 0 for environment_timeout when config has none" do
+      expect(envconf.environment_timeout).to eq(0)
+    end
+
+    it "can still retrieve raw setting" do
+      expect(envconf.raw_setting(:manifest)).to be_nil
+    end
+  end
+
+  describe "with disable_per_environment_manifest" do
+
+    let(:config) { stub('config') }
+    let(:envconf) { Puppet::Settings::EnvironmentConf.new("/some/direnv", config, ["/global/modulepath"]) }
+
+    context "set true" do
+
+      before(:each) do
+        Puppet[:default_manifest] = '/default/manifest'
+        Puppet[:disable_per_environment_manifest] = true
+      end
+
+      it "ignores environment.conf manifest" do
+        setup_environment_conf(config, :manifest => '/some/manifest.pp')
+
+        expect(envconf.manifest).to eq(File.expand_path('/default/manifest'))
+      end
+
+      it "logs error when environment.conf has manifest set" do
+        setup_environment_conf(config, :manifest => '/some/manifest.pp')
+
+        envconf.manifest
+        expect(@logs.first.to_s).to match(/disable_per_environment_manifest.*true.*environment.conf.*does not match the default_manifest/)
+      end
+
+      it "does not log an error when environment.conf does not have a manifest set" do
+        setup_environment_conf(config, :manifest => nil)
+
+        expect(envconf.manifest).to eq(File.expand_path('/default/manifest'))
+        expect(@logs).to be_empty
+      end
+    end
+
+    it "uses environment.conf when false" do
+      setup_environment_conf(config, :manifest => '/some/manifest.pp')
+
+      Puppet[:default_manifest] = '/default/manifest'
+      Puppet[:disable_per_environment_manifest] = false
+
+      expect(envconf.manifest).to eq(File.expand_path('/some/manifest.pp'))
+    end
   end
 end
diff --git a/spec/unit/settings/file_setting_spec.rb b/spec/unit/settings/file_setting_spec.rb
index 245dcfc8f..c77cc5981 100755
--- a/spec/unit/settings/file_setting_spec.rb
+++ b/spec/unit/settings/file_setting_spec.rb
@@ -129,7 +129,7 @@ describe Puppet::Settings::FileSetting do
       @settings = mock 'settings'
       @file = Puppet::Settings::FileSetting.new(:settings => @settings, :desc => "eh", :name => :myfile, :section => "mysect")
       @file.stubs(:create_files?).returns true
-      @settings.stubs(:value).with(:myfile).returns @basepath
+      @settings.stubs(:value).with(:myfile, nil, false).returns @basepath
     end
 
     it "should return :file as its type" do
@@ -153,13 +153,13 @@ describe Puppet::Settings::FileSetting do
 
     describe "on POSIX systems", :if => Puppet.features.posix? do
       it "should skip files in /dev" do
-        @settings.stubs(:value).with(:myfile).returns "/dev/file"
+        @settings.stubs(:value).with(:myfile, nil, false).returns "/dev/file"
         @file.to_resource.should be_nil
       end
     end
 
     it "should skip files whose paths are not strings" do
-      @settings.stubs(:value).with(:myfile).returns :foo
+      @settings.stubs(:value).with(:myfile, nil, false).returns :foo
       @file.to_resource.should be_nil
     end
 
@@ -170,7 +170,7 @@ describe Puppet::Settings::FileSetting do
     end
 
     it "should fully qualified returned files if necessary (#795)" do
-      @settings.stubs(:value).with(:myfile).returns "myfile"
+      @settings.stubs(:value).with(:myfile, nil, false).returns "myfile"
       path = File.expand_path('myfile')
       @file.to_resource.title.should == path
     end