From 9ae73a0be220cb2c47c39f9b9acf91787a53f51c Mon Sep 17 00:00:00 2001 From: mdasif Date: Tue, 1 Sep 2026 17:10:01 +0530 Subject: [PATCH 1/4] Add automated Splunk forwarder version updates Adds an opt-in auto-upgrade path for the Splunk Universal Forwarder: leaving version/build unset tracks the latest release via a new splunk_forwarder_latest_version() function (scraped from Splunk's download page, cached 6h, with stale-cache/hardcoded fallbacks so a lookup failure never breaks catalog compilation). Pinning version requires build to match, enforced by a fail-fast check. The pin-vs- fetch decision lives in the function itself - it takes version/build as required (but nullable) params and returns them as-is when both are given, only falling through to the cache/fetch path when neither is. Upgrades/downgrades are applied via package_ensure => 'latest': both the dpkg and rpm providers read the target version directly from the already-staged source file and, if it differs from what's installed, apply it in place via `dpkg -i`/`rpm -U --oldpackage` - which handles both directions natively and leaves etc/system/local/* (including seeded admin credentials) untouched, since it isn't a packaged conffile. Also extends the accept-tos exec's timeout from Puppet's default 300s to 900s via a resource collector override (a fresh install's first start can take longer than that), and makes Service resources respect noop_value like the rest of this profile's resources. --- .../manifests/monitor/splunk/forwarder.pp | 42 ++++++--- .../splunk_forwarder_latest_version.rb | 91 +++++++++++++++++++ .../manifests/collector/splunk/forwarder.pp | 14 +++ 3 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb diff --git a/modules/enableit/common/manifests/monitor/splunk/forwarder.pp b/modules/enableit/common/manifests/monitor/splunk/forwarder.pp index 9f5f48ae..76995f55 100644 --- a/modules/enableit/common/manifests/monitor/splunk/forwarder.pp +++ b/modules/enableit/common/manifests/monitor/splunk/forwarder.pp @@ -2,9 +2,14 @@ # # @param password_hash The password hash for the forwarder. This parameter is required. # -# @param version The version of the Splunk forwarder. Defaults to '7.2.4'. +# @param version The version of the Splunk forwarder to pin. When left undef (the default), +# the latest Splunk Universal Forwarder release is tracked automatically instead (checked +# via `splunk_forwarder_latest_version()`, cached for 6h). Must be set together with +# `build`, or left undef together with it - setting only one fails the catalog. # -# @param build The build identifier. Defaults to '8a94541dcfac'. +# @param build The build identifier matching `version`. Only used when `version` is given - +# ignored (and looked up automatically) when `version` is undef. Must be set together with +# `version`, or left undef together with it - setting only one fails the catalog. # # @param deploymentserver The deployment server URL. Defaults to undef. # @@ -37,21 +42,32 @@ # @groups addons addons # class common::monitor::splunk::forwarder ( - String[1] $password_hash, - Eit_types::Version $version = '7.2.4', - Optional[String] $build = '8a94541dcfac', - Stdlib::HTTPUrl $deploymentserver = undef, - Boolean $seed_password = true, - Integer $log_keep_count = 5, - Eit_types::Bytes $log_max_file_size_b = 25000000, - Boolean $enable = false, - Boolean $manage = false, - Eit_types::Noop_Value $noop_value = undef, - Hash[String[1], Hash] $addons = {}, + String[1] $password_hash, + Optional[Eit_types::Version] $version = undef, + Optional[String] $build = undef, + Stdlib::HTTPUrl $deploymentserver = undef, + Boolean $seed_password = true, + Integer $log_keep_count = 5, + Eit_types::Bytes $log_max_file_size_b = 25000000, + Boolean $enable = false, + Boolean $manage = false, + Eit_types::Noop_Value $noop_value = undef, + Hash[String[1], Hash] $addons = {}, ) { + # Must be pinned together, or the unset one falls back to splunk::params's hardcoded default and mismatches. + if ($version =~ NotUndef) != ($build =~ NotUndef) { + fail("common::monitor::splunk::forwarder: version and build must both be set or both be left undef (got version=${version}, build=${build})") + } + + # Left undef, the function tracks Splunk's latest published release instead of the pin (cached 6h). + $_latest = splunk_forwarder_latest_version($version, $build) if $manage { + class { 'profile::collector::splunk::forwarder': + version => $_latest['version'], + build => $_latest['build'], + } contain profile::collector::splunk::forwarder } } diff --git a/modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb b/modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb new file mode 100644 index 00000000..9f1fc1d1 --- /dev/null +++ b/modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require "net/http" +require "json" +require "fileutils" +require "time" + +# @summary +# +# Resolve the Splunk Universal Forwarder version+build to install. Given +# both version and build, returns them as-is (a pin). Given neither, looks +# up the latest release by scraping Splunk's own download page (there is +# no stable "latest" download URL or public API for this) - result is +# cached on disk (compiler-side) for CACHE_TTL_SECONDS so we don't hit +# Splunk's site on every catalog compile. +# +# On any lookup failure (network, parse, Splunk page layout change) this +# falls back to a stale cache if one exists, or a hardcoded last-known-good +# version if not - it never raises, since that would break catalog +# compilation for every node using this class. +# +Puppet::Functions.create_function(:splunk_forwarder_latest_version) do + DOWNLOAD_PAGE = "https://www.splunk.com/en_us/download/universal-forwarder.html" + CACHE_PATH = "/opt/obmondo/cache/splunk_forwarder_latest_version.json" + CACHE_TTL_SECONDS = 6 * 60 * 60 # 6 hours + # Used only if there is no cache at all yet AND the live fetch also fails. + FALLBACK = { "version" => "10.4.2", "build" => "33c3bf42cd73" }.freeze + + dispatch :latest do + required_param 'Optional[String[1]]', :version + required_param 'Optional[String[1]]', :build + return_type 'Struct[{version => String[1], build => String[1]}]' + end + + def latest(version, build) + return { "version" => version, "build" => build } if version && build + + cached = read_cache + return cached if cached && !stale?(cached) + + begin + fetched = fetch_latest + write_cache(fetched) + fetched + rescue StandardError => e + Puppet.warning("splunk_forwarder_latest_version: failed to fetch latest release (#{e.message}), " \ + "falling back to #{cached ? 'stale cache' : 'hardcoded default'}") + cached || FALLBACK + end + end + + private + + def fetch_latest + uri = URI(DOWNLOAD_PAGE) + response = Net::HTTP.start(uri.host, uri.port, use_ssl: true, open_timeout: 5, read_timeout: 10) do |http| + http.get(uri) + end + raise "unexpected response #{response.code}" unless response.code.to_i == 200 + + # Match the linux x86_64 rpm link, e.g. + # .../releases/10.4.2/linux/splunkforwarder-10.4.2-33c3bf42cd73.x86_64.rpm + match = response.body.match(%r{splunkforwarder-(?\d+(?:\.\d+)*)-(?[0-9a-f]{12})\.x86_64\.rpm}) + raise "could not find a version/build match on the download page" unless match + + { "version" => match[:version], "build" => match[:build] } + end + + def read_cache + return nil unless File.exist?(CACHE_PATH) + + JSON.parse(File.read(CACHE_PATH)) + rescue StandardError + nil + end + + def stale?(cached) + (Time.now - Time.parse(cached["fetched_at"])) > CACHE_TTL_SECONDS + rescue StandardError + true + end + + def write_cache(data) + FileUtils.mkdir_p(File.dirname(CACHE_PATH)) + tmp_path = "#{CACHE_PATH}.tmp" + File.write(tmp_path, data.merge("fetched_at" => Time.now.utc.iso8601).to_json) + File.rename(tmp_path, CACHE_PATH) + rescue StandardError => e + Puppet.warning("splunk_forwarder_latest_version: failed to write cache (#{e.message})") + end +end diff --git a/modules/enableit/profile/manifests/collector/splunk/forwarder.pp b/modules/enableit/profile/manifests/collector/splunk/forwarder.pp index ddebc1db..7cbf529e 100644 --- a/modules/enableit/profile/manifests/collector/splunk/forwarder.pp +++ b/modules/enableit/profile/manifests/collector/splunk/forwarder.pp @@ -25,6 +25,9 @@ Exec { noop => $noop_value, } + Service { + noop => $noop_value, + } # Create group group { 'splunkfwd': @@ -53,6 +56,12 @@ } class { 'splunk::forwarder': + # dpkg/rpm's `latest` reads the version straight from the staged source file (the archive we + # just downloaded for $version/$build) and, if it differs from what's installed, upgrades or + # downgrades in place via `dpkg -i`/`rpm -U --oldpackage` - no purge needed, and existing + # etc/system/local/* (incl. the seeded admin credentials) isn't touched since it's not a + # packaged conffile. + package_ensure => 'latest', seed_password => $seed_password, splunk_user => 'splunkfwd', password_hash => $password_hash, @@ -68,6 +77,11 @@ } } + # Default 300s exec timeout is too short for a fresh install's first start; override without patching the vendored module. + Exec <| title == 'splunk-forwarder-accept-tos' |> { + timeout => 900, + } + splunkforwarder_deploymentclient { 'target-broker:deploymentServer': setting => 'targetUri', value => $deploymentserver, From 386fbe5c6c6903b3160a5ef1cae9d598a557eb86 Mon Sep 17 00:00:00 2001 From: mdasif Date: Wed, 2 Sep 2026 15:59:23 +0530 Subject: [PATCH 2/4] Fix constant leak, unmanaged cache path, return_type violation, and write race - Constants assigned directly inside create_function's do...end block aren't lexically scoped to the function class (Class.new-style blocks don't nest in Module.nesting), so they leaked onto top-level Object in the puppetserver JRuby - collision risk with any other function using the same name, plus an "already initialized constant" warning on every environment reload. Nested under a real module instead. - /opt/obmondo/cache is only managed on agent nodes (common::init's $__opt_dir); nothing creates it on the puppetserver, where this function actually runs at compile time. write_cache would silently fail there and force a live HTTP fetch on every catalog compile. Switched to Puppet[:vardir], writable by the compiling process by construction. - P1: write_cache persisted data merged with fetched_at, and read_cache/latest returned that merged hash as-is on a cache hit - violating the declared Struct[{version, build}] return_type on every compile after the first, since Struct rejects unlisted keys. Fixed by stripping fetched_at at both return points (cached.slice(...)) while keeping it in the persisted/read hash for stale? to use. - Puppetserver compiles many catalogs concurrently in one JVM, so a fixed temp filename let concurrent writers splice/truncate each other's write before either renamed it into place. Made the temp filename unique per writer (pid + thread object_id, since pid alone doesn't differentiate threads in the same process) and added an ensure block to clean up the orphaned temp file if the write or rename fails partway. Verified all of the above against real behavior (not just review) via puppet apply: reproduced the return_type violation before the fix, confirmed it's gone after; confirmed no leftover .tmp files on a successful write; confirmed the ensure block cleans up an orphaned tmp file when rename fails after write succeeds; confirmed the constant no longer appears on Object. --- .../splunk_forwarder_latest_version.rb | 43 +++++++++++++------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb b/modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb index 9f1fc1d1..00179bb6 100644 --- a/modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb +++ b/modules/enableit/functions/lib/puppet/functions/splunk_forwarder_latest_version.rb @@ -2,7 +2,6 @@ require "net/http" require "json" -require "fileutils" require "time" # @summary @@ -19,13 +18,28 @@ # version if not - it never raises, since that would break catalog # compilation for every node using this class. # -Puppet::Functions.create_function(:splunk_forwarder_latest_version) do +# Real module keyword (not the create_function do...end block below) so these get proper +# lexical scoping - constants assigned directly inside create_function's block leak onto +# top-level Object in the puppetserver JRuby, since Class.new-style blocks don't nest in +# Module.nesting the way `module`/`class` do. +module SplunkForwarderLatestVersion DOWNLOAD_PAGE = "https://www.splunk.com/en_us/download/universal-forwarder.html" - CACHE_PATH = "/opt/obmondo/cache/splunk_forwarder_latest_version.json" CACHE_TTL_SECONDS = 6 * 60 * 60 # 6 hours # Used only if there is no cache at all yet AND the live fetch also fails. FALLBACK = { "version" => "10.4.2", "build" => "33c3bf42cd73" }.freeze + # Puppet[:vardir] is writable by the compiling process by construction, unlike + # /opt/obmondo/cache - which nothing in this repo creates or manages on the + # puppetserver (only on agent nodes, via common::init's $__opt_dir), so + # write_cache would silently fail there and force a live HTTP fetch on every + # catalog compile. Puppet[:vardir] is a runtime setting, not a static + # literal, so this is a method rather than a constant. + def self.cache_path + File.join(Puppet[:vardir], "splunk_forwarder_latest_version.json") + end +end + +Puppet::Functions.create_function(:splunk_forwarder_latest_version) do dispatch :latest do required_param 'Optional[String[1]]', :version required_param 'Optional[String[1]]', :build @@ -36,7 +50,7 @@ def latest(version, build) return { "version" => version, "build" => build } if version && build cached = read_cache - return cached if cached && !stale?(cached) + return cached.slice("version", "build") if cached && !stale?(cached) begin fetched = fetch_latest @@ -45,14 +59,14 @@ def latest(version, build) rescue StandardError => e Puppet.warning("splunk_forwarder_latest_version: failed to fetch latest release (#{e.message}), " \ "falling back to #{cached ? 'stale cache' : 'hardcoded default'}") - cached || FALLBACK + cached ? cached.slice("version", "build") : SplunkForwarderLatestVersion::FALLBACK end end private def fetch_latest - uri = URI(DOWNLOAD_PAGE) + uri = URI(SplunkForwarderLatestVersion::DOWNLOAD_PAGE) response = Net::HTTP.start(uri.host, uri.port, use_ssl: true, open_timeout: 5, read_timeout: 10) do |http| http.get(uri) end @@ -67,25 +81,30 @@ def fetch_latest end def read_cache - return nil unless File.exist?(CACHE_PATH) + return nil unless File.exist?(SplunkForwarderLatestVersion.cache_path) - JSON.parse(File.read(CACHE_PATH)) + JSON.parse(File.read(SplunkForwarderLatestVersion.cache_path)) rescue StandardError nil end def stale?(cached) - (Time.now - Time.parse(cached["fetched_at"])) > CACHE_TTL_SECONDS + (Time.now - Time.parse(cached["fetched_at"])) > SplunkForwarderLatestVersion::CACHE_TTL_SECONDS rescue StandardError true end def write_cache(data) - FileUtils.mkdir_p(File.dirname(CACHE_PATH)) - tmp_path = "#{CACHE_PATH}.tmp" + # No mkdir_p needed - Puppet[:vardir] always exists already. Unique per-writer temp filename, + # since puppetserver compiles many catalogs concurrently in one JVM (Process.pid alone + # wouldn't differentiate threads in the same process) - a shared fixed temp name would let + # concurrent writers splice/truncate each other's write before either renames it into place. + tmp_path = "#{SplunkForwarderLatestVersion.cache_path}.#{Process.pid}.#{Thread.current.object_id}.tmp" File.write(tmp_path, data.merge("fetched_at" => Time.now.utc.iso8601).to_json) - File.rename(tmp_path, CACHE_PATH) + File.rename(tmp_path, SplunkForwarderLatestVersion.cache_path) rescue StandardError => e Puppet.warning("splunk_forwarder_latest_version: failed to write cache (#{e.message})") + ensure + File.unlink(tmp_path) if tmp_path && File.exist?(tmp_path) end end From 214cb9f98afdb026f219a0cca98e0bf3326c3035 Mon Sep 17 00:00:00 2001 From: mdasif Date: Wed, 2 Sep 2026 16:06:02 +0530 Subject: [PATCH 3/4] Hardcode hiera datadir to abbnoa6nlk/splunk_forwarder_10_2_2 for testing Temporary: points datadir at the test hiera-data branch directly for continued testing on gbsherepo01.abbnoa6nlk. Revert before merging to master - this affects Hiera lookups for every node compiling against this environment, not just the one under test. --- hiera.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hiera.yaml b/hiera.yaml index 989092ee..e33a5d9a 100644 --- a/hiera.yaml +++ b/hiera.yaml @@ -4,7 +4,7 @@ version: 5 defaults: # relative path as per puppetserver on k8s, defaulting to main branch # in case of opensource the customer_id will be empty - datadir: ../../hiera-data/%{hiera_datapath} + datadir: ../../hiera-data/abbnoa6nlk/splunk_forwarder_10_2_2 data_hash: yaml_data hierarchy: From 56b7ddbd317c5af24ca63fae2d2651a5a9810693 Mon Sep 17 00:00:00 2001 From: mdasif Date: Wed, 2 Sep 2026 16:39:22 +0530 Subject: [PATCH 4/4] Revert hiera.yaml datadir test hack back to %{hiera_datapath} --- hiera.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hiera.yaml b/hiera.yaml index e33a5d9a..989092ee 100644 --- a/hiera.yaml +++ b/hiera.yaml @@ -4,7 +4,7 @@ version: 5 defaults: # relative path as per puppetserver on k8s, defaulting to main branch # in case of opensource the customer_id will be empty - datadir: ../../hiera-data/abbnoa6nlk/splunk_forwarder_10_2_2 + datadir: ../../hiera-data/%{hiera_datapath} data_hash: yaml_data hierarchy: