Skip to content

Commit

Permalink
PMM-13145 Certs Fix - Backward compatibility for older clients. (#3014)…
Browse files Browse the repository at this point in the history 
… (#3017)

* PMM-13145 Fix for older clients.

* PMM-13145 Small changes.

* PMM-13145 Lint.

* PMM-13145 Lint.

* PMM-13145 Logic fix.

* PMM-13145 Refactor.

* PMM-13145 Fix after refactor.

* Update managed/models/agent_model.go



* Update managed/models/agent_model.go



---------

Co-authored-by: Nurlan Moldomurov <[email protected]>
  • Loading branch information
@JiriCtvrtka @BupycHuk
JiriCtvrtka and BupycHuk authored Jun 5, 2024
1 parent 7221d3e commit 74e5752
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 48 deletions.
18 changes: 18 additions & 0 deletions managed/models/agent_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,24 @@ func FindPmmAgentIDToRunActionOrJob(pmmAgentID string, agents []*Agent) (string,
return "", status.Errorf(codes.FailedPrecondition, "Couldn't find pmm-agent-id to run action")
}

// ExtractPmmAgentVersionFromAgent extract PMM agent version from Agent by pmm-agent-id.
func ExtractPmmAgentVersionFromAgent(q *reform.Querier, agent *Agent) *version.Parsed {
pmmAgentID, err := ExtractPmmAgentID(agent)
if err != nil {
return nil
}
pmmAgent, err := FindAgentByID(q, pmmAgentID)
if err != nil {
return nil
}
version, err := version.Parse(pointer.GetString(pmmAgent.Version))
if err != nil {
return nil
}

return version
}

// ExtractPmmAgentID extract pmm-agent-id from Agent by type.
func ExtractPmmAgentID(agent *Agent) (string, error) {
switch agent.AgentType {
Expand Down
12 changes: 11 additions & 1 deletion managed/models/agent_model.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ const (
VMAgentType AgentType = "vmagent"
)

var v2_42 = version.MustParse("2.42.0-0")

// PMMServerAgentID is a special Agent ID representing pmm-agent on PMM Server.
const PMMServerAgentID = string("pmm-server") // no /agent_id/ prefix

Expand Down Expand Up @@ -313,7 +315,7 @@ type DSNParams struct {
}

// DSN returns a DSN string for accessing a given Service with this Agent (and an implicit driver).
func (s *Agent) DSN(service *Service, dsnParams DSNParams, tdp *DelimiterPair) string { //nolint:cyclop,maintidx
func (s *Agent) DSN(service *Service, dsnParams DSNParams, tdp *DelimiterPair, pmmAgentVersion *version.Parsed) string { //nolint:cyclop,maintidx
host := pointer.GetString(service.Address)
port := pointer.GetUint16(service.Port)
socket := pointer.GetString(service.Socket)
Expand All @@ -340,8 +342,12 @@ func (s *Agent) DSN(service *Service, dsnParams DSNParams, tdp *DelimiterPair) s
cfg.Params = make(map[string]string)
if s.TLS {
// It is mandatory to have "custom" as the first case.
// Except case for backward compatibility.
// Skip verify for "custom" is handled on pmm-agent side.
switch {
// Backward compatibility
case s.TLSSkipVerify && (pmmAgentVersion == nil || pmmAgentVersion.Less(v2_42)):
cfg.Params["tls"] = skipVerify
case len(s.Files()) != 0:
cfg.Params["tls"] = "custom"
case s.TLSSkipVerify:
Expand Down Expand Up @@ -371,8 +377,12 @@ func (s *Agent) DSN(service *Service, dsnParams DSNParams, tdp *DelimiterPair) s
cfg.Params = make(map[string]string)
if s.TLS {
// It is mandatory to have "custom" as the first case.
// Except case for backward compatibility.
// Skip verify for "custom" is handled on pmm-agent side.
switch {
// Backward compatibility
case pmmAgentVersion != nil && s.TLSSkipVerify && pmmAgentVersion.Less(v2_42):
cfg.Params["tls"] = skipVerify
case len(s.Files()) != 0:
cfg.Params["tls"] = "custom"
case s.TLSSkipVerify:
Expand Down
42 changes: 21 additions & 21 deletions managed/models/agent_model_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ func TestAgent(t *testing.T) {
} {
t.Run(string(typ), func(t *testing.T) {
agent.AgentType = typ
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}

t.Run("MongoDBNoDatabase", func(t *testing.T) {
agent.AgentType = models.MongoDBExporterType

assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?directConnection=true", agent.DSN(service, models.DSNParams{}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?directConnection=true", agent.DSN(service, models.DSNParams{}, nil, nil))
})
})

Expand All @@ -94,7 +94,7 @@ func TestAgent(t *testing.T) {
} {
t.Run(string(typ), func(t *testing.T) {
agent.AgentType = typ
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}
})
Expand All @@ -113,7 +113,7 @@ func TestAgent(t *testing.T) {
} {
t.Run(string(typ), func(t *testing.T) {
agent.AgentType = typ
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}
})
Expand Down Expand Up @@ -159,7 +159,7 @@ func TestAgent(t *testing.T) {
} {
t.Run(string(typ), func(t *testing.T) {
agent.AgentType = typ
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}

Expand All @@ -169,8 +169,8 @@ func TestAgent(t *testing.T) {
agent.MongoDBOptions.TLSCertificateKeyFilePassword = ""
agent.MongoDBOptions.AuthenticationMechanism = ""

assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?directConnection=true&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?directConnection=true&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{}, nil, nil))
expectedFiles := map[string]string{
"caFilePlaceholder": "cert",
"certificateKeyFilePlaceholder": "key",
Expand All @@ -185,8 +185,8 @@ func TestAgent(t *testing.T) {
agent.MongoDBOptions.AuthenticationMechanism = "MONGO-X509"
agent.MongoDBOptions.AuthenticationDatabase = "$external"

assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?authMechanism=MONGO-X509&authSource=%24external&connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?authMechanism=MONGO-X509&authSource=%24external&directConnection=true&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?authMechanism=MONGO-X509&authSource=%24external&connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?authMechanism=MONGO-X509&authSource=%24external&directConnection=true&ssl=true&tlsCaFile={{.TextFiles.caFilePlaceholder}}&tlsCertificateKeyFile={{.TextFiles.certificateKeyFilePlaceholder}}", agent.DSN(service, models.DSNParams{}, nil, nil))
expectedFiles := map[string]string{
"caFilePlaceholder": "cert",
"certificateKeyFilePlaceholder": "key",
Expand Down Expand Up @@ -217,15 +217,15 @@ func TestAgent(t *testing.T) {
} {
t.Run(string(typ), func(t *testing.T) {
agent.AgentType = typ
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}

t.Run("MongoDBNoDatabase", func(t *testing.T) {
agent.AgentType = models.MongoDBExporterType

assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000&ssl=true&tlsInsecure=true", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?directConnection=true&ssl=true&tlsInsecure=true", agent.DSN(service, models.DSNParams{}, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000&ssl=true&tlsInsecure=true", agent.DSN(service, models.DSNParams{DialTimeout: time.Second}, nil, nil))
assert.Equal(t, "mongodb://username:s3cur3%20p%[email protected]:12345/?directConnection=true&ssl=true&tlsInsecure=true", agent.DSN(service, models.DSNParams{}, nil, nil))
})
})
}
Expand Down Expand Up @@ -255,13 +255,13 @@ func TestPostgresAgentTLS(t *testing.T) {
t.Run(name, func(t *testing.T) {
agent.TLS = testCase.tls
agent.TLSSkipVerify = testCase.tlsSkipVerify
assert.Equal(t, testCase.expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, testCase.expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
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}
assert.Equal(t, testCase.expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, testCase.expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}
}
Expand All @@ -278,7 +278,7 @@ func TestPostgresWithSocket(t *testing.T) {
Socket: pointer.ToString("/var/run/postgres"),
}
expect := "postgres://username@/database?connect_timeout=1&host=%2Fvar%2Frun%2Fpostgres&sslmode=verify-ca"
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})

t.Run("empty-user-password", func(t *testing.T) {
Expand All @@ -289,7 +289,7 @@ func TestPostgresWithSocket(t *testing.T) {
Socket: pointer.ToString("/var/run/postgres"),
}
expect := "postgres:///database?connect_timeout=1&host=%2Fvar%2Frun%2Fpostgres&sslmode=disable"
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})

t.Run("dir-with-symbols", func(t *testing.T) {
Expand All @@ -300,7 +300,7 @@ func TestPostgresWithSocket(t *testing.T) {
Socket: pointer.ToString(`/tmp/123\ A0m\%\$\@\8\,\+\-`),
}
expect := "postgres:///database?connect_timeout=1&host=%2Ftmp%2F123%5C+A0m%5C%25%5C%24%5C%40%5C8%5C%2C%5C%2B%5C-&sslmode=disable"
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}

Expand All @@ -316,7 +316,7 @@ func TestMongoWithSocket(t *testing.T) {
Socket: pointer.ToString("/tmp/mongodb-27017.sock"),
}
expect := "mongodb://username@%2Ftmp%2Fmongodb-27017.sock/database?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000&ssl=true"
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})

t.Run("empty-user-password", func(t *testing.T) {
Expand All @@ -327,7 +327,7 @@ func TestMongoWithSocket(t *testing.T) {
Socket: pointer.ToString("/tmp/mongodb-27017.sock"),
}
expect := "mongodb://%2Ftmp%2Fmongodb-27017.sock/database?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000"
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})

t.Run("dir-with-symbols", func(t *testing.T) {
Expand All @@ -338,7 +338,7 @@ func TestMongoWithSocket(t *testing.T) {
Socket: pointer.ToString(`/tmp/123\ A0m\%\$\@\8\,\+\-/mongodb-27017.sock`),
}
expect := "mongodb://%2Ftmp%2F123%5C%20A0m%5C%25%5C$%5C%40%5C8%5C,%5C+%5C-%2Fmongodb-27017.sock/database?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000"
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil))
assert.Equal(t, expect, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil))
})
}

Expand Down
3 changes: 2 additions & 1 deletion managed/models/dsn_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ func FindDSNByServiceIDandPMMAgentID(q *reform.Querier, serviceID, pmmAgentID, d
}
if len(fexp) == 1 {
agent := fexp[0]
return agent.DSN(svc, dsnParams, nil), agent, nil
pmmAgentVersion := ExtractPmmAgentVersionFromAgent(q, agent)
return agent.DSN(svc, dsnParams, nil, pmmAgentVersion), agent, nil
}
if len(fexp) > 1 {
return "", nil, status.Errorf(codes.FailedPrecondition, "Couldn't resolve dsn, as there should be only one agent")
Expand Down
13 changes: 8 additions & 5 deletions managed/services/agents/connection_checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,14 @@ func (c *ConnectionChecker) CheckConnectionToService(ctx context.Context, q *ref

func connectionRequest(q *reform.Querier, service *models.Service, agent *models.Agent) (*agentpb.CheckConnectionRequest, error) {
var request *agentpb.CheckConnectionRequest

pmmAgentVersion := models.ExtractPmmAgentVersionFromAgent(q, agent)
switch service.ServiceType {
case models.MySQLServiceType:
tdp := agent.TemplateDelimiters(service)
request = &agentpb.CheckConnectionRequest{
Type: inventorypb.ServiceType_MYSQL_SERVICE,
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName}, nil),
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName}, nil, pmmAgentVersion),
Timeout: durationpb.New(3 * time.Second),
TextFiles: &agentpb.TextFiles{
Files: agent.Files(),
Expand All @@ -158,8 +160,9 @@ func connectionRequest(q *reform.Querier, service *models.Service, agent *models
return nil, err
}
request = &agentpb.CheckConnectionRequest{
Type: inventorypb.ServiceType_POSTGRESQL_SERVICE,
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName, PostgreSQLSupportsSSLSNI: sqlSniSupported}, nil),
Type: inventorypb.ServiceType_POSTGRESQL_SERVICE,
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName, PostgreSQLSupportsSSLSNI: sqlSniSupported},
nil, pmmAgentVersion),
Timeout: durationpb.New(3 * time.Second),
TextFiles: &agentpb.TextFiles{
Files: agent.Files(),
Expand All @@ -171,7 +174,7 @@ func connectionRequest(q *reform.Querier, service *models.Service, agent *models
tdp := agent.TemplateDelimiters(service)
request = &agentpb.CheckConnectionRequest{
Type: inventorypb.ServiceType_MONGODB_SERVICE,
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName}, nil),
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName}, nil, pmmAgentVersion),
Timeout: durationpb.New(3 * time.Second),
TextFiles: &agentpb.TextFiles{
Files: agent.Files(),
Expand All @@ -182,7 +185,7 @@ func connectionRequest(q *reform.Querier, service *models.Service, agent *models
case models.ProxySQLServiceType:
request = &agentpb.CheckConnectionRequest{
Type: inventorypb.ServiceType_PROXYSQL_SERVICE,
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName}, nil),
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: 2 * time.Second, Database: service.DatabaseName}, nil, pmmAgentVersion),
Timeout: durationpb.New(3 * time.Second),
}
case models.ExternalServiceType:
Expand Down
6 changes: 3 additions & 3 deletions managed/services/agents/mongodb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func mongodbExporterConfig(node *models.Node, service *models.Service, exporter
database = exporter.MongoDBOptions.AuthenticationDatabase
}
env := []string{
fmt.Sprintf("MONGODB_URI=%s", exporter.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: database}, tdp)),
fmt.Sprintf("MONGODB_URI=%s", exporter.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: database}, tdp, pmmAgentVersion)),
}

res := &agentpb.SetStateRequest_AgentProcess{
Expand Down Expand Up @@ -270,11 +270,11 @@ func defaultCollectors(collectAll bool) map[string]collectorArgs {
}

// qanMongoDBProfilerAgentConfig returns desired configuration of qan-mongodb-profiler-agent built-in agent.
func qanMongoDBProfilerAgentConfig(service *models.Service, agent *models.Agent) *agentpb.SetStateRequest_BuiltinAgent {
func qanMongoDBProfilerAgentConfig(service *models.Service, agent *models.Agent, pmmAgentVersion *version.Parsed) *agentpb.SetStateRequest_BuiltinAgent {
tdp := agent.TemplateDelimiters(service)
return &agentpb.SetStateRequest_BuiltinAgent{
Type: inventorypb.AgentType_QAN_MONGODB_PROFILER_AGENT,
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: ""}, nil),
Dsn: agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: ""}, nil, pmmAgentVersion),
DisableQueryExamples: agent.QueryExamplesDisabled,
MaxQueryLength: agent.MaxQueryLength,
TextFiles: &agentpb.TextFiles{
Expand Down
Loading

0 comments on commit 74e5752

Please sign in to comment.