From 01d08b4c259f8d411a420d2032d6d7f3a24f659e Mon Sep 17 00:00:00 2001 From: Michael Okoko <10512379+idoqo@users.noreply.github.com> Date: Mon, 27 Nov 2023 15:20:32 +0100 Subject: [PATCH] PMM-12422 use public IP for exporters in pull mode (#2642) * use pmm agent for node IP lookup * re-order switch cases * make address selection more explicit * always expose in pull mode * update tests --- managed/services/agents/agents.go | 6 ++--- managed/services/agents/agents_test.go | 26 +++++++++++++++------- managed/services/agents/mongodb_test.go | 22 +++++++++--------- managed/services/agents/mysql_test.go | 6 ++--- managed/services/agents/node_test.go | 6 ++--- managed/services/agents/postgresql_test.go | 12 +++++----- managed/services/agents/proxysql_test.go | 8 +++---- managed/services/agents/state.go | 2 +- 8 files changed, 49 insertions(+), 39 deletions(-) diff --git a/managed/services/agents/agents.go b/managed/services/agents/agents.go index 0e89aca0ed..7d71672f7d 100644 --- a/managed/services/agents/agents.go +++ b/managed/services/agents/agents.go @@ -145,11 +145,11 @@ func ensureAuthParams(exporter *models.Agent, params *agentpb.SetStateRequest_Ag // getExporterListenAddress returns the appropriate listen address to use for a given exporter. func getExporterListenAddress(node *models.Node, exporter *models.Agent) string { switch { - case !exporter.PushMetrics && node != nil: - return node.Address case exporter.ExposeExporter: return "0.0.0.0" - default: + case exporter.PushMetrics: return "127.0.0.1" + default: + return "0.0.0.0" } } diff --git a/managed/services/agents/agents_test.go b/managed/services/agents/agents_test.go index a99612691e..d3cae56a65 100644 --- a/managed/services/agents/agents_test.go +++ b/managed/services/agents/agents_test.go @@ -51,33 +51,43 @@ func TestPathsBaseForDifferentVersions(t *testing.T) { } func TestGetExporterListenAddress(t *testing.T) { - t.Run("uses node address in pull mode", func(t *testing.T) { + t.Run("uses 127.0.0.1 in push mode", func(t *testing.T) { node := &models.Node{ Address: "1.2.3.4", } - exporter := &models.Agent{} + exporter := &models.Agent{ + PushMetrics: true, + } - assert.Equal(t, "1.2.3.4", getExporterListenAddress(node, exporter)) + assert.Equal(t, "127.0.0.1", getExporterListenAddress(node, exporter)) }) - t.Run("uses 127.0.0.1 in push mode", func(t *testing.T) { + t.Run("exposes exporter address when enabled in push mode", func(t *testing.T) { node := &models.Node{ Address: "1.2.3.4", } exporter := &models.Agent{ - PushMetrics: true, + PushMetrics: true, + ExposeExporter: true, } - assert.Equal(t, "127.0.0.1", getExporterListenAddress(node, exporter)) + assert.Equal(t, "0.0.0.0", getExporterListenAddress(node, exporter)) }) - t.Run("exposes exporter address when enabled", func(t *testing.T) { + t.Run("exposes exporter address when enabled in pull mode", func(t *testing.T) { node := &models.Node{ Address: "1.2.3.4", } exporter := &models.Agent{ - PushMetrics: true, + PushMetrics: false, ExposeExporter: true, } assert.Equal(t, "0.0.0.0", getExporterListenAddress(node, exporter)) }) + t.Run("exposes exporter address if node IP is unavailable in pull mode", func(t *testing.T) { + exporter := &models.Agent{ + PushMetrics: false, + } + + assert.Equal(t, "0.0.0.0", getExporterListenAddress(nil, exporter)) + }) } diff --git a/managed/services/agents/mongodb_test.go b/managed/services/agents/mongodb_test.go index 4c8ae1d56c..2f11b0940f 100644 --- a/managed/services/agents/mongodb_test.go +++ b/managed/services/agents/mongodb_test.go @@ -53,7 +53,7 @@ func TestMongodbExporterConfig225(t *testing.T) { "--compatible-mode", "--discovering-mode", "--mongodb.global-conn-pool", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "MONGODB_URI=mongodb://username:s3cur3%20p%40$$w0r4.@1.2.3.4:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000", @@ -78,7 +78,7 @@ func TestMongodbExporterConfig225(t *testing.T) { "--discovering-mode", "--mongodb.collstats-colls=col1,col2,col3", "--mongodb.global-conn-pool", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", } actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion) require.NoError(t, err) @@ -113,7 +113,7 @@ func TestMongodbExporterConfig226(t *testing.T) { "--compatible-mode", "--discovering-mode", "--mongodb.global-conn-pool", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "MONGODB_URI=mongodb://username:s3cur3%20p%40$$w0r4.@1.2.3.4:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000", @@ -141,7 +141,7 @@ func TestMongodbExporterConfig226(t *testing.T) { "--mongodb.collstats-colls=col1,col2,col3", "--mongodb.global-conn-pool", "--mongodb.indexstats-colls=col1,col2,col3", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", } actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion) require.NoError(t, err) @@ -168,7 +168,7 @@ func TestMongodbExporterConfig226(t *testing.T) { "--mongodb.collstats-colls=col1,col2,col3", "--mongodb.global-conn-pool", "--mongodb.indexstats-colls=col1,col2,col3", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", } actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion) require.NoError(t, err) @@ -196,7 +196,7 @@ func TestMongodbExporterConfig226(t *testing.T) { "--mongodb.collstats-colls=db1.col1.one,db2.col2,db3", "--mongodb.global-conn-pool", "--mongodb.indexstats-colls=db1.col1.one,db2.col2,db3", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", } actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion) require.NoError(t, err) @@ -223,7 +223,7 @@ func TestMongodbExporterConfig226(t *testing.T) { "--mongodb.collstats-colls=db1.col1.one,db2.col2,db3", "--mongodb.global-conn-pool", "--mongodb.indexstats-colls=db1.col1.one,db2.col2,db3", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", } actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion) require.NoError(t, err) @@ -258,7 +258,7 @@ func TestMongodbExporterConfig(t *testing.T) { "--collect.topmetrics", "--no-collect.connpoolstats", "--no-collect.indexusage", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "MONGODB_URI=mongodb://username:s3cur3%20p%40$$w0r4.@1.2.3.4:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000", @@ -343,7 +343,7 @@ func TestMongodbExporterConfig(t *testing.T) { "--collect.database", "--no-collect.connpoolstats", "--no-collect.indexusage", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, } require.NoError(t, err) @@ -376,7 +376,7 @@ func TestNewMongodbExporterConfig(t *testing.T) { Args: []string{ "--compatible-mode", "--mongodb.global-conn-pool", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "MONGODB_URI=mongodb://username:s3cur3%20p%40$$w0r4.@1.2.3.4:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000", @@ -430,7 +430,7 @@ func TestMongodbExporterConfig228_WebConfigAuth(t *testing.T) { "--compatible-mode", "--discovering-mode", "--mongodb.global-conn-pool", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", "--web.config={{ .TextFiles.webConfigPlaceholder }}", } diff --git a/managed/services/agents/mysql_test.go b/managed/services/agents/mysql_test.go index 86c7af3f2c..c9ccd1801c 100644 --- a/managed/services/agents/mysql_test.go +++ b/managed/services/agents/mysql_test.go @@ -88,7 +88,7 @@ func TestMySQLdExporterConfig(t *testing.T) { "--exporter.global-conn-pool", "--exporter.max-idle-conns=3", "--exporter.max-open-conns=3", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s", @@ -193,7 +193,7 @@ func TestMySQLdExporterConfigTablestatsGroupDisabled(t *testing.T) { "--mysql.ssl-ca-file={{ .TextFiles.tlsCa }}", "--mysql.ssl-cert-file={{ .TextFiles.tlsCert }}", "--mysql.ssl-key-file={{ .TextFiles.tlsKey }}", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s&tls=custom", @@ -292,7 +292,7 @@ func TestMySQLdExporterConfigDisabledCollectors(t *testing.T) { "--exporter.global-conn-pool", "--exporter.max-idle-conns=3", "--exporter.max-open-conns=3", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s", diff --git a/managed/services/agents/node_test.go b/managed/services/agents/node_test.go index d82350e0ec..9d0889b631 100644 --- a/managed/services/agents/node_test.go +++ b/managed/services/agents/node_test.go @@ -164,7 +164,7 @@ func TestNodeExporterConfig(t *testing.T) { "--no-collector.xfs", "--no-collector.zfs", "--web.disable-exporter-metrics", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "HTTP_AUTH=pmm:agent-id", @@ -247,7 +247,7 @@ func TestNodeExporterConfig(t *testing.T) { "--no-collector.xfs", "--no-collector.zfs", "--web.disable-exporter-metrics", - "--web.listen-address=:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "HTTP_AUTH=pmm:agent-id", @@ -282,7 +282,7 @@ func TestNodeExporterConfig(t *testing.T) { "--collector.textfile.directory.lr=" + pathsBase(agentVersion, "{{", "}}") + "/collectors/textfile-collector/low-resolution", "--collector.textfile.directory.mr=" + pathsBase(agentVersion, "{{", "}}") + "/collectors/textfile-collector/medium-resolution", "--web.disable-exporter-metrics", - "--web.listen-address=:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "HTTP_AUTH=pmm:agent-id", diff --git a/managed/services/agents/postgresql_test.go b/managed/services/agents/postgresql_test.go index 442f808458..d2206ccd8a 100644 --- a/managed/services/agents/postgresql_test.go +++ b/managed/services/agents/postgresql_test.go @@ -66,7 +66,7 @@ func (s *PostgresExporterConfigTestSuite) SetupTest() { "--collect.custom_query.lr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/low-resolution", "--collect.custom_query.mr", "--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=postgres://username:s3cur3%20p%40$$w0r4.@1.2.3.4:5432/postgres?connect_timeout=1&sslmode=disable", @@ -167,7 +167,7 @@ func (s *PostgresExporterConfigTestSuite) TestDisabledCollectors() { "--collect.custom_query.lr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/low-resolution", "--collect.custom_query.mr", "--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, } requireNoDuplicateFlags(s.T(), actual.Args) @@ -205,7 +205,7 @@ func TestAutoDiscovery(t *testing.T) { "--collect.custom_query.lr.directory=" + pathsBase(pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/low-resolution", "--collect.custom_query.mr", "--collect.custom_query.mr.directory=" + pathsBase(pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=postgres://username:s3cur3%20p%40$$w0r4.@1.2.3.4:5432/postgres?connect_timeout=1&sslmode=disable", @@ -324,7 +324,7 @@ func (s *PostgresExporterConfigTestSuite) TestAzureTimeout() { "--collect.custom_query.mr", "--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution", "--exclude-databases=template0,template1,postgres,cloudsqladmin,pmm-managed-dev,azure_maintenance,rdsadmin", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=postgres://username:s3cur3%20p%40$$w0r4.@1.2.3.4:5432/postgres?connect_timeout=5&sslmode=disable", @@ -370,7 +370,7 @@ func (s *PostgresExporterConfigTestSuite) TestPrometheusWebConfig() { "--collect.custom_query.mr", "--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution", "--exclude-databases=template0,template1,postgres,cloudsqladmin,pmm-managed-dev,azure_maintenance,rdsadmin", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", "--web.config={{ .TextFiles.webConfigPlaceholder }}", }, Env: []string{ @@ -419,7 +419,7 @@ func (s *PostgresExporterConfigTestSuite) TestSSLSni() { "--collect.custom_query.mr", "--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution", "--exclude-databases=template0,template1,postgres,cloudsqladmin,pmm-managed-dev,azure_maintenance,rdsadmin", - "--web.listen-address=1.2.3.4:{{ .listen_port }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", "--web.config={{ .TextFiles.webConfigPlaceholder }}", }, Env: []string{ diff --git a/managed/services/agents/proxysql_test.go b/managed/services/agents/proxysql_test.go index 5c979dfac5..7e608169ba 100644 --- a/managed/services/agents/proxysql_test.go +++ b/managed/services/agents/proxysql_test.go @@ -55,7 +55,7 @@ func TestProxySQLExporterConfig(t *testing.T) { "-collect.mysql_connection_pool", "-collect.mysql_status", "-collect.stats_memory_metrics", - "-web.listen-address=1.2.3.4:{{ .listen_port }}", + "-web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s", @@ -89,7 +89,7 @@ func TestProxySQLExporterConfig(t *testing.T) { Args: []string{ "-collect.mysql_connection_pool", "-collect.mysql_status", - "-web.listen-address=1.2.3.4:{{ .listen_port }}", + "-web.listen-address=0.0.0.0:{{ .listen_port }}", }, } require.Equal(t, expected.Args, actual.Args) @@ -120,7 +120,7 @@ func TestProxySQLExporterConfig(t *testing.T) { "-collect.mysql_status", "-collect.stats_command_counter", "-collect.stats_memory_metrics", - "-web.listen-address=1.2.3.4:{{ .listen_port }}", + "-web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s", @@ -161,7 +161,7 @@ func TestProxySQLExporterConfig(t *testing.T) { "-collect.runtime_mysql_servers", "-collect.stats_command_counter", "-collect.stats_memory_metrics", - "-web.listen-address=1.2.3.4:{{ .listen_port }}", + "-web.listen-address=0.0.0.0:{{ .listen_port }}", }, Env: []string{ "DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s", diff --git a/managed/services/agents/state.go b/managed/services/agents/state.go index 49ad75ecbb..f2d840a6a5 100644 --- a/managed/services/agents/state.go +++ b/managed/services/agents/state.go @@ -226,7 +226,7 @@ func (u *StateUpdater) sendSetStateRequest(ctx context.Context, agent *pmmAgentI if err != nil { return err } - node, _ := models.FindNodeByID(u.db.Querier, pointer.GetString(row.NodeID)) + node, _ := models.FindNodeByID(u.db.Querier, pointer.GetString(pmmAgent.RunsOnNodeID)) switch row.AgentType { //nolint:exhaustive case models.MySQLdExporterType: agentProcesses[row.AgentID] = mysqldExporterConfig(node, service, row, redactMode, pmmAgentVersion)