Move Mercurial version Hiera lookup into the profile#1214
Open
jwmossmoz wants to merge 3 commits into
Open
Conversation
The lookup belongs in the profile per the repo's roles & profiles architecture rule: profiles do Hiera lookups and pass values to module classes as parameters. The win_packages::mercurial class now takes $version as a parameter; the lookup moves to roles_profiles::profiles::mercurial. The previous $srcloc lookup inside win_packages::mercurial was unused (win_msi_pkg constructs the URL from its own ext_pkg_src lookup), so it's dropped rather than threaded through. Fixes #521
The win_msi_pkg, win_exe_pkg, and win_zip_pkg defined types previously
called lookup('windows.ext_pkg_src') inline, violating the repo's roles
& profiles rule (lookups belong in profiles, then values pass into
modules as parameters). Same pattern existed for:
- win_packages::chrome (windows.googlechrome.version lookup)
- win_packages::drivers::intel_gfx (ext_pkg_src lookup)
- win_mozilla_build::install (windows.mozilla_build.version lookup)
All three defined types and every wrapper class that calls them now
take String $srcloc as a parameter. Profiles do the lookup and pass
the value through.
win_packages::sevenzip is required transitively by win_zip_pkg via
Puppet's `require` keyword, which cannot pass parameters, so its
srcloc is supplied via Hiera class-parameter auto-lookup in
data/os/Windows.yaml. Every other class is invoked explicitly from a
profile and receives srcloc that way.
Also moves win_packages::chrome and win_mozilla_build::install version
lookups into their profiles.
Addresses two issues found in adversarial review of the prior commit:
1. The profile was declaring `win_mozilla_maintenance_service::install`
before the parent class that initializes `$source_exe`. Compile-time
evaluation order meant install would see source_exe as undef, locking
in a broken `creates =>` path.
2. The profile was declaring `win_openssh::install` alongside but not
replacing the implicit declaration triggered by sibling
`win_openssh::service` / `::configuration`. The install class also
reads `$win_openssh::ssh_program_data` and `::pwrshl_run_script` from
a parent that is never declared anywhere (pre-existing gap, not
created by this PR).
Fix: have each affected parent class take `$srcloc` and call
`class { '...::install': srcloc => $srcloc }` instead of bare `include`.
Profiles now pass `srcloc` to the parent only; no standalone install
declarations. For `win_openssh` — whose parent is never declared by any
profile — supply `win_openssh::install::srcloc` via Hiera class-parameter
auto-lookup, the same pattern used for `win_packages::sevenzip`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
win_packages::mercurialand intoroles_profiles::profiles::mercurial, matching the repo's roles & profiles rulewindows.ext_pkg_srcout ofwin_*_pkgdefined types and every wrapper class that calls them; profiles now do the lookup and passsrclocas a parameterwindows.googlechrome.versionlookup out ofwin_packages::chrome(still hits chocolatey on install)windows.mozilla_build.versionlookup out ofwin_mozilla_build::installwin_packages::sevenzip::srclocHiera class-parameter key for the one case whererequiresemantics prevent explicit param passingResult: zero
lookup()calls remain in anywin_packages,win_nxlog,win_ultravnc,win_nsclient,win_openssh,win_mozilla_build::install,win_mozilla_maintenance_service::install, orwin_os_settings::disable_monitor2manifest.Audit of package availability
Verified before the refactor that every package URL
windows.ext_pkg_srcwould build is reachable on the Azure blob today (24 of 25 HEAD-probed; the one miss,Git-2.49.0-64-bit.exefromwin_packages::git, is a dead class with no callers — not in scope here). Windows hardware pools share the sameext_pkg_srcas Azure pools (no per-worker-type override exists indata/os/Windows/worker/).Files touched
win_msi_pkg,win_exe_pkg,win_zip_pkg)win_packages,win_nxlog,win_ultravnc,win_nsclient,win_mozilla_build,win_mozilla_maintenance_service,win_openssh,win_os_settingsroles_profiles::profilesdata/os/Windows.yamlTest plan
puppet-validateandpuppet-lintpass on every changed file (via prek)win_msi_pkg/win_exe_pkg/win_zip_pkgcall site verified to passsrcloc =>win116424h2azure(or equivalent) before mergeFixes #521