PMM-1519 Change QAN-PGSM fix for internal error.

[JiriCtvrtka]

PMM-15191

What is done

Reworks the guard that protects the QAN agent of PMM's internal PostgreSQL server when its
enabled state is pinned by the PMM_ENABLE_INTERNAL_PG_QAN environment variable.
The previous implementation lived at the end of ChangeQANPostgreSQLPgStatementsAgent, after
executeAgentChange had already committed the update. It had several problems:

• The rejected change was applied anyway. The error was returned only after the transaction
  had been committed, so the caller saw a failure while the agent had already been modified.
• It was bypassable. The inventory API picks the Change*Agent method from the request
  payload, not from the type of the agent being changed, so pointing any other Change*Agent
  method at the internal QAN agent skipped the check entirely.
• It over-rejected. The check only looked at pmm_agent_id, so any change to any agent
  running under PMM Server's pmm-agent (postgres_exporter, node_exporter, vmagent, ...) was
  refused with a message about QAN. Changes unrelated to the enabled state (e.g. custom labels,
  max query length) were refused as well.
• PMM_ENABLE_INTERNAL_PG_QAN=false behaved like a pin too, because the value was only
  checked for being non-empty, and the raw string was interpolated into the error message.

New behaviour

checkInternalPgQANEnvOverride now runs inside executeAgentChange, on the updated agent row and
before the transaction is committed, so a rejected request rolls back and leaves the agent
untouched. It rejects a request only when all of the following hold:

• the request changes the enabled state (enable is set),
• the target agent is a qan_postgresql_pgstatements_agent belonging to PMM Server's pmm-agent,
• PMM_ENABLE_INTERNAL_PG_QAN parses as a boolean and the requested state differs from it.
  Everything else stays changeable, and a non-boolean value in the variable pins nothing (invalid
  values are already reported by the env parser during startup).
  Also adds env.LookupBool, which returns *bool so callers can tell "not configured" apart from
  "configured as false"; env.GetBool is now a thin wrapper over it.

Tests

managed/services/inventory/agents_test.go covers the guard: rejection leaves the agent intact,
rejection also happens when the change is requested through the params of another agent type, and
requests are accepted for parameters unrelated to the enabled state, when the requested state
matches the variable, for QAN agents outside of PMM Server, for other agent types of PMM Server,
and when the variable holds a non-boolean value.

Summary by CodeRabbit

• Bug Fixes
  • Prevented changes to the enabled state of PMM’s internal PostgreSQL query analytics agent when controlled by an environment setting.
  • Ensured rejected changes are rolled back while allowing unrelated agent updates.
  • Improved handling of unset and invalid boolean environment settings.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.19%. Comparing base (31318c7) to head (25d5d76).
⚠️ Report is 87 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5703      +/-   ##
==========================================
+ Coverage   43.59%   45.19%   +1.59%     
==========================================
  Files         415      218     -197     
  Lines       43134    27858   -15276     
==========================================
- Hits        18804    12590    -6214     
+ Misses      22454    13919    -8535     
+ Partials     1876     1349     -527     

Flag	Coverage Δ
admin	?
agent	?
managed	45.19% <100.00%> (+2.21%)	⬆️
vmproxy	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JiriCtvrtka]

@copilot review

[JiriCtvrtka]

@copilot review

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b27ffaf1-14ff-4590-8b60-2a9aa4b331c7

📥 Commits

Reviewing files that changed from the base of the PR and between a50c452 and db66c3f.

📒 Files selected for processing (3)

• managed/services/inventory/agents.go
• managed/services/inventory/agents_test.go
• managed/utils/env/env.go

---

Walkthrough

The change adds tri-state boolean environment lookup and moves internal PostgreSQL QAN environment enforcement into the transactional agent-change flow. Rejected enablement changes roll back. Tests cover routing, rollback, allowed updates, and invalid values.

Changes

Internal PostgreSQL QAN guard

Layer / File(s)	Summary
Boolean environment lookup managed/utils/env/env.go	Adds LookupBool for parsed, unset, and invalid environment values. GetBool delegates to it and keeps its existing behavior.
Transactional agent-change enforcement managed/services/inventory/agents.go	Adds checkInternalPgQANEnvOverride and runs it after model persistence and before connection validation. Rejected enablement changes return a failed-precondition error and roll back.
Agent guard validation managed/services/inventory/agents_test.go	Tests rejection rollback, request routing, permitted updates, matching states, unaffected agents, invalid values, and shared agent ID usage.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ChangeQANPostgreSQLPgStatementsAgent
  participant executeAgentChange
  participant checkInternalPgQANEnvOverride
  participant env.LookupBool
  ChangeQANPostgreSQLPgStatementsAgent->>executeAgentChange: request agent change
  executeAgentChange->>checkInternalPgQANEnvOverride: validate internal QAN enablement
  checkInternalPgQANEnvOverride->>env.LookupBool: read PMM_ENABLE_INTERNAL_PG_QAN
  env.LookupBool-->>checkInternalPgQANEnvOverride: parsed state or nil
  checkInternalPgQANEnvOverride-->>executeAgentChange: allow or failed-precondition error
  executeAgentChange-->>ChangeQANPostgreSQLPgStatementsAgent: commit result or rollback error

Loading

Suggested reviewers: 4nte, ademidoff, maxkondr

Poem

A rabbit checks the QAN gate,
Reads the env before it’s late.
Matching states may safely stay,
Mismatched changes roll away.
Tests hop through each guarded case.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'PMM-1519 Change QAN-PGSM fix for internal error' references the ticket and describes a fix, but is vague about the specific change made.	Use a more specific title that describes what was changed, such as 'Move QAN-PGSM environment variable guard to transaction pre-commit point' or similar.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description includes ticket number, detailed technical explanation of problems and solutions, test coverage, and rationale for the changes, though it lacks the optional API documentation checkbox state.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch PMM-15191-admin-change-error

---

_{Comment @coderabbitai help to get the list of available commands.}