Skip to content

PMM-15129 Fix gosec Go linter issues (p1)#5641

Draft
maxkondr wants to merge 11 commits into
mainfrom
PMM-15129-fix-golint-gosec-p1
Draft

PMM-15129 Fix gosec Go linter issues (p1)#5641
maxkondr wants to merge 11 commits into
mainfrom
PMM-15129-fix-golint-gosec-p1

Conversation

@maxkondr

Copy link
Copy Markdown
Contributor

Ticket number: PMM-15129

This pull request introduces several improvements focused on resource management, context handling, and defensive programming in the MongoDB, MySQL, and PostgreSQL agents. The main themes are improved error handling for resource cleanup, enhanced support for context cancellation in long-running goroutines, and more robust handling of time and histogram data to prevent panics and ensure correctness.

Resource Management and Error Handling:

  • Replaced unchecked Close() and Stop() calls with error-checked versions in multiple agents (MongoDB, MySQL, PostgreSQL). Errors are now logged if resource cleanup fails, improving observability and reliability. (admin/commands/summary.go, agent/agents/mongodb/mongolog/mongodb.go, agent/agents/mongodb/profiler/mongodb.go, agent/agents/mongodb/profiler/internal/profiler.go, agent/agents/mysql/perfschema/perfschema.go, agent/agents/postgres/pgstatmonitor/pgstatmonitor.go, agent/agents/postgres/pgstatstatements/pgstatstatements.go) [1] [2] [3] [4] [5] [6] [7]

Context Handling and Graceful Shutdown:

  • Collector and parser goroutines now properly respond to context cancellation, ensuring that background operations terminate cleanly when requested. New tests verify that context cancellation reliably shuts down these components. (agent/agents/mongodb/profiler/internal/collector/collector.go, agent/agents/mongodb/profiler/internal/collector/collector_test.go, agent/agents/mongodb/profiler/internal/parser/parser.go, agent/agents/mongodb/profiler/internal/parser/parser_test.go) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

Defensive Programming for Time and Histogram Data:

  • Added defensive casts and comments to clarify and suppress linter warnings for Unix timestamps and durations stored as uint32, ensuring correctness up to the year 2106. (agent/agents/mongodb/shared/aggregator/aggregator.go, agent/agents/mysql/perfschema/perfschema.go, agent/agents/mysql/slowlog/slowlog.go, agent/agents/postgres/pgstatmonitor/pgstatmonitor.go)
  • In PostgreSQL pgStatStatements, periodStart is now clamped to the valid uint32 range to prevent overflows. (agent/agents/postgres/pgstatstatements/pgstatstatements.go) [1] [2] [3] [4] [5]

Histogram Parsing Robustness:

  • Refactored histogram parsing to use ParseUint instead of ParseInt, preventing negative values and ensuring only valid, non-negative frequencies are processed. Added logic to cap at zero if counters reset and to ignore excess buckets, avoiding panics. Comprehensive tests verify correct behavior for normal, overflow, counter reset, and invalid data cases. (agent/agents/postgres/pgstatmonitor/pgstatmonitor.go, agent/agents/postgres/pgstatmonitor/pgstatmonitor_test.go) [1] [2] [3]

Linting and Code Quality:

  • Updated .golangci.yml to disable the gosec linter for tests, acknowledging that security checks are less critical in test code and would otherwise require excessive suppression.

These changes together improve the reliability, maintainability, and correctness of the agents, especially in production environments where resource leaks or unhandled errors could cause subtle issues.

@maxkondr maxkondr self-assigned this Jul 14, 2026
@maxkondr maxkondr requested a review from Copilot July 14, 2026 11:37
@maxkondr maxkondr changed the title PMM-15129 fix golint gosec (p1) PMM-15129 Fix gosec Go linter issues (p1) Jul 14, 2026
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 30.93923% with 125 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.23%. Comparing base (31318c7) to head (eb38197).
⚠️ Report is 30 commits behind head on main.

Files with missing lines Patch % Lines
...ner/actions/postgresql_show_create_table_action.go 33.68% 39 Missing and 24 partials ⚠️
agent/runner/jobs/pbm_helpers.go 0.00% 16 Missing and 1 partial ⚠️
...ents/postgres/pgstatstatements/pgstatstatements.go 22.22% 5 Missing and 2 partials ⚠️
admin/commands/summary.go 0.00% 6 Missing ⚠️
...ent/agents/postgres/pgstatmonitor/pgstatmonitor.go 60.00% 5 Missing and 1 partial ⚠️
agent/runner/jobs/mongodb_backup_job.go 0.00% 4 Missing ⚠️
agent/runner/jobs/mongodb_restore_job.go 0.00% 4 Missing ⚠️
agent/agents/mysql/perfschema/perfschema.go 25.00% 3 Missing ⚠️
agent/runner/jobs/mysql_restore_job.go 0.00% 3 Missing ⚠️
agent/agents/mongodb/mongolog/mongodb.go 33.33% 1 Missing and 1 partial ⚠️
... and 6 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5641      +/-   ##
==========================================
+ Coverage   43.59%   44.23%   +0.64%     
==========================================
  Files         415      416       +1     
  Lines       43134    43296     +162     
