Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions admin/commands/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ func (cmd *ConfigCommand) args(globals *flags.GlobalFlags) ([]string, bool) {
res = append(res, "--server-insecure-tls")
}

if cmd.LogLevelFatalFlags.LogLevel != "" {
res = append(res, fmt.Sprintf("--log-level=%s", cmd.LogLevelFatalFlags.LogLevel))
if cmd.LogLevel != "" {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Linter issue from original PR, there and in admin commands.

res = append(res, fmt.Sprintf("--log-level=%s", cmd.LogLevel))
}
if globals.EnableDebug {
res = append(res, "--debug")
Expand Down
28 changes: 28 additions & 0 deletions admin/commands/helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (C) 2023 Percona LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package commands

import (
"time"
)

// DurationString returns the string representation of a duration flag.
func DurationString(value *time.Duration) string {
if value == nil {
return ""
}

return value.String()
}
Comment on lines +23 to +29
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fix for #5134 (comment)

6 changes: 5 additions & 1 deletion admin/commands/inventory/add_agent_mongodb_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package inventory

import (
"time"

"github.com/AlekSi/pointer"

"github.com/percona/pmm/admin/commands"
Expand Down Expand Up @@ -67,6 +69,7 @@ type AddAgentMongodbExporterCommand struct {
DisableCollectors []string `help:"Comma-separated list of collector names to exclude from exporter"`
StatsCollections []string `help:"Collections for collstats & indexstats"`
CollectionsLimit int32 `name:"max-collections-limit" placeholder:"number" help:"Disable collstats & indexstats if there are more than <n> collections"` //nolint:lll
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

flags.LogLevelFatalFlags
}
Expand Down Expand Up @@ -104,7 +107,8 @@ func (cmd *AddAgentMongodbExporterCommand) RunCmd() (commands.Result, error) {
DisableCollectors: commands.ParseDisableCollectors(cmd.DisableCollectors),
StatsCollections: commands.ParseDisableCollectors(cmd.StatsCollections),
CollectionsLimit: cmd.CollectionsLimit,
LogLevel: cmd.LogLevelFatalFlags.LogLevel.EnumValue(),
LogLevel: cmd.LogLevel.EnumValue(),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
},
},
Context: commands.Ctx,
Expand Down
5 changes: 4 additions & 1 deletion admin/commands/inventory/add_agent_mysqld_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package inventory
import (
"fmt"
"strconv"
"time"

"github.com/AlekSi/pointer"

Expand Down Expand Up @@ -103,6 +104,7 @@ type AddAgentMysqldExporterCommand struct {
PushMetrics bool `help:"Enables push metrics model flow, it will be sent to the server by an agent"`
ExposeExporter bool `help:"Expose the address of the exporter publicly on 0.0.0.0"`
DisableCollectors []string `help:"Comma-separated list of collector names to exclude from exporter"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

flags.LogLevelNoFatalFlags
}
Expand Down Expand Up @@ -152,7 +154,8 @@ func (cmd *AddAgentMysqldExporterCommand) RunCmd() (commands.Result, error) {
PushMetrics: cmd.PushMetrics,
ExposeExporter: cmd.ExposeExporter,
DisableCollectors: commands.ParseDisableCollectors(cmd.DisableCollectors),
LogLevel: cmd.LogLevelNoFatalFlags.LogLevel.EnumValue(),
LogLevel: cmd.LogLevel.EnumValue(),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
},
},
Context: commands.Ctx,
Expand Down
16 changes: 10 additions & 6 deletions admin/commands/inventory/add_agent_postgres_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package inventory

import (
"time"

"github.com/percona/pmm/admin/commands"
"github.com/percona/pmm/admin/pkg/flags"
"github.com/percona/pmm/api/inventory/v1/json/client"
Expand Down Expand Up @@ -67,6 +69,7 @@ type AddAgentPostgresExporterCommand struct {
TLSKeyFile string `help:"TLS certificate key file"`
AutoDiscoveryLimit int32 `default:"0" placeholder:"NUMBER" help:"Auto-discovery will be disabled if there are more than that number of databases (default: server-defined, -1: always disabled)"`
MaxExporterConnections int32 `default:"0" placeholder:"NUMBER" help:"Maximum number of connections that exporter can make to PostgreSQL instance (default: server-defined)"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

flags.LogLevelNoFatalFlags
}
Expand Down Expand Up @@ -112,12 +115,13 @@ func (cmd *AddAgentPostgresExporterCommand) RunCmd() (commands.Result, error) {
AutoDiscoveryLimit: cmd.AutoDiscoveryLimit,
MaxExporterConnections: cmd.MaxExporterConnections,

TLS: cmd.TLS,
TLSSkipVerify: cmd.TLSSkipVerify,
TLSCa: tlsCa,
TLSCert: tlsCert,
TLSKey: tlsKey,
LogLevel: cmd.LogLevelNoFatalFlags.LogLevel.EnumValue(),
TLS: cmd.TLS,
TLSSkipVerify: cmd.TLSSkipVerify,
TLSCa: tlsCa,
TLSCert: tlsCert,
TLSKey: tlsKey,
LogLevel: cmd.LogLevel.EnumValue(),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
},
},
Context: commands.Ctx,
Expand Down
6 changes: 5 additions & 1 deletion admin/commands/inventory/add_agent_proxysql_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package inventory

import (
"time"

"github.com/percona/pmm/admin/commands"
"github.com/percona/pmm/admin/pkg/flags"
"github.com/percona/pmm/api/inventory/v1/json/client"
Expand Down Expand Up @@ -60,6 +62,7 @@ type AddAgentProxysqlExporterCommand struct {
PushMetrics bool `help:"Enables push metrics model flow, it will be sent to the server by an agent"`
ExposeExporter bool `help:"Expose the address of the exporter publicly on 0.0.0.0"`
DisableCollectors []string `help:"Comma-separated list of collector names to exclude from exporter"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

flags.LogLevelFatalFlags
}
Expand All @@ -82,7 +85,8 @@ func (cmd *AddAgentProxysqlExporterCommand) RunCmd() (commands.Result, error) {
PushMetrics: cmd.PushMetrics,
ExposeExporter: cmd.ExposeExporter,
DisableCollectors: commands.ParseDisableCollectors(cmd.DisableCollectors),
LogLevel: cmd.LogLevelFatalFlags.LogLevel.EnumValue(),
LogLevel: cmd.LogLevel.EnumValue(),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
},
},
Context: commands.Ctx,
Expand Down
4 changes: 4 additions & 0 deletions admin/commands/inventory/add_agent_valkey_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package inventory

import (
"time"

"github.com/AlekSi/pointer"

"github.com/percona/pmm/admin/commands"
Expand Down Expand Up @@ -68,6 +70,7 @@ type AddAgentValkeyExporterCommand struct {
PushMetrics bool `help:"Enables push metrics model flow, it will be sent to the server by an agent"`
ExposeExporter bool `help:"Expose the address of the exporter publicly on 0.0.0.0"`
DisableCollectors []string `help:"Comma-separated list of collector names to exclude from exporter"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

flags.LogLevelNoFatalFlags
}
Expand Down Expand Up @@ -116,6 +119,7 @@ func (cmd *AddAgentValkeyExporterCommand) RunCmd() (commands.Result, error) {
ExposeExporter: cmd.ExposeExporter,
DisableCollectors: commands.ParseDisableCollectors(cmd.DisableCollectors),
LogLevel: convertLogLevelPtr(&cmd.LogLevel),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
},
},
Context: commands.Ctx,
Expand Down
9 changes: 6 additions & 3 deletions admin/commands/inventory/change_agent_mongodb_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package inventory
import (
"fmt"
"strings"
"time"

"github.com/percona/pmm/admin/commands"
"github.com/percona/pmm/admin/pkg/flags"
Expand Down Expand Up @@ -89,9 +90,10 @@ type ChangeAgentMongodbExporterCommand struct {
CollectionsLimit *int32 `help:"Collections limit"`

// Exporter options
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

// Custom labels
CustomLabels *map[string]string `mapsep:"," help:"Custom user-assigned labels"`
Expand Down Expand Up @@ -152,6 +154,7 @@ func (cmd *ChangeAgentMongodbExporterCommand) RunCmd() (commands.Result, error)
ExposeExporter: cmd.ExposeExporter,
EnablePushMetrics: cmd.PushMetrics,
LogLevel: convertLogLevelPtr(cmd.LogLevel),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Change commands may unintentionally clear connection timeout

Medium Severity

DurationString returns a plain string ("" when nil), but change-agent commands need pointer semantics to distinguish "not specified" from "clear the value." Other optional fields in these same change commands (e.g., LogLevel via convertLogLevelPtr) correctly return *string so that nil means "don't modify." Because DurationString always produces a non-nil value, every change operation unconditionally sets ConnectionTimeout on the request body — risking clearing a previously configured timeout when the user only intended to update an unrelated property.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ca21749. Configure here.

}

if customLabels != nil {
Expand Down
11 changes: 7 additions & 4 deletions admin/commands/inventory/change_agent_mysqld_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package inventory

import (
"fmt"
"time"

"github.com/percona/pmm/admin/commands"
"github.com/percona/pmm/admin/pkg/flags"
Expand Down Expand Up @@ -82,10 +83,11 @@ type ChangeAgentMysqldExporterCommand struct {
TLSKeyFile *string `help:"TLS certificate key file"`

// Exporter options
TablestatsGroupTableLimit *int32 `help:"Tablestats group collectors by table limit"`
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
TablestatsGroupTableLimit *int32 `help:"Tablestats group collectors by table limit"`
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

// Custom labels
CustomLabels *map[string]string `mapsep:"," help:"Custom user-assigned labels"`
Expand Down Expand Up @@ -143,6 +145,7 @@ func (cmd *ChangeAgentMysqldExporterCommand) RunCmd() (commands.Result, error) {
ExposeExporter: cmd.ExposeExporter,
EnablePushMetrics: cmd.PushMetrics,
LogLevel: convertLogLevelPtr(cmd.LogLevel),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
}

if customLabels != nil {
Expand Down
13 changes: 8 additions & 5 deletions admin/commands/inventory/change_agent_postgres_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package inventory

import (
"fmt"
"time"

"github.com/percona/pmm/admin/commands"
"github.com/percona/pmm/admin/pkg/flags"
Expand Down Expand Up @@ -82,11 +83,12 @@ type ChangeAgentPostgresExporterCommand struct {
TLSKeyFile *string `help:"TLS certificate key file"`

// Exporter options
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
AutoDiscoveryLimit *int32 `help:"Auto-discovery limit"`
MaxExporterConnections *int32 `help:"Maximum number of connections that exporter can make to PostgreSQL instance"`
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
AutoDiscoveryLimit *int32 `help:"Auto-discovery limit"`
MaxExporterConnections *int32 `help:"Maximum number of connections that exporter can make to PostgreSQL instance"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

// Custom labels
CustomLabels *map[string]string `mapsep:"," help:"Custom user-assigned labels"`
Expand Down Expand Up @@ -145,6 +147,7 @@ func (cmd *ChangeAgentPostgresExporterCommand) RunCmd() (commands.Result, error)
AutoDiscoveryLimit: cmd.AutoDiscoveryLimit,
MaxExporterConnections: cmd.MaxExporterConnections,
LogLevel: convertLogLevelPtr(cmd.LogLevel),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
}

if customLabels != nil {
Expand Down
9 changes: 6 additions & 3 deletions admin/commands/inventory/change_agent_proxysql_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package inventory

import (
"fmt"
"time"

"github.com/percona/pmm/admin/commands"
"github.com/percona/pmm/admin/pkg/flags"
Expand Down Expand Up @@ -79,9 +80,10 @@ type ChangeAgentProxysqlExporterCommand struct {
TLSSkipVerify *bool `help:"Skip TLS certificate and hostname validation"`

// Exporter options
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

// Custom labels
CustomLabels *map[string]string `mapsep:"," help:"Custom user-assigned labels"`
Expand All @@ -105,6 +107,7 @@ func (cmd *ChangeAgentProxysqlExporterCommand) RunCmd() (commands.Result, error)
ExposeExporter: cmd.ExposeExporter,
EnablePushMetrics: cmd.PushMetrics,
LogLevel: convertLogLevelPtr(cmd.LogLevel),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
}

if customLabels != nil {
Expand Down
9 changes: 6 additions & 3 deletions admin/commands/inventory/change_agent_valkey_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package inventory

import (
"fmt"
"time"

"github.com/percona/pmm/admin/commands"
"github.com/percona/pmm/admin/pkg/flags"
Expand Down Expand Up @@ -81,9 +82,10 @@ type ChangeAgentValkeyExporterCommand struct {
TLSKeyFile *string `help:"TLS certificate key file"`

// Exporter options
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
DisableCollectors []string `help:"List of collector names to disable"`
ExposeExporter *bool `help:"Expose the exporter process on all public interfaces"`
PushMetrics *bool `help:"Enable push metrics with vmagent"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

// Custom labels
CustomLabels *map[string]string `mapsep:"," help:"Custom user-assigned labels"`
Expand Down Expand Up @@ -137,6 +139,7 @@ func (cmd *ChangeAgentValkeyExporterCommand) RunCmd() (commands.Result, error) {
ExposeExporter: cmd.ExposeExporter,
EnablePushMetrics: cmd.PushMetrics,
LogLevel: convertLogLevelPtr(cmd.LogLevel),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
}

if customLabels != nil {
Expand Down
5 changes: 4 additions & 1 deletion admin/commands/management/add_mongodb.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package management

import (
"fmt"
"time"

"github.com/percona/pmm/admin/agentlocal"
"github.com/percona/pmm/admin/commands"
Expand Down Expand Up @@ -83,6 +84,7 @@ type AddMongoDBCommand struct {
CollectionsLimit int32 `name:"max-collections-limit" default:"-1" help:"Disable collstats, dbstats, topmetrics and indexstats if there are more than <n> collections. 0: No limit. Default is -1, which let PMM automatically set this value"`
ExposeExporter bool `name:"expose-exporter" help:"Optionally expose the address of the exporter publicly on 0.0.0.0"`
AgentEnvVars []string `name:"agent-env-vars" help:"Comma-separated list of environment variable names to pass to the exporter (values are read from the current environment), e.g. 'VAR1,VAR2'"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

AddCommonFlags
flags.MetricsModeFlags
Expand Down Expand Up @@ -202,7 +204,8 @@ func (cmd *AddMongoDBCommand) RunCmd() (commands.Result, error) {
DisableCollectors: commands.ParseDisableCollectors(cmd.DisableCollectors),
StatsCollections: commands.ParseDisableCollectors(cmd.StatsCollections),
CollectionsLimit: cmd.CollectionsLimit,
LogLevel: cmd.LogLevelFatalFlags.LogLevel.EnumValue(),
LogLevel: cmd.LogLevel.EnumValue(),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
},
},
Context: commands.Ctx,
Expand Down
5 changes: 4 additions & 1 deletion admin/commands/management/add_mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package management
import (
"fmt"
"strconv"
"time"

"github.com/AlekSi/pointer"
"github.com/alecthomas/units"
Expand Down Expand Up @@ -120,6 +121,7 @@ type AddMySQLCommand struct {
CreateUser bool `hidden:"" help:"Create pmm user"`
DisableCollectors []string `help:"Comma-separated list of collector names to exclude from exporter"`
ExposeExporter bool `name:"expose-exporter" help:"Optionally expose the address of the exporter publicly on 0.0.0.0"`
ConnectionTimeout *time.Duration `placeholder:"DURATION" help:"Connection timeout to use for exporter (e.g. 1s, 1.5s)"`

AddCommonFlags
flags.MetricsModeFlags
Expand Down Expand Up @@ -241,7 +243,8 @@ func (cmd *AddMySQLCommand) RunCmd() (commands.Result, error) {
TablestatsGroupTableLimit: tablestatsGroupTableLimit,
MetricsMode: cmd.MetricsModeFlags.MetricsMode.EnumValue(),
DisableCollectors: commands.ParseDisableCollectors(cmd.DisableCollectors),
LogLevel: cmd.LogLevelNoFatalFlags.LogLevel.EnumValue(),
LogLevel: cmd.LogLevel.EnumValue(),
ConnectionTimeout: commands.DurationString(cmd.ConnectionTimeout),
},
},
Context: commands.Ctx,
Expand Down
Loading
Loading