Skip to content

Commit 2e5bb6d

Browse files
authored
[configgrpc,exporter/otlp] Move gRPC client endpoint validation to configgrpc (#14221)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Moves validation of gRPC endpoint from OTLP exporter to configgrpc <!-- Issue number if applicable --> #### Link to tracking issue Fixes #10451
1 parent 8026972 commit 2e5bb6d

File tree

5 files changed

+79
-39
lines changed

5 files changed

+79
-39
lines changed
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/otlp)
7+
component: pkg/config/configgrpc
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Statically validate gRPC endpoint
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [10451]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
This validation was already done in the OTLP exporter. It will now be applied to any gRPC client.
20+
21+
# Optional: The change log or logs in which this entry should be included.
22+
# e.g. '[user]' or '[user, api]'
23+
# Include 'user' if the change is relevant to end users.
24+
# Include 'api' if there is a change to a library API.
25+
# Default: '[user]'
26+
change_logs: []

config/configgrpc/configgrpc.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import (
99
"errors"
1010
"fmt"
1111
"math"
12+
"net"
13+
"regexp"
14+
"strconv"
1215
"strings"
1316
"time"
1417

@@ -224,6 +227,24 @@ func NewDefaultServerConfig() ServerConfig {
224227
}
225228

226229
func (cc *ClientConfig) Validate() error {
230+
if after, ok := strings.CutPrefix(cc.Endpoint, "unix://"); ok {
231+
if after == "" {
232+
return errors.New("unix socket path cannot be empty")
233+
}
234+
return nil
235+
}
236+
237+
if endpoint := cc.sanitizedEndpoint(); endpoint != "" {
238+
// Validate that the port is in the address
239+
_, port, err := net.SplitHostPort(endpoint)
240+
if err != nil {
241+
return err
242+
}
243+
if _, err := strconv.Atoi(port); err != nil {
244+
return fmt.Errorf(`invalid port "%v"`, port)
245+
}
246+
}
247+
227248
if cc.BalancerName != "" {
228249
if balancer.Get(cc.BalancerName) == nil {
229250
return fmt.Errorf("invalid balancer_name: %s", cc.BalancerName)
@@ -240,6 +261,9 @@ func (cc *ClientConfig) sanitizedEndpoint() string {
240261
return strings.TrimPrefix(cc.Endpoint, "http://")
241262
case cc.isSchemeHTTPS():
242263
return strings.TrimPrefix(cc.Endpoint, "https://")
264+
case strings.HasPrefix(cc.Endpoint, "dns://"):
265+
r := regexp.MustCompile(`^dns:///?`)
266+
return r.ReplaceAllString(cc.Endpoint, "")
243267
default:
244268
return cc.Endpoint
245269
}

config/configgrpc/configgrpc_test.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,24 @@ func TestAllGrpcClientSettings(t *testing.T) {
263263
}
264264
}
265265

266+
func TestSanitizeEndpoint(t *testing.T) {
267+
cfg := NewDefaultClientConfig()
268+
cfg.Endpoint = "dns://authority/backend.example.com:4317"
269+
assert.Equal(t, "authority/backend.example.com:4317", cfg.sanitizedEndpoint())
270+
cfg.Endpoint = "dns:///backend.example.com:4317"
271+
assert.Equal(t, "backend.example.com:4317", cfg.sanitizedEndpoint())
272+
cfg.Endpoint = "dns:////backend.example.com:4317"
273+
assert.Equal(t, "/backend.example.com:4317", cfg.sanitizedEndpoint())
274+
}
275+
276+
func TestValidateEndpoint(t *testing.T) {
277+
cfg := NewDefaultClientConfig()
278+
cfg.Endpoint = "dns://authority/backend.example.com:4317"
279+
assert.NoError(t, cfg.Validate())
280+
cfg.Endpoint = "unix:///my/unix/socket.sock"
281+
assert.NoError(t, cfg.Validate())
282+
}
283+
266284
func TestHeaders(t *testing.T) {
267285
traceServer := &grpcTraceServer{}
268286
server, addr := traceServer.startTestServer(t, configoptional.Some(ServerConfig{
@@ -457,7 +475,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
457475
err: "failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:",
458476
settings: ClientConfig{
459477
Headers: nil,
460-
Endpoint: "",
478+
Endpoint: "localhost:1234",
461479
Compression: "",
462480
TLS: configtls.ClientConfig{
463481
Config: configtls.Config{
@@ -472,7 +490,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
472490
err: "failed to load TLS config: failed to load TLS cert and key: for auth via TLS, provide both certificate and key, or neither",
473491
settings: ClientConfig{
474492
Headers: nil,
475-
Endpoint: "",
493+
Endpoint: "localhost:1234",
476494
Compression: "",
477495
TLS: configtls.ClientConfig{
478496
Config: configtls.Config{

exporter/otlpexporter/config.go

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,14 @@ package otlpexporter // import "go.opentelemetry.io/collector/exporter/otlpexpor
55

66
import (
77
"errors"
8-
"fmt"
9-
"net"
108
"regexp"
11-
"strconv"
129
"strings"
1310

1411
"go.opentelemetry.io/collector/component"
1512
"go.opentelemetry.io/collector/config/configgrpc"
1613
"go.opentelemetry.io/collector/config/configoptional"
1714
"go.opentelemetry.io/collector/config/configretry"
15+
"go.opentelemetry.io/collector/confmap/xconfmap"
1816
"go.opentelemetry.io/collector/exporter/exporterhelper"
1917
)
2018

@@ -29,28 +27,15 @@ type Config struct {
2927
_ struct{}
3028
}
3129

32-
func (c *Config) Validate() error {
33-
if after, ok := strings.CutPrefix(c.ClientConfig.Endpoint, "unix://"); ok {
34-
if after == "" {
35-
return errors.New("unix socket path cannot be empty")
36-
}
37-
return nil
38-
}
30+
var (
31+
_ component.Config = (*Config)(nil)
32+
_ xconfmap.Validator = (*Config)(nil)
33+
)
3934

40-
endpoint := c.sanitizedEndpoint()
41-
if endpoint == "" {
35+
func (c *Config) Validate() error {
36+
if endpoint := c.sanitizedEndpoint(); endpoint == "" {
4237
return errors.New(`requires a non-empty "endpoint"`)
4338
}
44-
45-
// Validate that the port is in the address
46-
_, port, err := net.SplitHostPort(endpoint)
47-
if err != nil {
48-
return err
49-
}
50-
if _, err := strconv.Atoi(port); err != nil {
51-
return fmt.Errorf(`invalid port "%s"`, port)
52-
}
53-
5439
return nil
5540
}
5641

@@ -67,5 +52,3 @@ func (c *Config) sanitizedEndpoint() string {
6752
return c.ClientConfig.Endpoint
6853
}
6954
}
70-
71-
var _ component.Config = (*Config)(nil)

exporter/otlpexporter/config_test.go

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -178,23 +178,12 @@ func TestValidDNSEndpoint(t *testing.T) {
178178
factory := NewFactory()
179179
cfg := factory.CreateDefaultConfig().(*Config)
180180
cfg.ClientConfig.Endpoint = "dns://authority/backend.example.com:4317"
181-
assert.NoError(t, cfg.Validate())
181+
assert.NoError(t, xconfmap.Validate(cfg))
182182
}
183183

184184
func TestValidUnixSocketEndpoint(t *testing.T) {
185185
factory := NewFactory()
186186
cfg := factory.CreateDefaultConfig().(*Config)
187187
cfg.ClientConfig.Endpoint = "unix:///my/unix/socket.sock"
188-
assert.NoError(t, cfg.Validate())
189-
}
190-
191-
func TestSanitizeEndpoint(t *testing.T) {
192-
factory := NewFactory()
193-
cfg := factory.CreateDefaultConfig().(*Config)
194-
cfg.ClientConfig.Endpoint = "dns://authority/backend.example.com:4317"
195-
assert.Equal(t, "authority/backend.example.com:4317", cfg.sanitizedEndpoint())
196-
cfg.ClientConfig.Endpoint = "dns:///backend.example.com:4317"
197-
assert.Equal(t, "backend.example.com:4317", cfg.sanitizedEndpoint())
198-
cfg.ClientConfig.Endpoint = "dns:////backend.example.com:4317"
199-
assert.Equal(t, "/backend.example.com:4317", cfg.sanitizedEndpoint())
188+
assert.NoError(t, xconfmap.Validate(cfg))
200189
}

0 commit comments

Comments
 (0)