Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add more complete river identifier validation and sanitization #4998

Merged
merged 3 commits into from
Aug 30, 2023
Merged

add more complete river identifier validation and sanitization #4998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e0aeae
add more complete river identifier validation and sanitization
erikbaranowski Aug 29, 2023
d2c493b
allow start with underscore
erikbaranowski Aug 29, 2023
6858e32
use river functions to validate and sanitize river identifiers
erikbaranowski Aug 30, 2023
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
8 changes: 6 additions & 2 deletions converter/internal/promtailconvert/internal/build/scrape_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/grafana/agent/converter/internal/common"
"github.com/grafana/agent/converter/internal/prometheusconvert"
"github.com/grafana/loki/clients/pkg/promtail/scrapeconfig"
"github.com/grafana/river/scanner"
"github.com/grafana/river/token/builder"
"github.com/prometheus/common/model"
)
Expand Down Expand Up @@ -73,8 +74,11 @@ func (s *ScrapeConfigBuilder) Validate() {
}

func (s *ScrapeConfigBuilder) Sanitize() {
s.cfg.JobName = strings.ReplaceAll(s.cfg.JobName, "-", "_")
s.cfg.JobName = strings.ReplaceAll(s.cfg.JobName, "/", "_")
var err error
s.cfg.JobName, err = scanner.SanitizeIdentifier(s.cfg.JobName)
if err != nil {
s.diags.Add(diag.SeverityLevelCritical, fmt.Sprintf("failed to sanitize job name: %s", err))
}
}

func (s *ScrapeConfigBuilder) AppendLokiSourceFile() {
Expand Down
8 changes: 7 additions & 1 deletion converter/internal/staticconvert/internal/build/blackbox_exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/grafana/agent/converter/internal/prometheusconvert"
"github.com/grafana/agent/pkg/integrations/blackbox_exporter"
"github.com/grafana/river/rivertypes"
"github.com/grafana/river/scanner"
)

func (b *IntegrationsV1ConfigBuilder) appendBlackboxExporter(config *blackbox_exporter.Config) discovery.Exports {
Expand Down Expand Up @@ -45,8 +46,13 @@ func toBlackboxTargets(blackboxTargets []blackbox_exporter.BlackboxTarget) black
}

func toBlackboxTarget(target blackbox_exporter.BlackboxTarget) blackbox.BlackboxTarget {
sanitizedName, err := scanner.SanitizeIdentifier(target.Name)
if err != nil {
panic(err)
}

return blackbox.BlackboxTarget{
Name: target.Name,
Name: sanitizedName,
Target: target.Target,
Module: target.Module,
}
Expand Down
20 changes: 17 additions & 3 deletions converter/internal/staticconvert/internal/build/snmp_exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/grafana/agent/converter/internal/prometheusconvert"
"github.com/grafana/agent/pkg/integrations/snmp_exporter"
"github.com/grafana/river/rivertypes"
"github.com/grafana/river/scanner"
snmp_config "github.com/prometheus/snmp_exporter/config"
)

