[configgrpc,exporter/otlp] Move gRPC client endpoint validation to configgrpc

.chloggen/mx-psi_configgrpc-validation.yaml

@@ -0,0 +1,26 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. receiver/otlp)
+component: pkg/config/configgrpc
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Statically validate gRPC endpoint 
+
+# One or more tracking issues or pull requests related to the change
+issues: [10451]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  This validation was already done in the OTLP exporter. It will now be applied to any gRPC client.
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

config/configgrpc/configgrpc.go

@@ -9,6 +9,9 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"net"
+	"regexp"
+	"strconv"
 	"strings"
 	"time"
 
@@ -224,6 +227,27 @@ func NewDefaultServerConfig() ServerConfig {
 }
 
 func (cc *ClientConfig) Validate() error {
+	if after, ok := strings.CutPrefix(cc.Endpoint, "unix://"); ok {
+		if after == "" {
+			return errors.New("unix socket path cannot be empty")
+		}
+		return nil
+	}
+
+	endpoint := cc.sanitizedEndpoint()
+	if endpoint == "" {
+		return errors.New(`requires a non-empty "endpoint"`)
+	}
+
+	// Validate that the port is in the address
+	_, port, err := net.SplitHostPort(endpoint)
+	if err != nil {
+		return err
+	}
+	if _, err := strconv.Atoi(port); err != nil {
+		return fmt.Errorf(`invalid port "%v"`, port)
+	}
+
 	if cc.BalancerName != "" {
 		if balancer.Get(cc.BalancerName) == nil {
 			return fmt.Errorf("invalid balancer_name: %s", cc.BalancerName)
@@ -240,6 +264,9 @@ func (cc *ClientConfig) sanitizedEndpoint() string {
 		return strings.TrimPrefix(cc.Endpoint, "http://")
 	case cc.isSchemeHTTPS():
 		return strings.TrimPrefix(cc.Endpoint, "https://")
+	case strings.HasPrefix(cc.Endpoint, "dns://"):
+		r := regexp.MustCompile(`^dns:///?`)
+		return r.ReplaceAllString(cc.Endpoint, "")
 	default:
 		return cc.Endpoint
 	}

config/configgrpc/configgrpc_test.go

@@ -269,6 +269,24 @@ func TestAllGrpcClientSettings(t *testing.T) {
 	}
 }
 
+func TestSanitizeEndpoint(t *testing.T) {
+	cfg := NewDefaultClientConfig()
+	cfg.Endpoint = "dns://authority/backend.example.com:4317"
+	assert.Equal(t, "authority/backend.example.com:4317", cfg.sanitizedEndpoint())
+	cfg.Endpoint = "dns:///backend.example.com:4317"
+	assert.Equal(t, "backend.example.com:4317", cfg.sanitizedEndpoint())
+	cfg.Endpoint = "dns:////backend.example.com:4317"
+	assert.Equal(t, "/backend.example.com:4317", cfg.sanitizedEndpoint())
+}
+
+func TestValidateEndpoint(t *testing.T) {
+	cfg := NewDefaultClientConfig()
+	cfg.Endpoint = "dns://authority/backend.example.com:4317"
+	assert.NoError(t, cfg.Validate())
+	cfg.Endpoint = "unix:///my/unix/socket.sock"
+	assert.NoError(t, cfg.Validate())
+}
+
 func TestHeaders(t *testing.T) {
 	traceServer := &grpcTraceServer{}
 	server, addr := traceServer.startTestServer(t, configoptional.Some(ServerConfig{
@@ -465,7 +483,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 			err: "failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:",
 			settings: ClientConfig{
 				Headers:     nil,
-				Endpoint:    "",
+				Endpoint:    "localhost:1234",
 				Compression: "",
 				TLS: configtls.ClientConfig{
 					Config: configtls.Config{
@@ -480,7 +498,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 			err: "failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither",
 			settings: ClientConfig{
 				Headers:     nil,
-				Endpoint:    "",
+				Endpoint:    "localhost:1234",
 				Compression: "",
 				TLS: configtls.ClientConfig{
 					Config: configtls.Config{

exporter/otlpexporter/config.go

@@ -4,16 +4,10 @@
 package otlpexporter // import "go.opentelemetry.io/collector/exporter/otlpexporter"
 
 import (
-	"errors"
-	"fmt"
-	"net"
-	"regexp"
-	"strconv"
-	"strings"
-
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configgrpc"
 	"go.opentelemetry.io/collector/config/configretry"
+	"go.opentelemetry.io/collector/confmap/xconfmap"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
 )
 
@@ -28,43 +22,11 @@ type Config struct {
 	_ struct{}
 }
 
-func (c *Config) Validate() error {
-	if after, ok := strings.CutPrefix(c.ClientConfig.Endpoint, "unix://"); ok {
-		if after == "" {
-			return errors.New("unix socket path cannot be empty")
-		}
-		return nil
-	}
-
-	endpoint := c.sanitizedEndpoint()
-	if endpoint == "" {
-		return errors.New(`requires a non-empty "endpoint"`)
-	}
-
-	// Validate that the port is in the address
-	_, port, err := net.SplitHostPort(endpoint)
-	if err != nil {
-		return err
-	}
-	if _, err := strconv.Atoi(port); err != nil {
-		return fmt.Errorf(`invalid port "%s"`, port)
-	}
+var (
+	_ component.Config   = (*Config)(nil)
+	_ xconfmap.Validator = (*Config)(nil)
+)
 
+func (c *Config) Validate() error {
 	return nil
 }
-
-func (c *Config) sanitizedEndpoint() string {
-	switch {
-	case strings.HasPrefix(c.ClientConfig.Endpoint, "http://"):
-		return strings.TrimPrefix(c.ClientConfig.Endpoint, "http://")
-	case strings.HasPrefix(c.ClientConfig.Endpoint, "https://"):
-		return strings.TrimPrefix(c.ClientConfig.Endpoint, "https://")
-	case strings.HasPrefix(c.ClientConfig.Endpoint, "dns://"):
-		r := regexp.MustCompile(`^dns:///?`)
-		return r.ReplaceAllString(c.ClientConfig.Endpoint, "")
-	default:
-		return c.ClientConfig.Endpoint
-	}
-}
-
-var _ component.Config = (*Config)(nil)

exporter/otlpexporter/config_test.go

@@ -180,23 +180,12 @@ func TestValidDNSEndpoint(t *testing.T) {
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.ClientConfig.Endpoint = "dns://authority/backend.example.com:4317"
-	assert.NoError(t, cfg.Validate())
+	assert.NoError(t, xconfmap.Validate(cfg))
 }
 
 func TestValidUnixSocketEndpoint(t *testing.T) {
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.ClientConfig.Endpoint = "unix:///my/unix/socket.sock"
-	assert.NoError(t, cfg.Validate())
-}
-
-func TestSanitizeEndpoint(t *testing.T) {
-	factory := NewFactory()
-	cfg := factory.CreateDefaultConfig().(*Config)
-	cfg.ClientConfig.Endpoint = "dns://authority/backend.example.com:4317"
-	assert.Equal(t, "authority/backend.example.com:4317", cfg.sanitizedEndpoint())
-	cfg.ClientConfig.Endpoint = "dns:///backend.example.com:4317"
-	assert.Equal(t, "backend.example.com:4317", cfg.sanitizedEndpoint())
-	cfg.ClientConfig.Endpoint = "dns:////backend.example.com:4317"
-	assert.Equal(t, "/backend.example.com:4317", cfg.sanitizedEndpoint())
+	assert.NoError(t, xconfmap.Validate(cfg))
 }