Move environment loading into its own classes

lib/compliance_engine/data.rb

@@ -17,6 +17,7 @@
 require 'compliance_engine/data_loader/json'
 require 'compliance_engine/data_loader/yaml'
 require 'compliance_engine/module_loader'
+require 'compliance_engine/environment_loader'
 
 require 'deep_merge'
 require 'json'
@@ -91,33 +92,18 @@ def reset_collection
   # @param [String] path The Puppet environment archive file
   # @return [NilClass]
   def open_environment_zip(path)
-    require 'zip/filesystem'
-
-    self.modulepath = path
-
-    Zip::File.open(path) do |zipfile|
-      dir = zipfile.dir
-      file = zipfile.file
-
-      modules = dir.entries('/'.dup).select do |entry|
-        file.directory?(entry) && %r{\A[a-z][a-z0-9_]*\Z}.match?(entry.to_s)
-      end
-      open(*modules, fileclass: file, dirclass: dir)
-    end
+    environment = ComplianceEngine::EnvironmentLoader::Zip.new(path)
+    self.modulepath = environment.modulepath
+    open(environment)
   end
 
   # Scan a Puppet environment
   # @param [Array<String>] paths The Puppet modulepath components
   # @return [NilClass]
   def open_environment(*paths)
-    self.modulepath = paths
-    modules = paths.select { |path| File.directory?(path) }.map { |path|
-      Dir.children(path)
-         .select { |child| File.directory?(File.join(path, child)) }
-         .grep(%r{\A[a-z][a-z0-9_]*\Z})
-         .map { |child| File.join(path, child) }
-    }.flatten
-    open(*modules)
+    environment = ComplianceEngine::EnvironmentLoader.new(*paths)
+    self.modulepath = environment.modulepath
+    open(environment)
   end
 
   # Scan paths for compliance data files
@@ -130,6 +116,19 @@ def open(*paths, fileclass: File, dirclass: Dir)
     modules = {}
 
     paths.each do |path|
+      if path.is_a?(ComplianceEngine::EnvironmentLoader)
+        open(*path.modules)
+        next
+      end
+
+      if path.is_a?(ComplianceEngine::ModuleLoader)
+        modules[path.name] = path.version unless path.name.nil?
+        path.files.each do |file_loader|
+          update(file_loader)
+        end
+        next
+      end
+
       if path.is_a?(ComplianceEngine::DataLoader)
         update(path, key: path.key, fileclass: fileclass)
         next
@@ -146,11 +145,7 @@ def open(*paths, fileclass: File, dirclass: Dir)
       end
 
       if fileclass.directory?(path)
-        loader = ComplianceEngine::ModuleLoader.new(path, fileclass: fileclass, dirclass: dirclass)
-        modules[loader.name] = loader.version unless loader.name.nil?
-        loader.files.each do |file_loader|
-          update(file_loader)
-        end
+        open(ComplianceEngine::ModuleLoader.new(path, fileclass: fileclass, dirclass: dirclass))
         next
       end
 

lib/compliance_engine/environment_loader.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'compliance_engine'
+require 'compliance_engine/module_loader'
+
+# Load compliance engine data from a Puppet environment
+class ComplianceEngine::EnvironmentLoader
+  # Initialize an EnvironmentLoader from the components of a Puppet `modulepath`
+  #
+  # @param paths [Array] the paths to search for Puppet modules
+  # @param root [String] the root directory to search for Puppet modules
+  # @param fileclass [File] the class to use for file operations (default: `File`)
+  # @param dirclass [Dir] the class to use for directory operations (default: `Dir`)
+  def initialize(*paths, root: nil, fileclass: File, dirclass: Dir)
+    raise ArgumentError, 'No paths specified' if paths.empty?
+    @modulepath ||= paths
+    modules = paths.map do |path|
+      root ||= path
+      dirclass.entries(root)
+              .grep(%r{\A[a-z][a-z0-9_]*\Z})
+              .select { |child| fileclass.directory?(File.join(root, child)) }
+              .map { |child| File.join(root, child) }
+    rescue
+      []
+    end
+    modules.flatten!
+    @modules = modules.map { |path| ComplianceEngine::ModuleLoader.new(path, fileclass: fileclass, dirclass: dirclass) }
+  end
+
+  attr_reader :modulepath, :modules
+end

lib/compliance_engine/environment_loader/zip.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require 'compliance_engine'
+require 'compliance_engine/environment_loader'
+require 'zip/filesystem'
+
+# Load compliance engine data from a zip file containing a Puppet environment
+class ComplianceEngine::EnvironmentLoader::Zip < ComplianceEngine::EnvironmentLoader
+  # Initialize a ComplianceEngine::EnvironmentLoader::Zip object from a zip
+  # file and an optional root directory.
+  #
+  # @param path [String] the path to the zip file containing the Puppet environment
+  # @param root [String] a directory within the zip file to use as the root of the environment
+  def initialize(path, root: '/'.dup)
+    @modulepath = path
+
+    ::Zip::File.open(path) do |zipfile|
+      dir = zipfile.dir
+      file = zipfile.file
+
+      super(path, root: root, fileclass: file, dirclass: dir)
+    end
+  end
+end

lib/compliance_engine/module_loader.rb

@@ -6,6 +6,11 @@
 
 # Load compliance engine data from a Puppet module
 class ComplianceEngine::ModuleLoader
+  # Initialize a ModuleLoader from a Puppet module path
+  #
+  # @param path [String] the path to the Puppet module
+  # @param fileclass [File] the class to use for file operations (default: `File`)
+  # @param dirclass [Dir] the class to use for directory operations (default: `Dir`)
   def initialize(path, fileclass: File, dirclass: Dir)
     raise ComplianceEngine::Error, "#{path} is not a directory" unless fileclass.directory?(path)
 
@@ -28,9 +33,9 @@ def initialize(path, fileclass: File, dirclass: Dir)
     # In this directory, we want to look for all yaml and json files
     # under SIMP/compliance_profiles and simp/compliance_profiles.
     globs = ['SIMP/compliance_profiles', 'simp/compliance_profiles']
-            .select { |dir| fileclass.directory?("#{path}/#{dir}") }
+            .select { |dir| fileclass.directory?(File.join(path, dir)) }
             .map { |dir|
-      ['yaml', 'json'].map { |type| "#{path}/#{dir}/**/*.#{type}" }
+      ['yaml', 'json'].map { |type| File.join(path, dir, '**', "*.#{type}") }
     }.flatten
     # debug "Globs: #{globs}"
     # Using .each here to make mocking with rspec easier.

spec/classes/compliance_engine/environment_loader/zip_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'compliance_engine'
+require 'compliance_engine/environment_loader/zip'
+
+RSpec.describe ComplianceEngine::EnvironmentLoader::Zip do
+  before(:each) do
+    allow(Dir).to receive(:entries).and_call_original
+  end
+
+  context 'with no paths' do
+    it 'does not initialize' do
+      expect { described_class.new }.to raise_error(ArgumentError)
+    end
+  end
+
+  context 'with an invalid zip' do
+    subject(:environment_loader) { described_class.new(path) }
+
+    let(:path) { '/path/to/module.zip' }
+
+    before(:each) do
+      allow(::Zip::File).to receive(:open).with(path).and_raise(Zip::Error)
+    end
+
+    it 'does not initialize' do
+      expect { described_class.new(path) }.to raise_error(Zip::Error)
+    end
+  end
+
+  context 'with a valid zip' do
+    subject(:environment_loader) { described_class.new(path) }
+
+    let(:path) { File.expand_path('../../../fixtures/test_environment.zip', __dir__) }
+
+    before(:each) do
+      allow(ComplianceEngine::ModuleLoader).to receive(:new).and_return(instance_double(ComplianceEngine::ModuleLoader))
+    end
+
+    it 'initializes' do
+      expect(environment_loader).to be_instance_of(described_class)
+    end
+
+    it 'is not empty' do
+      expect(environment_loader.modules).to be_instance_of(Array)
+      expect(environment_loader.modules.count).to eq(2)
+      expect(environment_loader.modulepath).to eq(path)
+    end
+  end
+end

spec/classes/compliance_engine/environment_loader_spec.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'compliance_engine'
+require 'compliance_engine/environment_loader'
+
+RSpec.describe ComplianceEngine::EnvironmentLoader do
+  before(:each) do
+    allow(Dir).to receive(:entries).and_call_original
+  end
+
+  context 'with no paths' do
+    it 'does not initialize' do
+      expect { described_class.new }.to raise_error(ArgumentError)
+    end
+  end
+
+  context 'with an invalid path' do
+    subject(:environment_loader) { described_class.new(path) }
+
+    let(:path) { '/path/to/module' }
+
+    before(:each) do
+      allow(Dir).to receive(:entries).with(path).and_raise(Errno::ENOTDIR)
+    end
+
+    it 'initializes' do
+      expect(environment_loader).to be_instance_of(described_class)
+    end
+
+    it 'is empty' do
+      expect(environment_loader.modules).to be_empty
+    end
+  end
+
+  context 'with valid paths' do
+    subject(:environment_loader) { described_class.new(*paths) }
+
+    let(:paths) { ['/path1', '/path2'] }
+
+    before(:each) do
+      allow(Dir).to receive(:entries).with('/path1').and_return(['.', '..', 'a'])
+      allow(Dir).to receive(:entries).with('/path2').and_return(['.', '..', 'b'])
+      allow(File).to receive(:directory?).with('/path1/a').and_return(true)
+      allow(File).to receive(:directory?).with('/path1/b').and_return(true)
+      allow(ComplianceEngine::ModuleLoader).to receive(:new).and_return(instance_double(ComplianceEngine::ModuleLoader))
+    end
+
+    it 'initializes' do
+      expect(environment_loader).to be_instance_of(described_class)
+    end
+
+    it 'is not empty' do
+      expect(environment_loader.modules).to be_instance_of(Array)
+      expect(environment_loader.modules.count).to eq(2)
+      expect(environment_loader.modulepath).to eq(paths)
+    end
+  end
+end

spec/classes/compliance_engine/module_loader_spec.rb

@@ -34,6 +34,24 @@
     end
   end
 
+  context 'with no module data' do
+    subject(:module_loader) { described_class.new(path) }
+
+    let(:path) { '/path/to/module' }
+
+    before(:each) do
+      allow(File).to receive(:directory?).with(path).and_return(true)
+      allow(File).to receive(:exist?).with("#{path}/metadata.json").and_return(false)
+      allow(File).to receive(:directory?).with("#{path}/SIMP/compliance_profiles").and_return(false)
+      allow(File).to receive(:directory?).with("#{path}/simp/compliance_profiles").and_return(false)
+    end
+
+    it 'initializes' do
+      expect(module_loader).not_to be_nil
+      expect(module_loader).to be_instance_of(described_class)
+    end
+  end
+
   context 'with module data' do
     subject(:module_loader) { described_class.new(test_data.keys.first) }