Expand All @@ -27,8 +28,12 @@ func (b *IntegrationsV1ConfigBuilder) appendSnmpExporter(config *snmp_exporter.C
func toSnmpExporter(config *snmp_exporter.Config) *snmp.Arguments {
targets := make([]snmp.SNMPTarget, len(config.SnmpTargets))
for i, t := range config.SnmpTargets {
sanitizedName, err := scanner.SanitizeIdentifier(t.Name)
if err != nil {
panic(err)
}
targets[i] = snmp.SNMPTarget{
Name: t.Name,
Name: sanitizedName,
Target: t.Target,
Module: t.Module,
Auth: t.Auth,
Expand All @@ -39,10 +44,19 @@ func toSnmpExporter(config *snmp_exporter.Config) *snmp.Arguments {
walkParams := make([]snmp.WalkParam, len(config.WalkParams))
index := 0
for name, p := range config.WalkParams {
retries := 0
if p.Retries != nil {
retries = *p.Retries
}

sanitizedName, err := scanner.SanitizeIdentifier(name)
if err != nil {
panic(err)
}
walkParams[index] = snmp.WalkParam{
Name: name,
Name: sanitizedName,
MaxRepetitions: p.MaxRepetitions,
Retries: *p.Retries,
Retries: retries,
Timeout: p.Timeout,
UseUnconnectedUDPSocket: p.UseUnconnectedUDPSocket,
}
Expand Down
15 changes: 9 additions & 6 deletions converter/internal/staticconvert/staticconvert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"flag"
"fmt"
"strings"

"github.com/grafana/agent/component/discovery"
"github.com/grafana/agent/converter/diag"
Expand All @@ -17,6 +16,7 @@ import (
promtail_config "github.com/grafana/loki/clients/pkg/promtail/config"
"github.com/grafana/loki/clients/pkg/promtail/limit"
"github.com/grafana/loki/clients/pkg/promtail/targets/file"
"github.com/grafana/river/scanner"
"github.com/grafana/river/token/builder"
prom_config "github.com/prometheus/prometheus/config"

Expand Down Expand Up @@ -84,13 +84,16 @@ func appendStaticPrometheus(f *builder.File, staticConfig *config.Config) diag.D
}

jobNameToCompLabelsFunc := func(jobName string) string {
if jobName == "" {
return fmt.Sprintf("metrics_%s", instance.Name)
name := fmt.Sprintf("metrics_%s", instance.Name)
if jobName != "" {
name += fmt.Sprintf("_%s", jobName)
}

name, err := scanner.SanitizeIdentifier(name)
if err != nil {
diags.Add(diag.SeverityLevelCritical, fmt.Sprintf("failed to sanitize job name: %s", err))
}

name := fmt.Sprintf("metrics_%s_%s", instance.Name, jobName)
name = strings.ReplaceAll(name, "-", "_")
name = strings.ReplaceAll(name, "/", "_")
return name
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ require (
github.com/grafana/pyroscope/api v0.2.0
github.com/grafana/pyroscope/ebpf v0.2.1
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db
github.com/grafana/river v0.1.1
github.com/grafana/river v0.1.2-0.20230830200459-0ff21cf610eb
github.com/grafana/snowflake-prometheus-exporter v0.0.0-20221213150626-862cad8e9538
github.com/grafana/tail v0.0.0-20230510142333-77b18831edf0
github.com/grafana/vmware_exporter v0.0.4-beta
Expand Down
8 changes: 2 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1953,12 +1953,8 @@ github.com/grafana/pyroscope/ebpf v0.2.1/go.mod h1:KTvAJ+C8PFW2C0aI0KzSUhsqslQCc
github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A=
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db h1:7aN5cccjIqCLTzedH7MZzRZt5/lsAHch6Z3L2ZGn5FA=
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A=
github.com/grafana/river v0.0.0-20230825181326-a26c2d2f0e95 h1:FF6tydxeVmbzlqmk4ab+pfUiZrMpgYD+Ei3yLnHGtpo=
github.com/grafana/river v0.0.0-20230825181326-a26c2d2f0e95/go.mod h1:F8rcwfPL98xF/m+OSCFH7gJyLPYMIadXCVfY+/OcqEI=
github.com/grafana/river v0.0.0-20230825183552-da1ac6d738c3 h1:ZWV8aOLW/dGK0A/RKT0N8B5AOtGcxQHKHXRHFt2Kkaw=
github.com/grafana/river v0.0.0-20230825183552-da1ac6d738c3/go.mod h1:F8rcwfPL98xF/m+OSCFH7gJyLPYMIadXCVfY+/OcqEI=
github.com/grafana/river v0.1.1 h1:2Lt/j293FFJzwwW//VJSl+fzbyV8oPMzFbrTZFKAbRo=
github.com/grafana/river v0.1.1/go.mod h1:F8rcwfPL98xF/m+OSCFH7gJyLPYMIadXCVfY+/OcqEI=
github.com/grafana/river v0.1.2-0.20230830200459-0ff21cf610eb h1:hOblg36rOTgGIOp7A3+53OtlXqq0iNnI9qDcOn7fPfQ=
github.com/grafana/river v0.1.2-0.20230830200459-0ff21cf610eb/go.mod h1:F8rcwfPL98xF/m+OSCFH7gJyLPYMIadXCVfY+/OcqEI=
github.com/grafana/smimesign v0.2.1-0.20220408144937-2a5adf3481d3 h1:UPkAxuhlAcRmJT3/qd34OMTl+ZU7BLLfOO2+NXBlJpY=
github.com/grafana/smimesign v0.2.1-0.20220408144937-2a5adf3481d3/go.mod h1:iZiiwNT4HbtGRVqCQu7uJPEZCuEE5sfSSttcnePkDl4=
github.com/grafana/snowflake-prometheus-exporter v0.0.0-20221213150626-862cad8e9538 h1:tkT0yha3JzB5S5VNjfY4lT0cJAe20pU8XGt3Nuq73rM=
Expand Down
9 changes: 1 addition & 8 deletions pkg/flow/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/grafana/agent/pkg/flow/logging"
"github.com/grafana/agent/pkg/flow/tracing"
"github.com/grafana/river/scanner"
"github.com/grafana/river/token"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/exp/maps"
)
Expand All @@ -36,7 +35,7 @@ func newModuleController(o *moduleControllerOptions) controller.ModuleController

// NewModule creates a new, unstarted Module.
func (m *moduleController) NewModule(id string, export component.ExportFunc) (component.Module, error) {
if id != "" && !isValidIdentifier(id) {
if id != "" && !scanner.IsValidIdentifier(id) {
return nil, fmt.Errorf("module ID %q is not a valid River identifier", id)
}

Expand Down Expand Up @@ -65,12 +64,6 @@ func (m *moduleController) NewModule(id string, export component.ExportFunc) (co
return mod, nil
}

func isValidIdentifier(in string) bool {
s := scanner.New(nil, []byte(in), nil, 0)
_, tok, lit := s.Scan()
return tok == token.IDENT && lit == in
}

func (m *moduleController) removeID(id string) {
m.mut.Lock()
defer m.mut.Unlock()
Expand Down