diff --git a/documentation/docs/install-pmm/install-pmm-client/connect-database/valkey-redis.md b/documentation/docs/install-pmm/install-pmm-client/connect-database/valkey-redis.md index 1b70ebc628..e3e67da8be 100644 --- a/documentation/docs/install-pmm/install-pmm-client/connect-database/valkey-redis.md +++ b/documentation/docs/install-pmm/install-pmm-client/connect-database/valkey-redis.md @@ -141,6 +141,23 @@ You can add your Valkey or Redis service to PMM either through the user interfac Valkey-TLS ``` + === "With mutual TLS" + + Add an instance that requires client certificate authentication: + ```sh + pmm-admin add valkey \ + Valkey-mTLS \ + valkey-server.example.com:6379 \ + --username=pmm \ + --password=StrongPassword123! \ + --tls \ + --tls-ca=/path/to/ca.pem \ + --tls-cert=/path/to/client-cert.pem \ + --tls-key=/path/to/client-key.pem + ``` + + `--tls-ca` already supplies the trust anchor, so a self-signed certificate does not need `--tls-skip-verify`. Add that flag only when a validation failure cannot be corrected — a SAN mismatch, for example — and only in development or testing: it disables server authentication entirely. + === ":material-cog: Via inventory commands (Advanced)" PMM also provides inventory commands for more granular control: diff --git a/managed/models/agent_model.go b/managed/models/agent_model.go index 2cbe0659a4..8ae33fb2d7 100644 --- a/managed/models/agent_model.go +++ b/managed/models/agent_model.go @@ -43,6 +43,14 @@ import ( // pmm-managed's PostgreSQL, qan-api's ClickHouse, and VictoriaMetrics. type AgentType string +// Text file names carrying TLS material to pmm-agent. Exporter arguments reference them +// as {{ .TextFiles. }}, so renaming one is a change to the agent wire protocol. +const ( + TLSCaFileName = "tlsCa" + TLSCertFileName = "tlsCert" + TLSKeyFileName = "tlsKey" +) + const ( certificateFilePlaceholder = "certificateFilePlaceholder" certificateKeyFilePlaceholder = "certificateKeyFilePlaceholder" @@ -955,13 +963,13 @@ func (a Agent) Files() map[string]string { //nolint:gocognit files := make(map[string]string) if a.ValkeyOptions.SSLCa != "" { - files["tlsCa"] = a.ValkeyOptions.SSLCa + files[TLSCaFileName] = a.ValkeyOptions.SSLCa } if a.ValkeyOptions.SSLCert != "" { - files["tlsCert"] = a.ValkeyOptions.SSLCert + files[TLSCertFileName] = a.ValkeyOptions.SSLCert } if a.ValkeyOptions.SSLKey != "" { - files["tlsKey"] = a.ValkeyOptions.SSLKey + files[TLSKeyFileName] = a.ValkeyOptions.SSLKey } if len(files) != 0 { @@ -996,6 +1004,14 @@ func (a Agent) TemplateDelimiters(svc *Service) *DelimiterPair { if a.PostgreSQLOptions.SSLKey != "" { templateParams = append(templateParams, a.PostgreSQLOptions.SSLKey) } + case ValkeyServiceType: + // pmm-agent renders every text file's content as a template, so all three + // certificates have to be considered, not just the private key. + for _, s := range []string{a.ValkeyOptions.SSLCa, a.ValkeyOptions.SSLCert, a.ValkeyOptions.SSLKey} { + if s != "" { + templateParams = append(templateParams, s) + } + } case ProxySQLServiceType: case HAProxyServiceType: case ExternalServiceType: diff --git a/managed/models/agent_model_test.go b/managed/models/agent_model_test.go index c5b012975b..40e5b39002 100644 --- a/managed/models/agent_model_test.go +++ b/managed/models/agent_model_test.go @@ -286,7 +286,11 @@ func TestPostgresAgentTLS(t *testing.T) { } func TestValkey(t *testing.T) { + t.Parallel() + t.Run("Redis DSN", func(t *testing.T) { + t.Parallel() + agent := &models.Agent{ Username: new("username"), Password: new("s3cur3 p@$$w0r4."), @@ -305,6 +309,8 @@ func TestValkey(t *testing.T) { }) t.Run("Valkey DSN with TLS", func(t *testing.T) { + t.Parallel() + agent := &models.Agent{ Username: new("username"), Password: new("s3cur3 p@$$w0r4."), @@ -326,6 +332,48 @@ func TestValkey(t *testing.T) { require.Equal(t, expected, agent.DSN(service, models.DSNParams{DialTimeout: time.Second, Database: "database"}, nil, nil)) }) + + t.Run("Files", func(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + options models.ValkeyOptions + expected map[string]string + }{ + "all": {models.ValkeyOptions{SSLCa: "aa", SSLCert: "bb", SSLKey: "cc"}, map[string]string{"tlsCa": "aa", "tlsCert": "bb", "tlsKey": "cc"}}, + "ca": {models.ValkeyOptions{SSLCa: "aa"}, map[string]string{"tlsCa": "aa"}}, + "pair": {models.ValkeyOptions{SSLCert: "bb", SSLKey: "cc"}, map[string]string{"tlsCert": "bb", "tlsKey": "cc"}}, + "none": {models.ValkeyOptions{}, nil}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + agent := models.Agent{AgentType: models.ValkeyExporterType, ValkeyOptions: tc.options} + + require.Equal(t, tc.expected, agent.Files()) + }) + } + }) + + t.Run("TemplateDelimiters avoid certificate content", func(t *testing.T) { + t.Parallel() + + service := &models.Service{ServiceType: models.ValkeyServiceType, Address: new("1.2.3.4")} + + for name, options := range map[string]models.ValkeyOptions{ + "ca": {SSLCa: "aa {{ bb"}, + "cert": {SSLCert: "aa {{ bb"}, + "key": {SSLKey: "aa {{ bb"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + agent := models.Agent{AgentType: models.ValkeyExporterType, ValkeyOptions: options} + + require.Equal(t, &models.DelimiterPair{Left: "[[", Right: "]]"}, agent.TemplateDelimiters(service)) + }) + } + }) } func TestPostgresWithSocket(t *testing.T) { diff --git a/managed/services/agents/agents.go b/managed/services/agents/agents.go index c0bf77ad48..09ebb4cbae 100644 --- a/managed/services/agents/agents.go +++ b/managed/services/agents/agents.go @@ -118,6 +118,9 @@ func redactWords(agent *models.Agent) []string { if s := agent.PostgreSQLOptions.SSLKey; s != "" { words = append(words, s) } + if s := agent.ValkeyOptions.SSLKey; s != "" { + words = append(words, s) + } return words } diff --git a/managed/services/agents/valkey.go b/managed/services/agents/valkey.go index ea21abd864..2db0d06ee6 100644 --- a/managed/services/agents/valkey.go +++ b/managed/services/agents/valkey.go @@ -40,13 +40,33 @@ func valkeyExporterConfig(node *models.Node, service *models.Service, exporter * args = append(args, "--web.telemetry-path="+exporter.ExporterOptions.MetricsPath) } + textFiles := exporter.Files() + if exporter.TLS { + if exporter.TLSSkipVerify { + args = append(args, "--skip-tls-verification") + } + + // The flag names come from oliver006/redis_exporter, shipped as valkey_exporter; + // all four have been stable since v1.72.1, the build the first Valkey release used. + tlsFileFlags := []struct{ file, flag string }{ + {models.TLSCaFileName, "--tls-ca-cert-file"}, + {models.TLSCertFileName, "--tls-client-cert-file"}, + {models.TLSKeyFileName, "--tls-client-key-file"}, + } + for _, f := range tlsFileFlags { + if _, ok := textFiles[f.file]; ok { + args = append(args, f.flag+"="+tdp.Left+" .TextFiles."+f.file+" "+tdp.Right) + } + } + } + dsnParams := models.DSNParams{} connectionTimeout := exporter.EffectiveDialTimeout() - args = append(args, "--redis.addr="+exporter.DSN(service, dsnParams, nil, pmmAgentVersion)) + args = append(args, "--redis.addr="+exporter.DSN(service, dsnParams, tdp, pmmAgentVersion)) args = append(args, "--connection-timeout="+connectionTimeout.String()) // valkey_exporter parses flags with the stdlib flag package, which rejects --log.level - // and has no fatal level (PMM-15201). + // and has no fatal level. args = withLogLevelFlag(args, "--log-level", exporter.LogLevel, pmmAgentVersion, false) sort.Strings(args) @@ -55,7 +75,7 @@ func valkeyExporterConfig(node *models.Node, service *models.Service, exporter * TemplateLeftDelim: tdp.Left, TemplateRightDelim: tdp.Right, Args: args, - TextFiles: exporter.Files(), + TextFiles: textFiles, } if redactMode != exposeSecrets { res.RedactWords = redactWords(exporter) diff --git a/managed/services/agents/valkey_test.go b/managed/services/agents/valkey_test.go index 77c465a811..b469c9e4eb 100644 --- a/managed/services/agents/valkey_test.go +++ b/managed/services/agents/valkey_test.go @@ -34,8 +34,9 @@ func TestValkeyExporterConfig(t *testing.T) { pmmAgentVersion := version.MustParse("2.44.0") node := &models.Node{Address: "1.2.3.4"} service := &models.Service{ - Address: new("1.2.3.4"), - Port: new(uint16(6379)), + ServiceType: models.ValkeyServiceType, + Address: new("1.2.3.4"), + Port: new(uint16(6379)), } t.Run("DefaultTimeoutUsesFlag", func(t *testing.T) { @@ -78,7 +79,7 @@ func TestValkeyExporterConfig(t *testing.T) { require.Contains(t, actual.Args, "--redis.addr=redis://username:secret@1.2.3.4:6379") }) - // PMM-15201: valkey_exporter only knows --log-level. Passing --log.level made it print + // valkey_exporter only knows --log-level. Passing --log.level made it print // its usage, exit with code 2 and land the agent in the DONE state. t.Run("LogLevel", func(t *testing.T) { t.Parallel() @@ -106,4 +107,239 @@ func TestValkeyExporterConfig(t *testing.T) { }) } }) + + t.Run("TLS", func(t *testing.T) { + t.Parallel() + + // ValkeyOptions.TLS is deliberately left unset: the exporter arguments and the DSN scheme + // both key off Agent.TLS, and the two flags must not drift apart. + type exporterFixture struct { + tls bool + skipVerify bool + valkey models.ValkeyOptions + } + + newExporter := func(f exporterFixture) *models.Agent { + return &models.Agent{ + AgentID: "agent-id", + AgentType: models.ValkeyExporterType, + Username: new("username"), + Password: new("secret"), + TLS: f.tls, + TLSSkipVerify: f.skipVerify, + ValkeyOptions: f.valkey, + } + } + + allCertificates := models.ValkeyOptions{SSLCa: "ca-pem", SSLCert: "cert-pem", SSLKey: "key-pem"} + + requireNoCertificateArgs := func(t *testing.T, args []string) { + t.Helper() + for _, arg := range args { + require.False(t, strings.HasPrefix(arg, "--tls-"), "unexpected argument %q", arg) + } + } + + requireNoTLSArgs := func(t *testing.T, args []string) { + t.Helper() + requireNoCertificateArgs(t, args) + require.NotContains(t, args, "--skip-tls-verification") + } + + t.Run("MutualTLS", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: allCertificates}), redactSecrets, pmmAgentVersion) + expected := &agentv1.SetStateRequest_AgentProcess{ + Type: inventoryv1.AgentType_AGENT_TYPE_VALKEY_EXPORTER, + TemplateLeftDelim: "{{", + TemplateRightDelim: "}}", + Args: []string{ + "--connection-timeout=3s", + "--include-config-metrics", + "--include-system-metrics", + "--redis.addr=rediss://username:secret@1.2.3.4:6379", + "--tls-ca-cert-file={{ .TextFiles.tlsCa }}", + "--tls-client-cert-file={{ .TextFiles.tlsCert }}", + "--tls-client-key-file={{ .TextFiles.tlsKey }}", + "--web.listen-address=0.0.0.0:{{ .listen_port }}", + }, + TextFiles: map[string]string{ + "tlsCa": "ca-pem", + "tlsCert": "cert-pem", + "tlsKey": "key-pem", + }, + RedactWords: []string{"secret", "key-pem"}, + } + requireNoDuplicateFlags(t, actual.Args) + require.Equal(t, expected, actual) + }) + + t.Run("SkipVerifyWithoutCertificates", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, skipVerify: true}), redactSecrets, pmmAgentVersion) + require.Contains(t, actual.Args, "--skip-tls-verification") + require.Contains(t, actual.Args, "--redis.addr=rediss://username:secret@1.2.3.4:6379") + require.Nil(t, actual.TextFiles) + requireNoCertificateArgs(t, actual.Args) + }) + + t.Run("SkipVerifyWithCertificates", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, skipVerify: true, valkey: allCertificates}), redactSecrets, pmmAgentVersion) + require.Contains(t, actual.Args, "--skip-tls-verification") + require.Contains(t, actual.Args, "--tls-ca-cert-file={{ .TextFiles.tlsCa }}") + require.Contains(t, actual.Args, "--tls-client-cert-file={{ .TextFiles.tlsCert }}") + require.Contains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + }) + + t.Run("CertificateAuthorityOnly", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: models.ValkeyOptions{SSLCa: "ca-pem"}}), redactSecrets, pmmAgentVersion) + require.Contains(t, actual.Args, "--tls-ca-cert-file={{ .TextFiles.tlsCa }}") + require.NotContains(t, actual.Args, "--tls-client-cert-file={{ .TextFiles.tlsCert }}") + require.NotContains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + require.Equal(t, map[string]string{"tlsCa": "ca-pem"}, actual.TextFiles) + }) + + t.Run("ClientCertificateWithoutCertificateAuthority", func(t *testing.T) { + t.Parallel() + + options := models.ValkeyOptions{SSLCert: "cert-pem", SSLKey: "key-pem"} + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: options}), redactSecrets, pmmAgentVersion) + require.NotContains(t, actual.Args, "--tls-ca-cert-file={{ .TextFiles.tlsCa }}") + require.Contains(t, actual.Args, "--tls-client-cert-file={{ .TextFiles.tlsCert }}") + require.Contains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + }) + + // An incomplete key pair is rejected by the exporter, not silently completed here. + t.Run("ClientCertificateWithoutKey", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: models.ValkeyOptions{SSLCert: "cert-pem"}}), redactSecrets, pmmAgentVersion) + require.Contains(t, actual.Args, "--tls-client-cert-file={{ .TextFiles.tlsCert }}") + require.NotContains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + require.Equal(t, map[string]string{"tlsCert": "cert-pem"}, actual.TextFiles) + }) + + t.Run("PrivateKeyWithoutCertificate", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: models.ValkeyOptions{SSLKey: "key-pem"}}), redactSecrets, pmmAgentVersion) + require.Contains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + require.NotContains(t, actual.Args, "--tls-client-cert-file={{ .TextFiles.tlsCert }}") + require.Equal(t, map[string]string{"tlsKey": "key-pem"}, actual.TextFiles) + }) + + // The files still reach the host, but nothing must point the exporter at them over a plaintext link. + t.Run("CertificatesIgnoredWhenTLSDisabled", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{valkey: allCertificates}), redactSecrets, pmmAgentVersion) + requireNoTLSArgs(t, actual.Args) + require.Contains(t, actual.Args, "--redis.addr=redis://username:secret@1.2.3.4:6379") + require.Equal(t, map[string]string{"tlsCa": "ca-pem", "tlsCert": "cert-pem", "tlsKey": "key-pem"}, actual.TextFiles) + }) + + t.Run("SkipVerifyIgnoredWhenTLSDisabled", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{skipVerify: true, valkey: allCertificates}), redactSecrets, pmmAgentVersion) + requireNoTLSArgs(t, actual.Args) + require.Contains(t, actual.Args, "--redis.addr=redis://username:secret@1.2.3.4:6379") + }) + + t.Run("NoTLSArgumentsByDefault", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{}), redactSecrets, pmmAgentVersion) + requireNoTLSArgs(t, actual.Args) + require.Nil(t, actual.TextFiles) + }) + + t.Run("SocketConnection", func(t *testing.T) { + t.Parallel() + + socketService := &models.Service{ + ServiceType: models.ValkeyServiceType, + Socket: new("/tmp/valkey.sock"), + } + + actual := valkeyExporterConfig(node, socketService, newExporter(exporterFixture{tls: true, valkey: allCertificates}), redactSecrets, pmmAgentVersion) + require.Contains(t, actual.Args, "--tls-ca-cert-file={{ .TextFiles.tlsCa }}") + require.Contains(t, actual.Args, "--tls-client-cert-file={{ .TextFiles.tlsCert }}") + require.Contains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + require.Contains(t, actual.Args, "--redis.addr=rediss://username:secret@%2Ftmp%2Fvalkey.sock") + }) + + // No feature gate applies: every flag exists in the exporter shipped since Valkey support landed. + t.Run("UnknownAgentVersion", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, skipVerify: true, valkey: allCertificates}), redactSecrets, nil) + require.Contains(t, actual.Args, "--skip-tls-verification") + require.Contains(t, actual.Args, "--tls-ca-cert-file={{ .TextFiles.tlsCa }}") + require.Contains(t, actual.Args, "--tls-client-cert-file={{ .TextFiles.tlsCert }}") + require.Contains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + }) + + t.Run("PrivateKeyIsRedacted", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: allCertificates}), redactSecrets, pmmAgentVersion) + require.Contains(t, actual.RedactWords, "key-pem") + require.NotContains(t, actual.RedactWords, "ca-pem") + require.NotContains(t, actual.RedactWords, "cert-pem") + }) + + // Exposing secrets drops the redaction list only; the exporter still needs the material. + t.Run("SecretsExposedOnRequest", func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: allCertificates}), exposeSecrets, pmmAgentVersion) + require.Nil(t, actual.RedactWords) + require.Equal(t, "key-pem", actual.TextFiles["tlsKey"]) + require.Contains(t, actual.Args, "--tls-client-key-file={{ .TextFiles.tlsKey }}") + }) + + // pmm-agent renders the certificate contents as templates, so the delimiters have to + // avoid anything the certificates themselves contain. + t.Run("DelimitersAvoidCertificateContent", func(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + options models.ValkeyOptions + expected string + }{ + "ca": {models.ValkeyOptions{SSLCa: "ca {{ pem"}, "--tls-ca-cert-file=[[ .TextFiles.tlsCa ]]"}, + "cert": {models.ValkeyOptions{SSLCert: "cert {{ pem"}, "--tls-client-cert-file=[[ .TextFiles.tlsCert ]]"}, + "key": {models.ValkeyOptions{SSLKey: "key {{ pem"}, "--tls-client-key-file=[[ .TextFiles.tlsKey ]]"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + actual := valkeyExporterConfig(node, service, newExporter(exporterFixture{tls: true, valkey: tc.options}), redactSecrets, pmmAgentVersion) + require.Equal(t, "[[", actual.TemplateLeftDelim) + require.Equal(t, "]]", actual.TemplateRightDelim) + require.Contains(t, actual.Args, tc.expected) + require.Contains(t, actual.Args, "--web.listen-address=0.0.0.0:[[ .listen_port ]]") + require.NotContains(t, strings.Join(actual.Args, " "), "{{") + }) + } + }) + + t.Run("ArgumentsAreDeterministic", func(t *testing.T) { + t.Parallel() + + fixture := exporterFixture{tls: true, skipVerify: true, valkey: allCertificates} + + first := valkeyExporterConfig(node, service, newExporter(fixture), redactSecrets, pmmAgentVersion) + for range 10 { + require.Equal(t, first.Args, valkeyExporterConfig(node, service, newExporter(fixture), redactSecrets, pmmAgentVersion).Args) + } + }) + }) }