==========================================
+ Hits        18804    19153     +349     
+ Misses      22454    22230     -224     
- Partials     1876     1913      +37     
Flag Coverage Δ
admin 34.75% <0.00%> (-0.03%) ⬇️
agent 50.38% <32.00%> (+1.34%) ⬆️
managed 43.41% <ø> (+0.43%) ⬆️
vmproxy 72.22% <ø> (ø)

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR focuses on hardening PMM agent and admin code to satisfy golint/gosec requirements while improving runtime reliability—primarily by making cleanup/error paths observable, adding context-driven shutdown to long-running goroutines, and making timestamp/histogram handling more defensive.

Changes:

  • Added/expanded error handling for resource cleanup (DB closes, temp files, zip readers, profiler/mongolog shutdown) and improved logging.
  • Improved graceful shutdown behavior in MongoDB profiler collector/parser by honoring context cancellation, with new tests.
  • Hardened Postgres pg_stat_monitor histogram parsing and added additional tests; added defensive handling for Unix timestamp casts.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
agent/versioner/versioner.go Adds gosec suppression/documentation for exec wrapper used by version detection.
agent/runner/jobs/pbm_helpers.go Adds gosec suppression for PBM command execution; improves temp file close error handling.
agent/runner/jobs/mysql_restore_job.go Adds gosec suppression/comments for validated systemctl service name usage.
agent/runner/actions/pt_mysql_summary_action.go Checks temp file Close() errors before running external command.
agent/runner/actions/postgresql_show_create_table_action.go Adds structured logger to action; checks Close()/Write()/Fprintf() errors and logs them.
agent/cmd/pmm-agent-entrypoint/main.go Adds gosec suppression/comment for launching pmm-agent from entrypoint.
agent/agents/postgres/pgstatstatements/pgstatstatements.go Checks db close error; clamps period start Unix seconds to uint32 range.
agent/agents/postgres/pgstatmonitor/pgstatmonitor.go Adds close error logging; adds safer histogram parsing (ParseUint, bounds, counter-reset handling).
agent/agents/postgres/pgstatmonitor/pgstatmonitor_test.go Adds tests for histogram parsing and histogram range generation.
agent/agents/mysql/slowlog/slowlog.go Adds gosec suppression/comment for uint32 Unix timestamp cast.
agent/agents/mysql/perfschema/perfschema.go Checks db close error; adds gosec suppression/comment for uint32 Unix timestamp cast.
agent/agents/mongodb/shared/aggregator/aggregator.go Adds gosec suppression/comments for uint32 timestamp/duration casts.
agent/agents/mongodb/profiler/mongodb.go Checks profiler Stop() error and logs it.
agent/agents/mongodb/profiler/internal/profiler.go Checks MongoDB client disconnect error and logs it.
agent/agents/mongodb/profiler/internal/parser/parser.go Propagates Start(ctx) context to goroutine and exits on ctx.Done().
agent/agents/mongodb/profiler/internal/parser/parser_test.go Adds test validating parser exits on context cancellation.
agent/agents/mongodb/profiler/internal/collector/collector.go Propagates Start(ctx) context into collector loop; uses ctx for cursor operations and timeouts.
agent/agents/mongodb/profiler/internal/collector/collector_test.go Adds test validating collector exits on context cancellation and closes output channel.
agent/agents/mongodb/mongolog/mongodb.go Checks mongolog Stop() error and logs it.
admin/commands/summary.go Checks and logs zip reader Close() errors instead of ignoring them.
.golangci.yml Disables gosec for tests in golangci-lint configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread agent/agents/postgres/pgstatstatements/pgstatstatements.go Outdated
Comment thread agent/agents/mysql/perfschema/perfschema.go Outdated
Comment thread agent/agents/postgres/pgstatmonitor/pgstatmonitor_test.go Outdated
Comment thread agent/cmd/pmm-agent-entrypoint/main.go Outdated
Comment thread agent/runner/jobs/pbm_helpers.go
Comment thread agent/runner/jobs/pbm_helpers.go
Comment thread agent/agents/mongodb/profiler/internal/profiler.go Outdated
maxkondr and others added 6 commits July 14, 2026 14:50
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Comment thread agent/runner/jobs/mongodb_backup_job.go Outdated
Comment thread agent/runner/jobs/mongodb_restore_job.go Outdated
Comment thread agent/runner/jobs/pbm_helpers.go
Comment thread agent/agents/mongodb/profiler/internal/profiler.go
Comment thread agent/agents/postgres/pgstatmonitor/pgstatmonitor_test.go
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