(SIMP-9623) Disable caching across calls to compliance_markup::enforc…

…ement (#125) * (SIMP-9623) Disable caching across calls to compliance_markup::enforcement SIMP-9623 #close * Additional cleanup * Removed the actual defunct caching methods * Changed the `lock` value to something compliance markup-specific since we're using the global cache now * Removed the calls to 'cached_lookup` since we can't access the appropriate context at that point and the only things that used them don't seem to be used anywhere else in the codebase * Fix global hieradata lookup Hooked the global hieradata into the lookup stack. (Honestly, I have no idea how this was working properly before) * remove extraneous method * Add a test for overrides using `compliance_markup::compliance_map`. Co-authored-by: Trevor Vaughan <[email protected]>
simp · Mar 23, 2021 · cb66cf4 · cb66cf4
1 parent bbdb7d4
commit cb66cf4
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 61 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,6 @@
 * Wed Feb 24 2021 Steven Pritchard <[email protected]> - 3.1.4-0
 - Refactor `list_puppet_params` to avoid excessive looping
+- Disable caching (SIMP-9623)
 
 * Mon Nov 02 2020 Andy Adrian <[email protected]> - 3.1.4-0
 - Remove unused and broken telemetry functionality

diff --git a/lib/puppet/functions/compliance_markup/enforcement.rb b/lib/puppet/functions/compliance_markup/enforcement.rb
@@ -52,26 +52,7 @@ def hiera_enforcement(key, options, context)
 
     retval = nil
 
-    # This is needed to prevent infinite looping. Usually, the lookup function
-    # is called from different modules and, therefore, requires different
-    # scoping. However, in our case, we're actually looking across modules and
-    # need to maintain the same context at all times to ensure that caching
-    # works properly.
-    if @context
-      # This should never be called, but we're going to be extra safe and reload
-      # the underlying mapper code if it is.
-      if @context.environment_name != context.environment_name
-        filename = File.expand_path('../../../../puppetx/simp/compliance_mapper.rb', __FILE__)
-        self.instance_eval(File.read(filename), filename)
-
-        @context = context
-      end
-    else
-      @context ||= context
-    end
-
-    # Quick return if we already have a cached value for the key.
-    return cached_value(key) if cached_value(key)
+    @context = context
 
     begin
       # Parameters are frozen

diff --git a/lib/puppetx/simp/compliance_map.rb b/lib/puppetx/simp/compliance_map.rb
@@ -453,26 +453,6 @@ def custom_call_file_info
   return file_info
 end
 
-# These methods are part of the callback api for compliance engine.
-
-def cache(key, value)
-  if @hash == nil
-    @hash = {}
-  end
-  @hash[key] = value
-end
-def cached_value(key)
-  if @hash == nil
-    @hash = {}
-  end
-  @hash[key]
-end
-def cache_has_key(key)
-  if @hash == nil
-    @hash = {}
-  end
-  @hash.key?(key)
-end
 def debug(message)
   return
 end

diff --git a/lib/puppetx/simp/compliance_mapper.rb b/lib/puppetx/simp/compliance_mapper.rb
@@ -41,14 +41,13 @@ def enforcement(key, context=self, options={"mode" => "value"}, &block)
 
   retval = :notfound
 
-  lock = false
-  lock = context.cached_value("lock") if context.cache_has_key("lock")
+  lock = context.cached_value('_simp_compliance_markup_lock') if context.cache_has_key('_simp_compliance_markup_lock')
 
   unless lock
-    context.cache("lock", true)
+    context.cache('_simp_compliance_markup_lock', true)
 
     begin
-      profile_list = cached_lookup("compliance_markup::enforcement", [], &block)
+      profile_list = call_function('lookup', 'compliance_markup::enforcement', { 'default_value' => [] })
 
       unless profile_list == []
         debug("debug: compliance_markup::enforcement set to #{profile_list}, attempting to enforce")
@@ -112,7 +111,7 @@ def enforcement(key, context=self, options={"mode" => "value"}, &block)
       debug(e.message)
       debug(e.backtrace.inspect)
     ensure
-      context.cache("lock", false)
+      context.cache('_simp_compliance_markup_lock', false)
     end
   end
 
@@ -121,19 +120,6 @@ def enforcement(key, context=self, options={"mode" => "value"}, &block)
   retval
 end
 
-# These cache functions are assumed to be created by the wrapper
-# object backend.
-def cached_lookup(key, default, &block)
-  if cache_has_key(key)
-    retval = cached_value(key)
-  else
-    retval = yield key, default
-    cache(key, retval)
-  end
-
-  retval
-end
-
 def compiler_class()
   Class.new do
     attr_reader :compliance_data
@@ -152,9 +138,8 @@ def initialize(object)
     def load(options={}, &block)
       @callback.debug("callback = #{callback.codebase}")
 
-      module_scope_compliance_map = callback.cached_lookup "compliance_markup::compliance_map", {}, &block
-      top_scope_compliance_map    = callback.cached_lookup "compliance_map", {}, &block
-
+      module_scope_compliance_map = block.call 'compliance_markup::compliance_map', {}
+      top_scope_compliance_map    = block.call 'compliance_map', {}
 
       @compliance_data = {}
 

diff --git a/spec/functions/compliance_markup/05_enforcement_override_spec.rb b/spec/functions/compliance_markup/05_enforcement_override_spec.rb
@@ -0,0 +1,122 @@
+#!/usr/bin/env ruby -S rspec
+
+require 'spec_helper'
+require 'semantic_puppet'
+require 'puppet/pops/lookup/context'
+require 'yaml'
+require 'fileutils'
+
+puppetver = SemanticPuppet::Version.parse(Puppet.version)
+requiredver = SemanticPuppet::Version.parse("4.10.0")
+
+describe 'lookup' do
+  # Generate a fake module with dummy data for lookup().
+  profile_yaml = {
+    'version'  => '2.0.0',
+    'profiles' => {
+      'profile_test1' => {
+        'ces' => {
+          '05_profile_test1' => true,
+          '05_profile_test2' => true,
+        },
+      },
+    },
+  }.to_yaml
+
+  ces_yaml = {
+    'version' => '2.0.0',
+    'ce'      => {
+      '05_profile_test1' => {},
+      '05_profile_test2' => {},
+    },
+  }.to_yaml
+
+  checks_yaml = {
+    'version' => '2.0.0',
+    'checks'  => {
+      '05_hash check1'   => {
+        'type'     => 'puppet-class-parameter',
+        'settings' => {
+          'parameter' => 'test_module_05::hash_param',
+          'value'     => {
+            'hash key 1' => 'hash value 1',
+          },
+        },
+        'ces'      => [
+          '05_profile_test1',
+        ],
+      },
+      '05_hash check2'   => {
+        'type'     => 'puppet-class-parameter',
+        'settings' => {
+          'parameter' => 'test_module_05::hash_param',
+          'value'     => {
+            'hash key 2' => 'hash value 2',
+          },
+        },
+        'ces'      => [
+          '05_profile_test2',
+        ],
+      },
+    },
+  }.to_yaml
+
+  fixtures = File.expand_path('../../fixtures', __dir__)
+
+  compliance_dir = File.join(fixtures, 'modules', 'test_module_05', 'SIMP', 'compliance_profiles')
+  FileUtils.mkdir_p(compliance_dir)
+
+  File.open(File.join(compliance_dir, 'profile.yaml'), 'w') do |fh|
+    fh.puts profile_yaml
+  end
+
+  File.open(File.join(compliance_dir, 'ces.yaml'), 'w') do |fh|
+    fh.puts ces_yaml
+  end
+
+  File.open(File.join(compliance_dir, 'checks.yaml'), 'w') do |fh|
+    fh.puts checks_yaml
+  end
+
+  on_supported_os.each do |os, os_facts|
+    context "on #{os} with compliance data in modules" do
+      before(:all) do
+        File.open(File.join(fixtures, 'hieradata', 'profile-merging.yaml'), 'w') do |fh|
+          test_hiera = {'compliance_markup::enforcement' => ['profile_test1']}.to_yaml
+          fh.puts test_hiera
+        end
+      end
+
+      let(:hieradata) { 'profile-merging' }
+
+      # Test a simple hash.
+      it { is_expected.to run.with_params('test_module_05::hash_param').and_return({'hash key 1' => 'hash value 1', 'hash key 2' => 'hash value 2'}) }
+    end
+
+    context "on #{os} with compliance_markup::compliance_map override" do
+      before(:all) do
+        File.open(File.join(fixtures, 'hieradata', 'profile-merging.yaml'), 'w') do |fh|
+          test_hiera = {
+            'compliance_markup::enforcement'    => ['profile_test1'],
+            'compliance_markup::compliance_map' => {
+              'version'  => '2.0.0',
+              'profiles' => {
+                'profile_test1' => {
+                  'ces' => {
+                    '05_profile_test2' => false,
+                  },
+                },
+              },
+            },
+          }.to_yaml
+          fh.puts test_hiera
+        end
+      end
+
+      let(:hieradata) { 'profile-merging' }
+
+      # Test a simple hash.
+      it { is_expected.to run.with_params('test_module_05::hash_param').and_return({'hash key 1' => 'hash value 1'}) }
+    end
+  end
+end