(PUP-2035) Fix issues with loader configuration

During the review of the functionality for PUP-2035 it was discovered
that it was not possible to call a function in a module from the
environment. This was due to the initial configuration, and that the
evaluator used the wrong loader.

This commit fixes the logic so that:
* the environment loader is now split into two parts, a public (sees
only things in the environment), and private (the public + all modules).
* a module loader is parented by the public_environment_loader (does not
see all modules - only those it depends on).
* functions defined outside of a module are added to the public loader
(i.e. from the command line, or site.pp)

This fix is needed to be able to evaluate PUP-2176

lib/puppet/parser/ast/pops_bridge.rb

@@ -181,7 +181,8 @@ def instantiate_FunctionDefinition(function_definition, modname)
         #        A private environment loader could be used for logic outside of modules, then only that logic
         #        would see the function.
         #
-        loaders.environment_loader()
+        # Use the private loader, this function may see the environment's dependencies (currently, all modules)
+        loaders.private_environment_loader()
       else
         # TODO : Later check if function is private, and then add it to
         #        private_loader_for_module

lib/puppet/pops/evaluator/runtime3_support.rb

@@ -232,9 +232,9 @@ def call_function(name, args, o, scope)
     # Call via 4x API if it is available, and the function exists
     #
     if loaders = Puppet.lookup(:loaders) {nil}
-      # find the loader that loaded the code, or use the system loader
+      # find the loader that loaded the code, or use the private_environment_loader (sees env + all modules)
       adapter = Puppet::Pops::Utils.find_adapter(o, Puppet::Pops::Adapters::LoaderAdapter)
-      loader = adapter.nil? ? loaders.environment_loader : adapter.loader
+      loader = adapter.nil? ? loaders.private_environment_loader : adapter.loader
       if loader && func = loader.load(:function, name)
         return func.call(scope, *args)
       end

lib/puppet/pops/loader/dependency_loader.rb

@@ -49,7 +49,7 @@ def find(typed_name)
   end
 
   def to_s()
-    "(DependencyLoader '#{@name}' [" + @dependency_loaders.map {|loader| loader.to_s }.join(' ,') + "])"
+    "(DependencyLoader '#{@loader_name}' [" + @dependency_loaders.map {|loader| loader.to_s }.join(' ,') + "])"
   end
 
   private

lib/puppet/pops/loaders.rb

@@ -3,7 +3,8 @@ class LoaderError < Puppet::Error; end
 
   attr_reader :static_loader
   attr_reader :puppet_system_loader
-  attr_reader :environment_loader
+  attr_reader :public_environment_loader
+  attr_reader :private_environment_loader
 
   def initialize()
     # The static loader can only be changed after a reboot
@@ -20,7 +21,7 @@ def initialize()
     #    concept of environment the same way as when running as a master (except when doing apply).
     #    The creation mechanisms should probably differ between the two.
     #
-    @environment_loader = create_environment_loader()
+    @private_environment_loader = create_environment_loader()
 
     # 3. module loaders are set up from the create_environment_loader, they register themselves
   end
@@ -107,6 +108,14 @@ def create_environment_loader()
     end
     # An environment has a module path even if it has a null loader
     configure_loaders_for_modules(loader, current_environment)
+    # modules should see this loader
+    @public_environment_loader = loader
+
+    # Code in the environment gets to see all modules (since there is no metadata for the environment)
+    # but since this is not given to the module loaders, they can not load global code (since they can not
+    # have prior knowledge about this
+    loader = Puppet::Pops::Loader::DependencyLoader.new(loader, "environment", @module_resolver.all_module_loaders())
+
     loader
   end
 
@@ -142,7 +151,7 @@ def initialize(puppet_module)
       @state = :initial
       @puppet_module = puppet_module
       @resolutions = []
-      @loader = nil
+      @public_loader = nil
       @private_loader = nil
     end
 
@@ -185,7 +194,7 @@ def []=(name, module_data)
     end
 
     def all_module_loaders
-      @all_module_loaders ||= @index.map {|md| md.loader }
+      @all_module_loaders ||= @index.values.map {|md| md.public_loader }
     end
 
     def resolve(module_data)

spec/unit/pops/loaders/loaders_spec.rb

@@ -23,9 +23,12 @@ def config_dir(config_name)
   it 'creates an environment loader' do
     loaders = Puppet::Pops::Loaders.new()
     # When this test is running, there is no environments dir configured, and a NullLoader is therefore used a.t.m
-    expect(loaders.environment_loader().class).to be(Puppet::Pops::Loader::SimpleEnvironmentLoader)
+    expect(loaders.public_environment_loader().class).to be(Puppet::Pops::Loader::SimpleEnvironmentLoader)
     # The default name of the enironment is '*root*', and the loader should identify itself that way
-    expect(loaders.environment_loader().to_s).to eql("(SimpleEnvironmentLoader 'environment:*root*')")
+    expect(loaders.public_environment_loader().to_s).to eql("(SimpleEnvironmentLoader 'environment:*root*')")
+
+    expect(loaders.private_environment_loader().class).to be(Puppet::Pops::Loader::DependencyLoader)
+    expect(loaders.private_environment_loader().to_s).to eql("(DependencyLoader 'environment' [])")
   end
 
   context 'when delegating 3x to 4x' do