diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 2bf15063c57..e774bb690f7 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -202,6 +202,16 @@ func NewDefaultServerConfig() *ServerConfig { } } +func (gcs *ClientConfig) Validate() error { + if gcs.BalancerName != "" { + if balancer.Get(gcs.BalancerName) == nil { + return fmt.Errorf("invalid balancer_name: %s", gcs.BalancerName) + } + } + + return nil +} + // sanitizedEndpoint strips the prefix of either http:// or https:// from configgrpc.ClientConfig.Endpoint. func (gcs *ClientConfig) sanitizedEndpoint() string { switch { @@ -334,10 +344,6 @@ func (gcs *ClientConfig) getGrpcDialOptions( } if gcs.BalancerName != "" { - valid := validateBalancerName(gcs.BalancerName) - if !valid { - return nil, fmt.Errorf("invalid balancer_name: %s", gcs.BalancerName) - } opts = append(opts, grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingPolicy":"%s"}`, gcs.BalancerName))) } @@ -363,10 +369,6 @@ func (gcs *ClientConfig) getGrpcDialOptions( return opts, nil } -func validateBalancerName(balancerName string) bool { - return balancer.Get(balancerName) != nil -} - func (gss *ServerConfig) Validate() error { if gss.MaxRecvMsgSizeMiB*1024*1024 < 0 { return fmt.Errorf("invalid max_recv_msg_size_mib value, must be between 1 and %d: %d", math.MaxInt/1024/1024, gss.MaxRecvMsgSizeMiB) diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index e46c8a546d3..6f7398ceac6 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -18,7 +18,6 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" "google.golang.org/grpc" - "google.golang.org/grpc/balancer" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" @@ -92,21 +91,6 @@ func TestNewDefaultServerConfig(t *testing.T) { assert.Equal(t, expected, result) } -// testBalancerBuilder facilitates testing validateBalancerName(). -type testBalancerBuilder struct{} - -func (testBalancerBuilder) Build(balancer.ClientConn, balancer.BuildOptions) balancer.Balancer { - return nil -} - -func (testBalancerBuilder) Name() string { - return "configgrpc_balancer_test" -} - -func init() { - balancer.Register(testBalancerBuilder{}) -} - var ( componentID = component.MustNewID("component") testAuthID = component.MustNewID("testauth") @@ -333,7 +317,7 @@ func TestGrpcServerValidate(t *testing.T) { t.Run(tt.err, func(t *testing.T) { err := tt.gss.Validate() require.Error(t, err) - assert.Regexp(t, tt.err, err) + assert.ErrorContains(t, err, tt.err) }) } } @@ -390,18 +374,37 @@ func TestGrpcServerAuthSettings(t *testing.T) { assert.NotNil(t, srv) } -func TestGRPCClientSettingsError(t *testing.T) { - tt, err := componenttest.SetupTelemetry(componentID) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) +func TestGrpcClientConfigInvalidBalancer(t *testing.T) { + settings := ClientConfig{ + Headers: map[string]configopaque.String{ + "test": "test", + }, + Endpoint: "localhost:1234", + Compression: "gzip", + TLSSetting: configtls.ClientConfig{ + Insecure: false, + }, + Keepalive: &KeepaliveClientConfig{ + Time: time.Second, + Timeout: time.Second, + PermitWithoutStream: true, + }, + ReadBufferSize: 1024, + WriteBufferSize: 1024, + WaitForReady: true, + BalancerName: "test", + } + assert.ErrorContains(t, settings.Validate(), "invalid balancer_name: test") +} +func TestGRPCClientSettingsError(t *testing.T) { tests := []struct { settings ClientConfig err string host component.Host }{ { - err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", + err: "failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", settings: ClientConfig{ Headers: nil, Endpoint: "", @@ -417,7 +420,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", + 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: "", @@ -432,28 +435,6 @@ func TestGRPCClientSettingsError(t *testing.T) { Keepalive: nil, }, }, - { - err: "invalid balancer_name: test", - settings: ClientConfig{ - Headers: map[string]configopaque.String{ - "test": "test", - }, - Endpoint: "localhost:1234", - Compression: "gzip", - TLSSetting: configtls.ClientConfig{ - Insecure: false, - }, - Keepalive: &KeepaliveClientConfig{ - Time: time.Second, - Timeout: time.Second, - PermitWithoutStream: true, - }, - ReadBufferSize: 1024, - WriteBufferSize: 1024, - WaitForReady: true, - BalancerName: "test", - }, - }, { err: "failed to resolve authenticator \"doesntexist\": authenticator not found", settings: ClientConfig{ @@ -506,9 +487,10 @@ func TestGRPCClientSettingsError(t *testing.T) { } for _, test := range tests { t.Run(test.err, func(t *testing.T) { - _, err := test.settings.ToClientConn(context.Background(), test.host, tt.TelemetrySettings()) + require.NoError(t, test.settings.Validate()) + _, err := test.settings.ToClientConn(context.Background(), test.host, componenttest.NewNopTelemetrySettings()) require.Error(t, err) - assert.Regexp(t, test.err, err) + assert.ErrorContains(t, err, test.err) }) } } @@ -587,7 +569,7 @@ func TestGRPCServerSettingsError(t *testing.T) { err string }{ { - err: "^failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", + err: "failed to load TLS config: failed to load CA CertPool File: failed to load cert /doesnt/exist:", settings: ServerConfig{ NetAddr: confignet.AddrConfig{ Endpoint: "127.0.0.1:1234", @@ -601,7 +583,7 @@ func TestGRPCServerSettingsError(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", + 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: ServerConfig{ NetAddr: confignet.AddrConfig{ Endpoint: "127.0.0.1:1234", @@ -615,7 +597,7 @@ func TestGRPCServerSettingsError(t *testing.T) { }, }, { - err: "^failed to load client CA CertPool: failed to load CA /doesnt/exist:", + err: "failed to load client CA CertPool: failed to load CA /doesnt/exist:", settings: ServerConfig{ NetAddr: confignet.AddrConfig{ Endpoint: "127.0.0.1:1234", @@ -630,7 +612,7 @@ func TestGRPCServerSettingsError(t *testing.T) { for _, test := range tests { t.Run(test.err, func(t *testing.T) { _, err := test.settings.ToServer(context.Background(), componenttest.NewNopHost(), componenttest.NewNopTelemetrySettings()) - assert.Regexp(t, test.err, err) + assert.ErrorContains(t, err, test.err) }) } }