Skip to content

Modernize CI to OpenVox 8 / Ruby 3.2+4.0 and drop legacy Windows support - #136

Merged
silug merged 4 commits into
ayohrling:mainfrom
michael-riddle:issue_135
Jul 21, 2026
Merged

Modernize CI to OpenVox 8 / Ruby 3.2+4.0 and drop legacy Windows support#136
silug merged 4 commits into
ayohrling:mainfrom
michael-riddle:issue_135

Conversation

@michael-riddle

@michael-riddle michael-riddle commented Jul 21, 2026

Copy link
Copy Markdown

Replace the Puppet 6/7 CI matrix with OpenVox 8 on Ruby 3.2 and Ruby 4.0. This required replacing the Gemfile's per-Ruby-minor-version PDK meta-gems
(which don't exist for Ruby 4.0, and were the root cause of the earlier patron/libcurl CI failures) with voxpupuli-test and an explicit openvox
dependency, matching the pattern used by pupmod-simp-useradd and pupmod-voxpupuli-kmod. Rakefile, .rubocop.yml, and spec/spec_helper.rb are updated
accordingly:

  • Rakefile now loads tasks via voxpupuli/test/rake
  • .rubocop.yml inherits voxpupuli-test's shared config instead of the frozen PDK-era cop list
  • spec_helper.rb shims PuppetlabsSpec::Fixtures#my_fixture (dropped by the new toolchain) and fixes a YAML.safe_load call broken by modern Psych's
    keyword-arg signature
  • check:symlinks is intentionally dropped from the syntax/lint rake invocation in .github/workflows/pr.yml — that task no longer exists under the
    modernized toolchain (confirmed via bundle exec rake --tasks); the remaining checks (check:git_ignore, check:dot_underscore, check:test_file) are
    unaffected

Also drop support for Windows 10, Server 2012, and Server 2012 R2, and add support for Windows Server 2025 (metadata.json, new win2025.yml nodeset). This
is a breaking change, so bump to 2.0.0.

Rubocop offenses: the newer rubocop/rubocop-rspec versions pulled in by voxpupuli-test surfaced ~70 pre-existing style offenses across vendored/spec code
that the old frozen PDK-era config never caught. These were initially grandfathered via a generated .rubocop_todo.yml so CI would pass, then fixed
properly in a follow-up commit and the todo file removed entirely — the repo now has zero rubocop offenses with no grandfathering. Each autocorrection was
individually reviewed for behavioral equivalence and the full test suite (69 examples) was confirmed unchanged throughout. A handful of offenses were
deliberately kept as documented exceptions in .rubocop.yml rather than "fixed", because the fix would have been riskier than the offense — see the second
commit message for details (vendored inifile.rb internals, legitimate existing conventions, and diagnostic test output).

Found but not fixed: IniFile#taint/#tainted? in the vendored inifile.rb call Object#taint, which was removed entirely in Ruby 3.2 (our new target
version). This is dead code today — nothing in this module calls .dup/.clone/.taint on an IniFile — but it will raise NoMethodError if ever exercised.
Left as-is since fixing it means rewriting vendored logic, flagging here for a maintainer decision.

michael-riddle and others added 2 commits July 21, 2026 15:32
Replace the Puppet 6/7 CI matrix with OpenVox 8 on Ruby 3.2 and Ruby
4.0. This required replacing the Gemfile's per-Ruby-minor-version PDK
meta-gems (which don't exist for Ruby 4.0, and were the root cause of
the earlier patron/libcurl CI failures) with voxpupuli-test and an
explicit openvox dependency, matching the pattern used by
pupmod-simp-useradd and pupmod-voxpupuli-kmod. Rakefile, .rubocop.yml,
and spec/spec_helper.rb are updated accordingly:
- Rakefile now loads tasks via voxpupuli/test/rake
- .rubocop.yml inherits voxpupuli-test's shared config instead of the
  frozen PDK-era cop list; the newer rubocop/rubocop-rspec versions
  surface ~70 pre-existing style offenses across vendored/spec code,
  grandfathered via a generated .rubocop_todo.yml rather than risk
  autocorrecting the vendored inifile.rb
- spec_helper.rb shims PuppetlabsSpec::Fixtures#my_fixture (dropped by
  the new toolchain) and fixes a YAML.safe_load call broken by
  modern Psych's keyword-arg signature

Also drop support for Windows 10, Server 2012, and Server 2012 R2, and
add support for Windows Server 2025 (metadata.json, new win2025.yml
nodeset). This is a breaking change, so bump to 2.0.0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Applies safe and verified-safe autocorrections (whitespace, redundant
syntax, string interpolation, RSpec equivalences like eq(false) ->
be(false) and .exactly(1).times -> .once), each checked for behavioral
equivalence and confirmed via the full test suite (69 examples, 0
failures, 2 pendings, unchanged throughout).

A few offenses were deliberately kept as documented exceptions in
.rubocop.yml rather than "fixed", because the fix would have been
riskier than the offense:
- has_section? in the vendored inifile.rb (Naming/PredicatePrefix) is
  unused internally but is synced/vendored public API
- the character-escaping case/when in the same file (Style/HashLikeCase)
- RSpec/MultipleMemoizedHelpers and the top-level `include` in
  spec_helper.rb (Style/MixinUsage) are legitimate existing conventions
