(PE-45697) Fall back to the ssl check when PuppetDB's cleartext status port is unavailable

[jonathannewman]

PE-45384 (puppet-enterprise-modules) and PE-44906 (puppetdb-private) disabled PuppetDB's cleartext HTTP status listener (port 8080) by default on fresh installs, as a security fix (SECVULN-1792) for an unauthenticated local endpoint that could accept replace_catalog commands. sleep_until_puppetdb_started's nonssl status check unconditionally assumed that port was reachable and checked it before the ssl status port, retrying 120 times before giving up — on an affected host it now exhausts every retry and fails outright, even though the method already has a second, independent ssl-based liveness check.

Confirmed live via a puppet-enterprise-modules frankenbuild (PR #2450, build enterprise_puppet-enterprise-modules_init-multijob_frankenbuilder-main-main #194, 2026-08-08): the monolithic-upgrade smoke job's "Upgrade Puppet Enterprise" step failed with curl: (7) Failed to connect to localhost:8080 after exhausting all 120 retries, tracing back to this method (beaker-pe's upgrade_pe → generic_install → sleep_until_puppetdb_started). This affects every consumer of beaker-pe/beaker-puppet performing a PE install or upgrade in acceptance testing, not just pe_acceptance_tests (tracked upstream as PE-45695, which fixed the two pe_acceptance_tests call sites with the same root cause).

Fix: check ssl liveness first — it's always enabled, and it's what the method's return value already reflects — then treat the nonssl check as a quick, best-effort secondary confirmation with a much smaller retry budget (120 → 5) rather than raising, since by the time it runs ssl has already proven the service is up. This collapses the worst-case delay from ~180s (120 nonssl retries plus ssl's own retry budget) down to roughly ssl's retry budget alone, and it applies to every pe_ver without needing a version gate against an unreleased PE line (2025.12.x, where the security fix first ships, hasn't shipped yet).

The rescue only swallows the exact "exhausted retries" RuntimeError retry_on raises for this specific command — compared by exact message match rather than a loose regex — so an unrelated RuntimeError, or a same-shaped error for a different command, still propagates instead of being misclassified. Logs via logger.warn (not logger.notify, which can be silently dropped below :notify verbosity) so the fallback is always visible to an operator.

Review history: addressed feedback from Copilot (loose regex match could swallow an unrelated templated error — narrowed to an exact match) and @joshcooper (nonssl-first ordering caused a ~2 minute delay on affected hosts — swapped to ssl-first).

Testing:

• Specs cover: default happy path, custom ports, ssl-then-nonssl ordering, the disabled-nonssl-port fallback, the ssl-check-fails-first case, an unrelated RuntimeError propagating, a differently-commanded templated RuntimeError propagating, and both pe_ver endpoint branches.
• Full suite: 356 examples, 0 failures. rubocop: no offenses.
• Not yet re-verified live against a real frankenbuild with this revised version — this PR isn't merged/released yet, and pe_acceptance_tests' beaker-puppet pin needs bumping once it is. Tracked as an open acceptance criterion on PE-45697.

PE-45697: https://perforce.atlassian.net/browse/PE-45697

Not merging this PR yet — opening for CI/review only per explicit instruction.

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

[Copilot]

Pull request overview

Adds SSL fallback when PuppetDB’s cleartext status endpoint is unavailable.

Changes:

• Handles cleartext status-check retry failures before SSL checking.
• Adds fallback and error-propagation tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
lib/beaker-puppet/helpers/puppet_helpers.rb	Adds fallback error handling and warning.
spec/beaker-puppet/helpers/puppet_helpers_spec.rb	Tests fallback and failure behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

spec/beaker-puppet/helpers/puppet_helpers_spec.rb:1066

• This fallback spec does not assert the new five-retry budget because _opts is ignored. A regression back to 120 retries—the delay this PR is intended to remove—would still pass. Match the options explicitly so the operational behavior is covered.

        expect(subject).to receive(:retry_on) do |_host, command, _opts|
          raise "Command `#{command}` failed."
        end.once.ordered

[jonathannewman]

Re: Copilot's suppressed comment on the fallback spec (line 1066) — good catch. Confirmed by temporarily reverting max_retries back to 120 and re-running: the spec still passed because the block ignored _opts, so a regression back to the original 120-retry budget (the delay this PR removes) wouldn't have been caught. Fixed by adding .with(anything, anything, { max_retries: 5 }) to the expectation alongside the block; re-verified it now fails against the reverted code and passes against the real fix. Pushed in 7c179bc.

[joshcooper]

seems fine, can't imagine anyone actually expects ONLY the cleartext listener to be enabled.