Puppet 8 modernization: add sensu_pipeline type, fix annotations, drop EOL platforms

[sourabhpatel-sumo]

Description

Modernizes the module for Puppet 8 and current OS support. Adds sensu_pipeline resource type
and pipeline support in sensu_check. Fixes annotations to accept mixed value types. Drops EOL
platforms.

The sensu-plugin-ruby removal originally scoped here has been split into a follow-up,
stacked PR targeting 6.0.0 (plugin-removal, based on this branch) — it's a breaking change
with no prior deprecation cycle and shouldn't ride along with this otherwise non-breaking
5.12.0 release. sensu_bonsai_asset is the documented replacement (README.md).

Related Issues

• Update dependency limits #1325 — stdlib dependency too restrictive, conflicted with other modules requiring >= 7.0.0
• Deprecated functions #1347 — deprecated to_yaml function replaced with stdlib::to_yaml in backend.pp
• [question] sensu_check Puppet resource does not support pipelines parameter. #1346 — sensu_check lacked pipelines parameter; resolved by adding pipelines/fallback_pipeline properties and new sensu_pipeline type
• Annotations data type is too restrictive #1345 — annotations data type was too restrictive (strings only); resolved by allowing Array/Hash values

Motivation and Context

Puppet 6/7 and the previously supported OS platforms are EOL.

Breaking / behavior changes

• sensu_user's configure_url default changed from http:// to https://. A sensu_user
  resource that doesn't set configure_url explicitly (outside sensu::backend, which already
  derives it correctly from use_ssl) will now attempt HTTPS. On use_ssl => false deployments
  this will fail unless configure_url is set explicitly.
• tasks/install_agent_windows.ps1's $PackageSource no longer has a hardcoded fallback MSI
  URL (the old default pointed at a stale Sensu Go 5.13.1 build) — it must now be supplied, and
  fails with a clear error if it's missing rather than a stale silent default.
• appveyor.yml is removed (CI has moved to GitHub Actions); .pdkignore/.sync.yml updated
  to match.

How Has This Been Tested?

• bundle exec rake sensu_validate lint strings:generate reference — passes clean
• bundle exec rake spec — 3719 examples, 0 failures (spec/unit + spec/classes)
• Acceptance tests:
  • rocky-9 base mode — clean except sensu::plugins, which is skip-marked (see below)
  • rocky-8-cluster — 24 examples, 0 failures (exercises the EL8 repo.pp path specifically)
  • debian-12 base mode — 77 examples, 0 failures, 14 pending
• Ruby 3.4 / Puppet 8 on macOS (arm64)

sensu::plugins acceptance coverage (spec/acceptance/04_plugins_spec.rb) is intentionally
skip-marked: the legacy packagecloud.io/sensu/community repo has no sensu-plugins-ruby
package for any OS this module currently supports — confirmed failing identically across
rocky-9/debian-12/almalinux-9/ubuntu-22.04. This is exactly why the subsystem is being removed
in the follow-up PR rather than fixed here.

General

• Update README.md with any necessary configuration snippets
• New parameters are documented
• New parameters have tests
• Tests pass - bundle exec rake sensu_validate lint spec

[sarvodaya-kumar-26]

Code Review

Solid modernization PR — tests are comprehensive and the scope is well-documented. Found a few issues worth addressing before merge, ranging from a potential data-loss risk to minor code quality items.

[amdprophet]

Review

Thanks for taking this on — the module is unusable on Puppet 8 today and this fixes that. The sensu_pipeline type, the annotations fix (#1345), and the stdlib::to_yaml fix (#1347) all look right, and I verified the new sensu_check properties against upstream (details below). Most of my comments are about a few changes that aren't mentioned in the description and are riskier than the rest of the PR.

Reviewed at 211 files, +4,122 / −4,980. I read the non-spec diff in full and checked the new Sensu fields against the sensu/core protos and sensu-go v6.14.2.

---

Blocking

1. manifests/repo.pp — the EL8 branch disables all package signature verification

exec { 'disable sensu gpg check':
  command => 'sed -i "s/gpgcheck=1/gpgcheck=0/g" /etc/yum.repos.d/sensu_stable.repo',
  unless  => 'grep -q "gpgcheck=0" /etc/yum.repos.d/sensu_stable.repo',
}

I fetched the file the packagecloud script actually installs (config_file.repo?os=el&dist=8). It contains both repo_gpgcheck=1 and gpgcheck=1. Running that sed against it gives:

 4:repo_gpgcheck=0     <- collateral damage from the substring match
 5:gpgcheck=0
16:repo_gpgcheck=0
17:gpgcheck=0

The pattern gpgcheck=1 matches inside repo_gpgcheck=1, so it turns that off too. And the unless guard doesn't match on a fresh install, so this does run.

That matters because packagecloud signs repo metadata, not the RPMs — repo_gpgcheck=1 is the only signature check available here. It's why the else branch on line 47 deliberately keeps repo_gpgcheck => 1, gpgcheck => 0. The EL8 path discards the one check the non-EL8 path preserves, as an unintended side effect.

Combined with curl -s ... | bash (line 24) running unpinned remote code as root, EL8 nodes end up installing Sensu with no cryptographic verification at all.

The yumrepo in the else branch already handles el/8 — the baseurl scheme is identical. Unless there's a specific EL8 failure driving this, I'd delete the whole if String($repo_release) == '8' branch. If there is one, could you describe it in the PR? From the diff the special case doesn't have an obvious motivation.

Worth noting your reported acceptance runs were Rocky 9 and Ubuntu 24.04, so this branch — the riskiest change in the PR — wasn't covered by them.

Minor, same block: package { 'curl': } on line 18 is unnamespaced and will duplicate-declare on any node that manages curl elsewhere.

2. manifests/backend.pp — dropping api-url breaks TLS for the backend's internal client

The new $default_config removes the api-url key that master emitted as api-url: https://<certname>:8080.

In sensu-go v6.14.2 backend/cmd/start.go, flagAPIURL defaults to "http://localhost:8080" — plaintext. Since the module defaults to use_ssl => true, the API listener serves TLS while the backend's own client now falls back to plain HTTP against it.

The spec change makes this read as intentional rather than flagging it:

-          |api-url: https://test.example.com:8080

Suggest restoring it, derived from use_ssl.

On the rest of the new $default_config: I diffed all eight hardcoded keys against start.go and seven are byte-identical to the upstream defaults (agent-port: 8081, api-listen-address: "[::]:8080", the etcd URLs, etcd-initial-cluster-state: new). Only etcd-advertise-client-urls differs (127.0.0.1 vs upstream localhost), which is benign. So the values are fine — it's specifically the api-url removal that regresses. See also the partial-override note below.

3. manifests/agent.pp:318-368 — permanently shadowing the packaged systemd unit

This writes a hand-rolled unit to /etc/systemd/system/sensu-agent.service, which takes precedence over /usr/lib/systemd/system/sensu-agent.service indefinitely. Every future upstream change to that unit — hardening flags, dependency ordering, LimitNOFILE — is silently ignored on managed nodes.

Four issues in that block:

• User=sensu / Group=sensu are hardcoded (lines 334-335), but sensu::user and sensu::group are documented public parameters (init.pp:70,72). Setting them now produces a broken unit.
• The ExecStart on line 337 is dead. The systemd::dropin_file immediately below clears it (ExecStart=) and re-sets it from $service_path / $sensu::agent_config_path. Two sources of truth, one of which never takes effect.
• Exec['systemd-reload'] (line 351) is an unnamespaced global title, likely to collide in a real catalog. It's also redundant — systemd::dropin_file already triggers a daemon-reload.
• $_env_file_line is emitted twice — line 336 in the unit, line 364 in the dropin.

spec/classes/agent_spec.rb isn't in the diff, and the existing coverage only asserts contain_systemd__dropin_file('sensu-agent-start.conf') — nothing exercises the new unit file.

If the dropin already worked on master, I'd drop this block entirely. If it didn't, systemd::unit_file with parameterized user/group is the right tool, plus spec coverage.

---

Should fix

Version should be 6.0.0, not 5.12.0

To be clear about what I'm not claiming: dropping Puppet 6/7 as a minor is consistent with this module's own precedent — v5.8.0 dropped Puppet 5 (2947ca8d) and v5.9.0 removed EOL OSes (8f224ab9), both minors. No objection there.

The issue is the plugin subsystem, which is separate:

Removed	Status on master
sensu::plugins	Public, REFERENCE.md:1118, ~90 lines of README with six worked examples
sensu_plugin type	Public, REFERENCE.md:4070 + six documented parameters
sensu_gem package provider	Public (package { provider => sensu_gem })

None of these carry a deprecation warning on master. The description says the mechanism is "deprecated," but that's not signalled in code — a user on 5.11.1 gets no warning, then a compilation failure on 5.12.0. This project's pattern has been deprecate-then-remove-at-a-major (v5.0.0 carried #1273 "Remove various deprecations").

Since sensu_bonsai_asset already exists and you've documented the migration at README.md:892, I don't think a deprecation cycle is needed — just bump to 6.0.0 and list the removals. Alternatively, split the PR: Puppet 8 migration as 5.12.0 (urgent, uncontroversial, ships immediately) and the plugin removal as 6.0.0. That would also let the systemd and repo changes bake separately.

tasks/install_agent_windows.ps1 — undisclosed breaking change

$PackageSource becomes Mandatory = $True and the hardcoded MSI URL is removed. Existing Windows task invocations that omit it now fail at parameter binding. The two schemas also disagree: install_agent_windows.json declares String[1] (required) while install_agent.json declares Optional[String[1]].

Missing test coverage for new code

• spec/unit/provider/sensu_pipeline/ doesn't exist. Two new providers (~135 lines each) ship with zero unit specs, while every other type in the module has them.
• No specs anywhere for puppet_x/sensu/*, despite this PR changing insync? and adding validate/munge to HashProperty. Those are exactly the changes that want direct tests.

ArrayOfHashesProperty#insync? is order-sensitive

is.map(&strip_nils) == should.map(&strip_nils)

pipelines, subdues, and output_metric_thresholds are all semantically sets. If sensuctl returns them in a different order than declared, every run reports drift and re-applies. Suggest sorting by a stable key before comparing.

Dead labels filter in HashProperty#insync?

The sensu.io/managed_by filter added there can't run: all labels properties use HashOfStringsProperty, which already has an identical filter. HashProperty only parents annotations, fallback_pipeline, and subdue. Worth removing so the next reader doesn't assume it's load-bearing.

sensu-backend init lost both its config file and its readiness guard

-  'command' => 'sensu-backend init --config-file /etc/sensu/backend.yml',
-  'require' => 'Sensu_api_validator[sensu]',
+  'command' => 'sensu-backend init',
+  'require' => 'Service[sensu-backend]',

Without --config-file, init uses its own defaults rather than the managed backend.yml, which diverges as soon as anyone sets config_hash. And the service being started isn't the same as the API being ready — Sensu_api_validator was bridging exactly that gap. This looks like a source of flaky first-run failures.

etcd partial-override footgun

Because all eight etcd keys are now always present in backend.yml, an operator who overrides only etcd-initial-cluster via config_hash silently keeps advertising http://127.0.0.1:2380 as their peer URL. tests/sensu-backend-cluster.pp:16-23 overrides all eight, so acceptance won't catch it. Either omit these keys when unset, or validate that they're set as a group.

sensu_api.rb — nil-URL fallback

uri = url ? URI.join(url, path) : URI("http://localhost#{path}"), three times. A nil URL is a config bug; silently retargeting to plaintext localhost turns a loud failure into a confusing one. Suggest raising.

sensu_user.rb — configure_url default flips http:// → https://

Right direction, but it's an undocumented behavior change. Worth a line in the description.

CI coverage

• The validate step drops strings:generate reference, so REFERENCE.md drift is no longer enforced in CI. I can see you run strings:validate:reference locally and REFERENCE.md is in fact fully in sync in this PR — it's the next PR I'd worry about.
• Both .fixtures-latest.yml allow-failure jobs are deleted. Collapsing the puppet dimension 4 → 1 is correct now, but those canaries were testing against latest stdlib/postgresql/systemd, not older Puppet. Worth keeping one.
• parallel_spec → spec slows CI and leaves parallel_tests unused in the Gemfile.

Dependency bounds contradict each other

metadata.json pins stdlib >= 9.0.0 < 10.0.0, but tests/provision_basic_el.sh and provision_basic_debian.sh install >= 8.0.0 < 11.0.0. Acceptance can pass against a stdlib version the module declares unsupported.

tests/generate-etcd-ssl.sh — unverified download

curl -sL of cfssl 1.6.5 into /usr/local/bin with no checksum or signature. Test-only, but it's a root-owned binary on a fresh box — a sha256sum -c would be cheap.

Smaller items

• parse_yaml_dump's rescue falls back to output.split('---').map { YAML.load(d) } — the exact broken behavior being fixed. It'll mangle any document containing --- in a value.
• Five providers (sensu_cluster_federation, ..._member, sensu_etcd_replicator, sensu_postgres_config, sensu_secrets_vault_provider) inline the dump+parse logic instead of calling dump(), and inconsistently (self.parse_yaml_dump vs Puppet::Provider::Sensuctl.parse_yaml_dump). dump() still exists, now used once.
• SENSU_PIPELINE_VALIDATE_REF is a top-level global constant in lib/puppet/type/sensu_pipeline.rb — worth moving inside the type or namespacing.
• def self.sensuctl(args, failonfail: nil, ..., **_kwargs) — all call sites are migrated correctly, but the catch-all will silently swallow typo'd option names in future ones.
• URI.escape(name, '/') → name.gsub('/', '%2F') escapes only /. Fine for Bonsai's namespace/name; URI.encode_www_form_component is the drop-in if names ever widen.
• .rubocop.yml sets TargetRubyVersion: '3.4', but puppet-agent 8 ships Ruby 3.2 — should probably be 3.2, or you'll accept syntax the agent can't parse.
• metadata.json removes pdk-version / template-url / template-ref while .sync.yml stays. Worth finishing the move away from modulesync or keeping the keys.
• The Gemfile drops eight puppet-lint plugins including legacy_facts and top_scope_facts — both directly useful for a Puppet 8 migration.
• Amazon 2023 is added to the supported-OS list and mapped to repo_release = '9', but spec/fixtures/facts/amazon-2-x86_64.facts and amazon-2018-x86_64.facts are deleted with no 2023 fixture added, so the new mapping is untested.

---

Nits

• RUN_ACCEPTANCE: 1 is set in the acceptance workflows and read nowhere in the repo.
• acceptance-full's exclude: {puppet: puppet8, ci_build: yes} is a no-op after the matrix rewrite.
• fallback_pipeline requires name/type/api_version but doesn't reject unknown keys, while pipelines and subdues do. Same shape, different strictness.
• BEAKER_debug: true → false will make CI failures harder to diagnose.
• The cluster acceptance set drops use_agent: yes without explanation.

---

What's good here

• The new sensu_check properties are all real. I checked each against sensu/core v2/*.proto: subdue = 16, pipelines = 32, output_metric_thresholds = 33, subdues = 34, fallback_pipeline = 35, ttl_status = 36, asset_status = 37, plus Pipeline.ContinueOnError = 3. The Puppet property types map correctly to the wire types (ttl_status→Integer for int32, asset_status→Array for repeated string, etc.). Validation is more thorough than it had to be — the nested days/day-name checks on subdue and the thresholds/tags structure on output_metric_thresholds are good. Only leniencies: min/max aren't validated as Strings and repeat elements aren't type-checked.
• Two spec files renamed into existence: spec/acceptance/sensu_bonsai_asset.rb → _spec.rb and spec/unit/sensu_license.rb → _spec.rb. Those assertions had never run. Nice catch.
• REFERENCE.md is genuinely complete and in sync — subdue (2615), subdues (2629), sensu_pipeline (4091+), continue_on_error (4140), all seven keepalive_*, and zero stale references to the removed plugin API.
• The README migration note at line 892 pointing sensu::plugins users at sensu_bonsai_asset is the right way to do a removal.
• Test certs are valid through Aug 2036 with current CRLs — no hidden expiry cliff.
• Both Ruby shims (spec/vendor/archive/tar/minitar.rb, spec/openssl_unfreeze.rb) are small, commented, explain why they exist, and are excluded from the published module by .pdkignore. That's the right way to carry a workaround.
• Docker image caching + timeout-minutes in acceptance, and the local-run docs in tests/README.md (Apple Silicon user_agent workaround, BEAKER_DESTROY=no, corporate CA note) will save the next contributor real time.

---

Suggested landing path

1. Delete the repo.pp EL8 branch and let yumrepo handle el/8.
2. Restore api-url, derived from use_ssl.
3. Drop the hand-rolled systemd unit — or parameterize user/group, namespace the exec, remove the dead ExecStart, and add agent_spec.rb coverage.
4. Add spec/unit/provider/sensu_pipeline/{sensuctl,sensu_api}_spec.rb.
5. Bump to 6.0.0 and enumerate the removals, or split the plugin removal into its own PR.
6. Restore strings:generate reference to the validate step and keep one .fixtures-latest.yml canary.

The pipeline support and the Ruby 3 migration are solid and I'd take them as-is. The systemd and repo changes read more like debugging artifacts that survived into the diff — I'd want those split out and justified separately.

---

_{Caveat: I couldn't run the suite locally (no Ruby ≥ 3.2 available on this machine), so all of the above is from static reading plus verification against the sensu/core protos, sensu-go v6.14.2 backend/cmd/start.go, the live packagecloud config file, and Puppet's exec/posix provider. I read the non-spec diff in full; spec and nodeset diffs by diffstat and targeted greps. Review assisted by Claude Code.}

[sourabhpatel-sumo]

@amdprophet @sarvodaya-kumar-26 — pushed updates addressing both reviews:

• Split the plugin removal into Remove deprecated sensu-plugins-ruby subsystem (v6.0.0) #1359 (stacked on this branch, targets 6.0.0), per @amdprophet's suggestion — this PR stays non-breaking at 5.12.0.
• Fixed all blocking items: etcd-initial-cluster-state data-loss risk, curl | bash + GPG-disable on EL8, sensu-backend init --config-file regression, SENSU_PIPELINE_VALIDATE_REF namespaced under PuppetX::Sensu, YAML.safe_load with permitted_classes on both sensuctl.rb call sites, hand-rolled systemd unit removed (dropin only remains).
• sensu_user's configure_url http→https change is now disclosed in the description's "Breaking / behavior changes" section.
• Description and test numbers updated to current state (3719 examples, 0 failures; three acceptance runs listed).

Let me know if anything needs another look.