- two diagnostic puts/ap lines in a real (non-pending) spec, restored
  after autocorrect deleted them

Also: replaced the leaky top-level provider_class local variable with
RSpec's described_class idiom, removed a no-op empty after(:suite)
block, and gave both pending `xit` tests real reasons instead of
RSpec's generic default message -- verified empirically why each is
skipped rather than guessing.

Found but did not fix: IniFile#taint/#tainted? call Object#taint,
which was removed entirely in Ruby 3.2 (our new target version). Dead
code today -- nothing in this module calls .dup/.clone/.taint on an
IniFile -- but will raise NoMethodError if ever exercised.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@silug

silug commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Review: Modernize CI to OpenVox 8 / Ruby 3.2+4.0 and drop legacy Windows support

Overview

Solid, well-scoped modernization. It swaps the Puppet 6/7 CI matrix (with its fragile per-Ruby-minor PDK meta-gems and the libcurl/patron workaround) for OpenVox 8 on Ruby 3.2 + 4.0 via voxpupuli-test, migrates the RuboCop config to the shared voxpupuli gem, fixes toolchain breakage in spec_helper.rb, drops EOL Windows targets, adds Server 2025, and bumps to 2.0.0. The Ruby refactors are almost entirely style-driven and behavior-preserving. +157/−623 is a healthy direction.

Correctness — verified behavior-preserving

  • policy.rb fixup_value :principal refactor — the branches are inverted (!~ ? convert : keep=~ ? keep : convert) but logically identical, and '*' + sid.to_s vs "*#{sid}" are equivalent. .sort.join(',') preserved. ✅
  • super(value)super — bare super forwards the same arg. Equivalent. ✅
  • def initialize; end removal in security_policy.rb — empty override, safe to drop; SecurityPolicy.new still works. ✅
  • YAML modernizationsafe_load(read(f), [], [], true)safe_load_file(f, permitted_classes:, permitted_symbols:, aliases:) is the correct Psych 4 signature. ✅
  • 99_hiera_spec.rb — merging two hosts.each loops preserves per-host assertions. ✅

Issues / risks

  • PR description references a .rubocop_todo.yml that isn't in the branch. The body says ~70 pre-existing offenses were "grandfathered via a generated .rubocop_todo.yml," but no such file exists in the changed files or the repo. In reality the offenses appear to have been fixed inline (the refactors + # rubocop:disable comments). Not a code bug, but the description is misleading and should be corrected so nobody assumes there's a safety net that isn't there.

  • check:symlinks was silently dropped from the lint job (rake syntax lint metadata_lint check:symlinks check:git_ignore ... → without check:symlinks). This isn't mentioned in the PR body. If it was dropped because the new toolchain no longer provides the task, fine — but please call it out; otherwise it's a quiet reduction in CI coverage.

  • my_fixture_dir shim can NoMethodError on nil. In spec_helper.rb:

    path = callers.find { |c| c =~ %r{_spec\.rb} }
    path = path.split(%r{:\d+}).first   # NoMethodError if find returned nil

    If my_fixture is ever called from a non-_spec.rb frame, this raises a confusing error instead of a clear one. Low risk (matches the spirit of the original), but a guard or clearer message would be more robust.

  • metadata.json requirement renamed puppetopenvox. Consistent with the useradd/kmod precedent and intentional, but be aware Puppet Forge's metadata validator historically only special-cases puppet as a requirement name. Worth confirming metadata_lint stays green on the new matrix.

Minor / non-blocking

  • actions/checkout@v2 is pre-existing and out of scope, but since you're already modernizing CI, bumping to @v4 while you're here would retire a Node 16 deprecation warning.
  • openvox '~> 8.0' default now hardcodes the major line in two places (Gemfile + metadata) — maintenance note for the next bump.
  • Version bump to 2.0.0 for the breaking Windows/Puppet drop is correct per SemVer; CHANGELOG entries are clear and appropriately marked BREAKING.

Test coverage

Good. The two xitit ... skip: conversions with explanatory reasons are an improvement over silent xit (the "provider confinement returns 0 instances off-Windows" note is genuinely useful). receive_messages consolidation and described_class usage are idiomatic. No loss of assertions anywhere I can see.

Verdict

Approve with minor changes. No correctness regressions found. Before merge I'd (1) fix the PR description's .rubocop_todo.yml claim, and (2) confirm the check:symlinks removal was intentional and note it. Everything else is optional polish.

🤖 Generated with Claude Code

michael-riddle and others added 2 commits July 21, 2026 16:10
Per review feedback: confirm and record that this was intentional,
not an accidental omission. That rake task no longer exists under
the voxpupuli-test toolchain, unlike the other check: tasks kept
alongside it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review feedback on PR ayohrling#136:
- my_fixture_dir raised a confusing NoMethodError on nil if ever
  called from a non-*_spec.rb frame. Now raises a clear error instead.
  Verified this doesn't affect the two files that actually use it.
- Bumped actions/checkout@v2 -> @v4 in both CI jobs (flagged as minor
  polish; retires a Node 16 deprecation warning while touching CI
  anyway).

Also verified and can confirm: `rake metadata_lint` passes cleanly
with the openvox requirement name -- metadata-json-lint doesn't
restrict requirements[].name to an allowlist, so the puppet -> openvox
rename isn't a validation concern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@silug
silug merged commit eb419fe into ayohrling:main Jul 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants