From a1c0437da29e0274e73d125d87167e8125fee81e Mon Sep 17 00:00:00 2001 From: Bernd Ahlers Date: Sat, 14 Mar 2026 18:27:45 +0100 Subject: [PATCH] Fix Debian arch detection, Hiera scope errors, and test reliability Use dpkg --print-architecture on Debian to return Facter-compatible arch values (amd64, arm64) instead of RbConfig's x86_64/aarch64, since recipes use Facts.arch in download URLs expecting Debian-style names. Hiera::Scope#[] now raises ScopeError for unknown keys instead of returning nil, making the Facter removal a loud failure for direct callers. Normal Hiera flow is unaffected since include? gates access. Fix DependencyInspector#update_package_db_once to only set the updated flag after a successful refresh, so failed apt-get update/yum makecache commands are retried. Fix test pollution in recipe_spec.rb where class_eval redefined Facts.arch and Facts.target permanently, leaking across tests. Replace with scoped RSpec stubs. Additional changes: - Add DependencyInspector.reset! for cleaner test setup - Extract find_command helper to resolve full paths (eliminates TOCTOU) - Warn on unmapped platforms in detect_osfamily - Fix bundler version selection for Ruby < 3.2 in distro tests --- Rakefile | 16 +++++- lib/fpm/cookery/dependency_inspector.rb | 12 +++- lib/fpm/cookery/facts.rb | 29 ++++++++-- lib/fpm/cookery/hiera/scope.rb | 16 +++++- spec/dependency_inspector_spec.rb | 6 +- spec/facts_spec.rb | 73 +++++++++++++++++++++++-- spec/hiera_spec.rb | 4 +- spec/recipe_spec.rb | 19 +------ 8 files changed, 133 insertions(+), 42 deletions(-) diff --git a/Rakefile b/Rakefile index 7cd5112..f812594 100644 --- a/Rakefile +++ b/Rakefile @@ -42,6 +42,13 @@ namespace 'test:distro' do 'alpine-3.20' => 'alpine:3.20', 'fedora-40' => 'fedora:40', 'fedora-41' => 'fedora:41', + # openSUSE Tumbleweed ships Ruby 4.0 which is incompatible with + # our dependencies (cabin/fpm require 'logger' which was removed + # from Ruby's standard library in 4.0). Leap 15.x ships Ruby 2.5 + # which is below our minimum (2.7). Re-enable when dependencies + # support Ruby 4.0. + #'opensuse-tw' => 'opensuse/tumbleweed:latest', + 'archlinux' => 'archlinux:latest', } distros.each do |name, image| @@ -55,16 +62,21 @@ namespace 'test:distro' do 'dnf install -y ruby ruby-devel gcc gcc-c++ make git redhat-rpm-config' when /alpine/ 'apk add ruby ruby-dev build-base git' + when /opensuse/ + 'zypper -n install ruby ruby-devel gcc gcc-c++ make git' + when /archlinux/ + 'pacman -Sy --noconfirm ruby base-devel git' end - # Install bundler with version check for Ruby < 3.0 - bundler_install = 'ruby -e "puts RUBY_VERSION" | grep -q "^2\\." && gem install bundler -v 2.4.22 || gem install bundler' + # Bundler >= 2.5 requires Ruby >= 3.2. Use 2.4.22 for older Ruby. + bundler_install = 'ruby -e "exit(Gem::Version.new(RUBY_VERSION) >= Gem::Version.new(%(3.2)) ? 0 : 1)" && gem install bundler || gem install bundler -v 2.4.22' desc "Run tests on #{name}" task name do sh %(docker run -i --rm -v #{src}:/src #{image} sh -c ' #{install_cmd} && #{bundler_install} && + export PATH="$(ruby -e "puts Gem.user_dir")/bin:$PATH" && git config --global --add safe.directory /src && cp -r /src /work && cd /work && bundle install -j 4 && diff --git a/lib/fpm/cookery/dependency_inspector.rb b/lib/fpm/cookery/dependency_inspector.rb index fdefc15..04ea016 100644 --- a/lib/fpm/cookery/dependency_inspector.rb +++ b/lib/fpm/cookery/dependency_inspector.rb @@ -119,6 +119,11 @@ def package_suitable?(package) true end + def reset! + @unsupported_platform_warned = false + @package_db_updated = false + end + private def current_backend @@ -138,14 +143,17 @@ def warn_unsupported_platform_once def update_package_db_once return if @package_db_updated - @package_db_updated = true backend = current_backend return unless backend && backend[:update] return unless Process.euid == 0 Log.info "Updating package database" - backend[:update].call + if backend[:update].call + @package_db_updated = true + else + Log.warn "Package database update failed; will retry on next verify" + end end end end diff --git a/lib/fpm/cookery/facts.rb b/lib/fpm/cookery/facts.rb index a013e2d..b6051a0 100644 --- a/lib/fpm/cookery/facts.rb +++ b/lib/fpm/cookery/facts.rb @@ -1,4 +1,5 @@ require 'rbconfig' +require 'shellwords' module FPM module Cookery @@ -77,6 +78,14 @@ def parse_os_release end def detect_arch + case osfamily + when :debian + dpkg_path = find_command('dpkg') + raise PlatformDetectionError, "dpkg is required to detect architecture on Debian-based systems" unless dpkg_path + output = `#{Shellwords.escape(dpkg_path)} --print-architecture 2>/dev/null`.strip + return output.downcase.to_sym unless output.empty? + end + arch = RbConfig::CONFIG['host_cpu']&.downcase return nil unless arch arch.to_sym @@ -155,6 +164,10 @@ def detect_osfamily :gentoo when :darwin :darwin + else + warn "fpm-cookery: Unknown OS family for platform '#{source}'. " \ + "Set manually: FPM::Cookery::Facts.osfamily = 'debian'" + nil end end @@ -172,8 +185,9 @@ def detect_lsbcodename end def lsb_release_codename - return nil unless command_exists?('lsb_release') - output = `lsb_release -cs 2>/dev/null`.strip + path = find_command('lsb_release') + return nil unless path + output = `#{Shellwords.escape(path)} -cs 2>/dev/null`.strip return nil if output.empty? # Validate output before creating symbol to prevent symbol table exhaustion # Codenames are usually short (e.g., "bookworm", "jammy") and alphanumeric with hyphens @@ -185,13 +199,18 @@ def lsb_release_codename end def command_exists?(cmd) + !find_command(cmd).nil? + end + + def find_command(cmd) # Validate command name - must be simple name, no paths or shell metacharacters - return false unless cmd.match?(/\A[a-zA-Z0-9._-]+\z/) + return nil unless cmd.match?(/\A[a-zA-Z0-9._-]+\z/) - ENV['PATH'].to_s.split(File::PATH_SEPARATOR).any? do |dir| + ENV['PATH'].to_s.split(File::PATH_SEPARATOR).each do |dir| path = File.join(dir, cmd) - File.executable?(path) && File.file?(path) + return path if File.executable?(path) && File.file?(path) end + nil end end end diff --git a/lib/fpm/cookery/hiera/scope.rb b/lib/fpm/cookery/hiera/scope.rb index 4547633..a558a4c 100644 --- a/lib/fpm/cookery/hiera/scope.rb +++ b/lib/fpm/cookery/hiera/scope.rb @@ -3,8 +3,15 @@ module FPM module Cookery module Hiera + class ScopeError < StandardError; end + # Wraps a recipe class, adding a +[]+ method so that it can be used as a # +Hiera+ scope. + # + # NOTE: Facter fallback was removed. Only recipe methods and + # FPM::Cookery::Facts methods are available for Hiera interpolation. + # Facter-specific facts like +processorcount+ or +ipaddress+ are no + # longer resolvable via +%{scope("key")}+. class Scope attr_reader :recipe @@ -14,8 +21,9 @@ def initialize(recipe) # Allow Hiera to perform +%{scope("key")}+ interpolations using data # from the recipe class and +FPM::Cookery::Facts+. Expects +name+ to - # be a method name. Returns the result of the lookup. Will be +nil+ - # if lookup failed to fetch a result. + # be a method name. Raises +ScopeError+ if the key cannot be resolved + # (e.g. via direct calls). In normal Hiera flow, +include?+ gates + # access so unresolvable keys are treated as empty strings by Hiera. def [](name) [recipe, FPM::Cookery::Facts].each do |source| if source.respond_to?(name) @@ -23,7 +31,9 @@ def [](name) end end - nil + raise ScopeError, "Unknown Hiera scope key '#{name}'. " \ + "The Facter fallback has been removed. Only recipe methods and " \ + "FPM::Cookery::Facts methods are available for interpolation." end # Newer versions of Hiera requires also +#include?+ method for context diff --git a/spec/dependency_inspector_spec.rb b/spec/dependency_inspector_spec.rb index b0c1b2d..76f2847 100644 --- a/spec/dependency_inspector_spec.rb +++ b/spec/dependency_inspector_spec.rb @@ -4,6 +4,7 @@ describe FPM::Cookery::DependencyInspector do before do FPM::Cookery::Facts.reset! + described_class.reset! end describe '.package_suitable?' do @@ -122,7 +123,6 @@ context 'on unsupported platform' do before do FPM::Cookery::Facts.osfamily = 'unknown' - described_class.instance_variable_set(:@unsupported_platform_warned, false) end it 'returns true and logs warning' do @@ -324,10 +324,6 @@ end describe 'package database update' do - before do - described_class.instance_variable_set(:@package_db_updated, false) - end - context 'on debian family' do before do FPM::Cookery::Facts.osfamily = 'debian' diff --git a/spec/facts_spec.rb b/spec/facts_spec.rb index e6bf280..023d773 100644 --- a/spec/facts_spec.rb +++ b/spec/facts_spec.rb @@ -14,6 +14,60 @@ it "returns a valid architecture" do expect(FPM::Cookery::Facts.arch).to_not be_nil end + + context "on debian family" do + before do + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with('/etc/os-release').and_return(true) + allow(File).to receive(:readlines).with('/etc/os-release').and_return([ + 'ID=ubuntu', + 'ID_LIKE=debian' + ]) + end + + context "when dpkg is available" do + before do + allow(FPM::Cookery::Facts).to receive(:find_command).with('dpkg').and_return('/usr/bin/dpkg') + end + + it "returns amd64 from dpkg" do + allow(FPM::Cookery::Facts).to receive(:`).with('/usr/bin/dpkg --print-architecture 2>/dev/null').and_return("amd64\n") + expect(FPM::Cookery::Facts.arch).to eq(:amd64) + end + + it "returns arm64 from dpkg" do + allow(FPM::Cookery::Facts).to receive(:`).with('/usr/bin/dpkg --print-architecture 2>/dev/null').and_return("arm64\n") + expect(FPM::Cookery::Facts.arch).to eq(:arm64) + end + end + + context "when dpkg is not available" do + before do + allow(FPM::Cookery::Facts).to receive(:find_command).with('dpkg').and_return(nil) + end + + it "raises PlatformDetectionError" do + expect { FPM::Cookery::Facts.arch }.to raise_error(FPM::Cookery::PlatformDetectionError, /dpkg is required/) + end + end + end + + context "on non-debian family" do + before do + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with('/etc/os-release').and_return(true) + allow(File).to receive(:readlines).with('/etc/os-release').and_return([ + 'ID=rocky', + 'ID_LIKE="rhel centos fedora"' + ]) + end + + it "falls back to RbConfig host_cpu" do + arch = FPM::Cookery::Facts.arch + expect(arch).to be_a(Symbol) + expect(arch).to eq(RbConfig::CONFIG['host_cpu'].downcase.to_sym) + end + end end describe "platform" do @@ -208,32 +262,32 @@ allow(File).to receive(:readlines).with('/etc/os-release').and_return([ 'ID=debian' ]) - allow(FPM::Cookery::Facts).to receive(:system).and_return(true) + allow(FPM::Cookery::Facts).to receive(:find_command).with('lsb_release').and_return('/usr/bin/lsb_release') end it "rejects output longer than 64 characters" do long_output = 'a' * 65 - allow(FPM::Cookery::Facts).to receive(:`).with('lsb_release -cs 2>/dev/null').and_return(long_output) + allow(FPM::Cookery::Facts).to receive(:`).with('/usr/bin/lsb_release -cs 2>/dev/null').and_return(long_output) expect(FPM::Cookery::Facts.lsbcodename).to be_nil end it "rejects output with invalid characters" do - allow(FPM::Cookery::Facts).to receive(:`).with('lsb_release -cs 2>/dev/null').and_return("bookworm; rm -rf /") + allow(FPM::Cookery::Facts).to receive(:`).with('/usr/bin/lsb_release -cs 2>/dev/null').and_return("bookworm; rm -rf /") expect(FPM::Cookery::Facts.lsbcodename).to be_nil end it "accepts valid codenames with dots" do - allow(FPM::Cookery::Facts).to receive(:`).with('lsb_release -cs 2>/dev/null').and_return("n/a") + allow(FPM::Cookery::Facts).to receive(:`).with('/usr/bin/lsb_release -cs 2>/dev/null').and_return("n/a") expect(FPM::Cookery::Facts.lsbcodename).to be_nil end it "accepts valid codenames with hyphens and underscores" do - allow(FPM::Cookery::Facts).to receive(:`).with('lsb_release -cs 2>/dev/null').and_return("test-code_name.1") + allow(FPM::Cookery::Facts).to receive(:`).with('/usr/bin/lsb_release -cs 2>/dev/null').and_return("test-code_name.1") expect(FPM::Cookery::Facts.lsbcodename).to eq(:"test-code_name.1") end it "returns valid codename from lsb_release" do - allow(FPM::Cookery::Facts).to receive(:`).with('lsb_release -cs 2>/dev/null').and_return("bookworm\n") + allow(FPM::Cookery::Facts).to receive(:`).with('/usr/bin/lsb_release -cs 2>/dev/null').and_return("bookworm\n") expect(FPM::Cookery::Facts.lsbcodename).to eq(:bookworm) end end @@ -275,6 +329,13 @@ end end + describe "with os family Archlinux" do + it "returns pacman" do + FPM::Cookery::Facts.osfamily = 'Archlinux' + expect(FPM::Cookery::Facts.target).to eq(:pacman) + end + end + describe "with an unknown os family" do it "returns nil" do FPM::Cookery::Facts.osfamily = '___X___' diff --git a/spec/hiera_spec.rb b/spec/hiera_spec.rb index e6f87ed..e841024 100644 --- a/spec/hiera_spec.rb +++ b/spec/hiera_spec.rb @@ -54,8 +54,8 @@ end context 'given an otherwise unresolvable argument' do - it 'returns nil' do - expect(scope['nonexistent_key']).to be_nil + it 'raises a ScopeError' do + expect { scope['nonexistent_key'] }.to raise_error(FPM::Cookery::Hiera::ScopeError, /Unknown Hiera scope key/) end end end diff --git a/spec/recipe_spec.rb b/spec/recipe_spec.rb index b29fe07..a1e344e 100644 --- a/spec/recipe_spec.rb +++ b/spec/recipe_spec.rb @@ -442,9 +442,7 @@ def self.platform; :centos; end describe ".architectures" do before do - FPM::Cookery::Facts.class_eval do - def self.arch; :x86_64; end - end + allow(FPM::Cookery::Facts).to receive(:arch).and_return(:x86_64) end describe "with a list of architectures" do @@ -492,20 +490,7 @@ def self.arch; :x86_64; end describe ".targets" do before do - FPM::Cookery::Facts.class_eval do - class << self - alias_method :target_orig, :target - def target; :rpm; end - end - end - end - - after do - FPM::Cookery::Facts.class_eval do - class << self - alias_method :target, :target_orig - end - end + allow(FPM::Cookery::Facts).to receive(:target).and_return(:rpm) end describe "with a list of targets" do