PMM-5086-12634 Fix Postgres nil handling.

percona · Dec 3, 2024 · b5933ae · b5933ae
1 parent e57bbc9
commit b5933ae
Show file tree

Hide file tree

Showing 8 changed files with 24 additions and 38 deletions.
diff --git a/managed/models/agent_helpers.go b/managed/models/agent_helpers.go
@@ -64,7 +64,7 @@ type PostgreSQLOptionsParams interface {
 
 // PostgreSQLExtendedOptionsParams contains extended parameters for PostgreSQL exporter.
 type PostgreSQLExtendedOptionsParams interface {
-	GetAutoDiscoveryLimit() int32
+	GetAutoDiscoveryLimit() *int32
 	GetMaxExporterConnections() int32
 }
 

diff --git a/managed/models/agent_model.go b/managed/models/agent_model.go
@@ -182,7 +182,7 @@ type PostgreSQLOptions struct {
 	SSLCa                  string  `json:"ssl_ca"`
 	SSLCert                string  `json:"ssl_cert"`
 	SSLKey                 string  `json:"ssl_key"`
-	AutoDiscoveryLimit     int32   `json:"auto_discovery_limit"`
+	AutoDiscoveryLimit     *int32  `json:"auto_discovery_limit"`
 	DatabaseCount          int32   `json:"database_count"`
 	PGSMVersion            *string `json:"pgsm_version"`
 	MaxExporterConnections int32   `json:"max_exporter_connections"`

diff --git a/managed/models/agent_model_test.go b/managed/models/agent_model_test.go
@@ -260,7 +260,7 @@ func TestPostgresAgentTLS(t *testing.T) {
 		t.Run(fmt.Sprintf("AutodiscoveryLimit set TLS:%v/TLSSkipVerify:%v", testCase.tls, testCase.tlsSkipVerify), func(t *testing.T) {
 			agent.TLS = testCase.tls
 			agent.TLSSkipVerify = testCase.tlsSkipVerify
-			agent.PostgreSQLOptions = &models.PostgreSQLOptions{AutoDiscoveryLimit: 10}
+			agent.PostgreSQLOptions = &models.PostgreSQLOptions{AutoDiscoveryLimit: pointer.ToInt32(10)}
 			assert.Equal(t, testCase.expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
 		})
 	}

diff --git a/managed/services/agents/postgresql.go b/managed/services/agents/postgresql.go
@@ -22,6 +22,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/AlekSi/pointer"
 	agentv1 "github.com/percona/pmm/api/agent/v1"
 	inventoryv1 "github.com/percona/pmm/api/inventory/v1"
 	"github.com/percona/pmm/managed/models"
@@ -84,20 +85,16 @@ func postgresExporterConfig(node *models.Node, service *models.Service, exporter
 		"--web.listen-address=" + listenAddress + ":" + tdp.Left + " .listen_port " + tdp.Right,
 	}
 
-	if exporter.PostgreSQLOptions == nil {
-		exporter.PostgreSQLOptions = &models.PostgreSQLOptions{}
-	}
-
 	autoDiscovery := false
 	if !pmmAgentVersion.Less(postgresExporterAutodiscoveryVersion) {
 		switch {
-		case exporter.PostgreSQLOptions == nil:
+		case exporter.PostgreSQLOptions.AutoDiscoveryLimit == nil:
 			autoDiscovery = true
-		case exporter.PostgreSQLOptions.AutoDiscoveryLimit == 0: // server defined
+		case pointer.GetInt32(exporter.PostgreSQLOptions.AutoDiscoveryLimit) == 0: // server defined
 			autoDiscovery = exporter.PostgreSQLOptions.DatabaseCount <= defaultAutoDiscoveryDatabaseLimit
-		case exporter.PostgreSQLOptions.AutoDiscoveryLimit < 0: // always disabled
+		case pointer.GetInt32(exporter.PostgreSQLOptions.AutoDiscoveryLimit) < 0: // always disabled
 		default:
-			autoDiscovery = exporter.PostgreSQLOptions.DatabaseCount <= exporter.PostgreSQLOptions.AutoDiscoveryLimit
+			autoDiscovery = exporter.PostgreSQLOptions.DatabaseCount <= pointer.GetInt32(exporter.PostgreSQLOptions.AutoDiscoveryLimit)
 		}
 	}
 	if autoDiscovery {
@@ -107,15 +104,10 @@ func postgresExporterConfig(node *models.Node, service *models.Service, exporter
 	}
 
 	if !pmmAgentVersion.Less(postgresMaxExporterConnsVersion) &&
-		exporter.PostgreSQLOptions != nil &&
 		exporter.PostgreSQLOptions.MaxExporterConnections != 0 {
 		args = append(args, "--max-connections="+strconv.Itoa(int(exporter.PostgreSQLOptions.MaxExporterConnections)))
 	}
 
-	if exporter.ExporterOptions == nil {
-		exporter.ExporterOptions = &models.ExporterOptions{}
-	}
-
 	if exporter.ExporterOptions.MetricsPath != nil {
 		args = append(args, "--web.telemetry-path="+*exporter.ExporterOptions.MetricsPath)
 	}
@@ -137,10 +129,7 @@ func postgresExporterConfig(node *models.Node, service *models.Service, exporter
 		PostgreSQLSupportsSSLSNI: !pmmAgentVersion.Less(postgresSSLSniVersion),
 	}
 
-	if exporter.AzureOptions == nil {
-		exporter.AzureOptions = &models.AzureOptions{}
-	}
-	if exporter.AzureOptions != nil {
+	if exporter.AzureOptions.ClientID != "" {
 		dnsParams.DialTimeout = 5 * time.Second
 	}
 
@@ -174,9 +163,6 @@ func qanPostgreSQLPgStatementsAgentConfig(service *models.Service, agent *models
 		Database:                 service.DatabaseName,
 		PostgreSQLSupportsSSLSNI: !pmmAgentVersion.Less(postgresSSLSniVersion),
 	}
-	if agent.QANOptions == nil {
-		agent.QANOptions = &models.QANOptions{}
-	}
 
 	return &agentv1.SetStateRequest_BuiltinAgent{
 		Type:                   inventoryv1.AgentType_AGENT_TYPE_QAN_POSTGRESQL_PGSTATEMENTS_AGENT,
@@ -199,9 +185,6 @@ func qanPostgreSQLPgStatMonitorAgentConfig(service *models.Service, agent *model
 		Database:                 service.DatabaseName,
 		PostgreSQLSupportsSSLSNI: !pmmAgentVersion.Less(postgresSSLSniVersion),
 	}
-	if agent.QANOptions == nil {
-		agent.QANOptions = &models.QANOptions{}
-	}
 
 	return &agentv1.SetStateRequest_BuiltinAgent{
 		Type:                   inventoryv1.AgentType_AGENT_TYPE_QAN_POSTGRESQL_PGSTATMONITOR_AGENT,

diff --git a/managed/services/agents/postgresql_test.go b/managed/services/agents/postgresql_test.go
@@ -196,10 +196,13 @@ func TestAutoDiscovery(t *testing.T) {
 		DatabaseName: "postgres",
 	}
 	exporter := &models.Agent{
-		AgentID:   "agent-id",
-		AgentType: models.PostgresExporterType,
-		Username:  pointer.ToString("username"),
-		Password:  pointer.ToString("s3cur3 p@$$w0r4."),
+		AgentID:           "agent-id",
+		AgentType:         models.PostgresExporterType,
+		Username:          pointer.ToString("username"),
+		Password:          pointer.ToString("s3cur3 p@$$w0r4."),
+		ExporterOptions:   &models.ExporterOptions{},
+		AzureOptions:      &models.AzureOptions{},
+		PostgreSQLOptions: &models.PostgreSQLOptions{},
 	}
 
 	expected := &agentv1.SetStateRequest_AgentProcess{
@@ -240,7 +243,7 @@ func TestAutoDiscovery(t *testing.T) {
 
 	t.Run("Database count more than limit - disabled", func(t *testing.T) {
 		exporter.PostgreSQLOptions = &models.PostgreSQLOptions{
-			AutoDiscoveryLimit: 5,
+			AutoDiscoveryLimit: pointer.ToInt32(5),
 			DatabaseCount:      10,
 		}
 		res, err := postgresExporterConfig(node, postgresql, exporter, redactSecrets, pmmAgentVersion)
@@ -251,7 +254,7 @@ func TestAutoDiscovery(t *testing.T) {
 
 	t.Run("Database count equal to limit - enabled", func(t *testing.T) {
 		exporter.PostgreSQLOptions = &models.PostgreSQLOptions{
-			AutoDiscoveryLimit: 5,
+			AutoDiscoveryLimit: pointer.ToInt32(5),
 			DatabaseCount:      5,
 		}
 		res, err := postgresExporterConfig(node, postgresql, exporter, redactSecrets, pmmAgentVersion)
@@ -262,7 +265,7 @@ func TestAutoDiscovery(t *testing.T) {
 
 	t.Run("Database count less than limit - enabled", func(t *testing.T) {
 		exporter.PostgreSQLOptions = &models.PostgreSQLOptions{
-			AutoDiscoveryLimit: 5,
+			AutoDiscoveryLimit: pointer.ToInt32(5),
 			DatabaseCount:      3,
 		}
 		res, err := postgresExporterConfig(node, postgresql, exporter, redactSecrets, pmmAgentVersion)
@@ -273,7 +276,7 @@ func TestAutoDiscovery(t *testing.T) {
 
 	t.Run("Negative limit - disabled", func(t *testing.T) {
 		exporter.PostgreSQLOptions = &models.PostgreSQLOptions{
-			AutoDiscoveryLimit: -1,
+			AutoDiscoveryLimit: pointer.ToInt32(-1),
 			DatabaseCount:      3,
 		}
 		res, err := postgresExporterConfig(node, postgresql, exporter, redactSecrets, pmmAgentVersion)
@@ -284,7 +287,7 @@ func TestAutoDiscovery(t *testing.T) {
 
 	t.Run("Default - enabled", func(t *testing.T) {
 		exporter.PostgreSQLOptions = &models.PostgreSQLOptions{
-			AutoDiscoveryLimit: 0,
+			AutoDiscoveryLimit: pointer.ToInt32(0),
 			DatabaseCount:      3,
 		}
 		res, err := postgresExporterConfig(node, postgresql, exporter, redactSecrets, pmmAgentVersion)

diff --git a/managed/services/converters.go b/managed/services/converters.go
@@ -330,7 +330,7 @@ func ToAPIAgent(q *reform.Querier, agent *models.Agent) (inventoryv1.Agent, erro
 			MetricsResolutions: ConvertMetricsResolutions(agent.ExporterOptions.MetricsResolutions),
 		}
 
-		exporter.AutoDiscoveryLimit = agent.PostgreSQLOptions.AutoDiscoveryLimit
+		exporter.AutoDiscoveryLimit = pointer.GetInt32(agent.PostgreSQLOptions.AutoDiscoveryLimit)
 		exporter.MaxExporterConnections = agent.PostgreSQLOptions.MaxExporterConnections
 
 		return exporter, nil

diff --git a/managed/services/management/agent.go b/managed/services/management/agent.go
@@ -203,7 +203,7 @@ func (s *ManagementService) agentToAPI(agent *models.Agent) (*managementv1.Unive
 	if agent.PostgreSQLOptions != nil {
 		ua.PostgresqlOptions = &managementv1.UniversalAgent_PostgreSQLOptions{
 			IsSslKeySet:            agent.PostgreSQLOptions.SSLKey != "",
-			AutoDiscoveryLimit:     agent.PostgreSQLOptions.AutoDiscoveryLimit,
+			AutoDiscoveryLimit:     pointer.GetInt32(agent.PostgreSQLOptions.AutoDiscoveryLimit),
 			MaxExporterConnections: agent.PostgreSQLOptions.MaxExporterConnections,
 		}
 	}

diff --git a/managed/services/management/rds.go b/managed/services/management/rds.go
@@ -416,7 +416,7 @@ func (s *ManagementService) addRDS(ctx context.Context, req *managementv1.AddRDS
 				},
 
 				PostgreSQLOptions: &models.PostgreSQLOptions{
-					AutoDiscoveryLimit:     req.AutoDiscoveryLimit,
+					AutoDiscoveryLimit:     pointer.ToInt32(req.AutoDiscoveryLimit),
 					MaxExporterConnections: req.MaxPostgresqlExporterConnections,
 				},
 			})