add more complete river identifier validation and sanitization (#4998)

* use river functions to validate and sanitize river identifiers

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>

---------

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
(cherry picked from commit ce3fe1c)
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>

converter/internal/promtailconvert/internal/build/scrape_builder.go

@@ -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"
 )
@@ -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() {

converter/internal/staticconvert/internal/build/blackbox_exporter.go

@@ -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 {
@@ -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,
 	}

converter/internal/staticconvert/internal/build/snmp_exporter.go

@@ -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"
 )
 
@@ -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,
@@ -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,
 		}

converter/internal/staticconvert/staticconvert.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"flag"
 	"fmt"
-	"strings"
 
 	"github.com/grafana/agent/component/discovery"
 	"github.com/grafana/agent/converter/diag"
@@ -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"
 
@@ -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
 		}
 

go.mod

@@ -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

go.sum

@@ -1953,8 +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.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=

pkg/flow/module.go

@@ -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"
 )
@@ -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)
 	}
 
@@ -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()