Skip to content

Commit

Permalink
Do not embed ProtocolSettings in gRPC
Browse files Browse the repository at this point in the history 
Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
@bogdandrutu
bogdandrutu committed Jun 26, 2020
1 parent 3d5af8c commit 1c473fa
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 98 deletions.
14 changes: 9 additions & 5 deletions config/configgrpc/configgrpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ import (
"strings"
"time"

"github.com/pkg/errors"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/encoding/gzip"
"google.golang.org/grpc/keepalive"

"go.opentelemetry.io/collector/config/configprotocol"
"go.opentelemetry.io/collector/config/configtls"
)

Expand Down Expand Up @@ -103,8 +101,14 @@ type KeepaliveEnforcementPolicy struct {
}

type GRPCServerSettings struct {
// Configures the generic server protocol.
configprotocol.ProtocolServerSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
// The target to which the exporter is going to send traces or metrics,
// using the gRPC protocol. The valid syntax is described at
// https://github.com/grpc/grpc/blob/master/doc/naming.md.
Endpoint string `mapstructure:"endpoint"`

// Configures the protocol to use TLS.
// The default value is nil, which will cause the protocol to not use TLS.
TLSCredentials *configtls.TLSServerSetting `mapstructure:"tls_credentials, omitempty"`

// MaxRecvMsgSizeMiB sets the maximum size (in MiB) of messages accepted by the server.
MaxRecvMsgSizeMiB uint64 `mapstructure:"max_recv_msg_size_mib,omitempty"`
Expand Down Expand Up @@ -158,7 +162,7 @@ func (gss *GRPCServerSettings) ToServerOption() ([]grpc.ServerOption, error) {
if gss.TLSCredentials != nil {
tlsCfg, err := gss.TLSCredentials.LoadTLSConfig()
if err != nil {
return nil, errors.Wrap(err, "error initializing TLS Credentials")
return nil, err
}
opts = append(opts, grpc.Creds(credentials.NewTLS(tlsCfg)))
}
Expand Down
49 changes: 48 additions & 1 deletion config/configgrpc/configgrpc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestBasicGrpcSettings(t *testing.T) {
assert.NoError(t, err)
}

func TestInvalidPemFile(t *testing.T) {
func TestGRPCClientSettingsError(t *testing.T) {
tests := []struct {
settings GRPCClientSettings
err string
Expand Down Expand Up @@ -93,6 +93,53 @@ func TestUseSecure(t *testing.T) {
assert.Equal(t, len(dialOpts), 1)
}

func TestGRPCServerSettingsError(t *testing.T) {
tests := []struct {
settings GRPCServerSettings
err string
}{
{
err: "^failed to load TLS config: failed to load CA CertPool: failed to load CA /doesnt/exist:",
settings: GRPCServerSettings{
Endpoint: "",
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CAFile: "/doesnt/exist",
},
},
Keepalive: nil,
},
},
{
err: "^failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither",
settings: GRPCServerSettings{
Endpoint: "",
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "/doesnt/exist",
},
},
Keepalive: nil,
},
},
{
err: "^failed to load TLS config: failed to load client CA CertPool: failed to load CA /doesnt/exist:",
settings: GRPCServerSettings{
Endpoint: "",
TLSCredentials: &configtls.TLSServerSetting{
ClientCAFile: "/doesnt/exist",
},
},
},
}
for _, test := range tests {
t.Run(test.err, func(t *testing.T) {
_, err := test.settings.ToServerOption()
assert.Regexp(t, test.err, err)
})
}
}

func TestGetGRPCCompressionKey(t *testing.T) {
if GetGRPCCompressionKey("gzip") != CompressionGzip {
t.Error("gzip is marked as supported but returned unsupported")
Expand Down
44 changes: 13 additions & 31 deletions receiver/opencensusreceiver/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/config/configprotocol"
"go.opentelemetry.io/collector/config/configtls"
)

Expand Down Expand Up @@ -53,10 +52,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "opencensus/customname",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:9090",
TLSCredentials: nil,
},
Endpoint: "0.0.0.0:9090",
},
Transport: "tcp",
})
Expand All @@ -69,10 +65,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "opencensus/keepalive",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
TLSCredentials: nil,
Endpoint: "0.0.0.0:55678",
},
Endpoint: "0.0.0.0:55678",
Keepalive: &configgrpc.KeepaliveServerConfig{
ServerParameters: &configgrpc.KeepaliveServerParameters{
MaxConnectionIdle: 11 * time.Second,
Expand All @@ -98,10 +91,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "opencensus/msg-size-conc-connect-max-idle",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55678",
TLSCredentials: nil,
},
Endpoint: "0.0.0.0:55678",
MaxRecvMsgSizeMiB: 32,
MaxConcurrentStreams: 16,
Keepalive: &configgrpc.KeepaliveServerConfig{
Expand All @@ -123,13 +113,11 @@ func TestLoadConfig(t *testing.T) {
NameVal: "opencensus/tlscredentials",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55678",
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "test.crt",
KeyFile: "test.key",
},
Endpoint: "0.0.0.0:55678",
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "test.crt",
KeyFile: "test.key",
},
},
},
Expand All @@ -144,9 +132,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "opencensus/cors",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55678",
},
Endpoint: "0.0.0.0:55678",
},
Transport: "tcp",
CorsOrigins: []string{"https://*.test.com", "https://test.com"},
Expand All @@ -160,9 +146,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "opencensus/uds",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "/tmp/opencensus.sock",
},
Endpoint: "/tmp/opencensus.sock",
},
Transport: "unix",
})
Expand All @@ -174,11 +158,9 @@ func TestBuildOptions_TLSCredentials(t *testing.T) {
NameVal: "IncorrectTLS",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "willfail",
},
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "willfail",
},
},
},
Expand Down
5 changes: 1 addition & 4 deletions receiver/opencensusreceiver/factory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/config/configprotocol"
"go.opentelemetry.io/collector/consumer"
)

Expand Down Expand Up @@ -53,9 +52,7 @@ func (f *Factory) CreateDefaultConfig() configmodels.Receiver {
NameVal: typeStr,
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55678",
},
Endpoint: "0.0.0.0:55678",
},
Transport: "tcp",
}
Expand Down
50 changes: 16 additions & 34 deletions receiver/otlpreceiver/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/config/configprotocol"
"go.opentelemetry.io/collector/config/configtls"
)

Expand Down Expand Up @@ -53,10 +52,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "otlp/customname",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "localhost:9090",
TLSCredentials: nil,
},
Endpoint: "localhost:9090",
},
Transport: "tcp",
})
Expand All @@ -69,10 +65,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "otlp/keepalive",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55680",
TLSCredentials: nil,
},
Endpoint: "0.0.0.0:55680",
Keepalive: &configgrpc.KeepaliveServerConfig{
ServerParameters: &configgrpc.KeepaliveServerParameters{
MaxConnectionIdle: 11 * time.Second,
Expand All @@ -98,10 +91,7 @@ func TestLoadConfig(t *testing.T) {
NameVal: "otlp/msg-size-conc-connect-max-idle",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55680",
TLSCredentials: nil,
},
Endpoint: "0.0.0.0:55680",
MaxRecvMsgSizeMiB: 32,
MaxConcurrentStreams: 16,
Keepalive: &configgrpc.KeepaliveServerConfig{
Expand All @@ -123,13 +113,11 @@ func TestLoadConfig(t *testing.T) {
NameVal: "otlp/tlscredentials",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55680",
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "test.crt",
KeyFile: "test.key",
},
Endpoint: "0.0.0.0:55680",
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "test.crt",
KeyFile: "test.key",
},
},
},
Expand All @@ -144,10 +132,8 @@ func TestLoadConfig(t *testing.T) {
NameVal: "otlp/cors",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55680",
TLSCredentials: nil,
},
Endpoint: "0.0.0.0:55680",
TLSCredentials: nil,
},
Transport: "tcp",
CorsOrigins: []string{"https://*.test.com", "https://test.com"},
Expand All @@ -161,10 +147,8 @@ func TestLoadConfig(t *testing.T) {
NameVal: "otlp/uds",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "/tmp/otlp.sock",
TLSCredentials: nil,
},
Endpoint: "/tmp/otlp.sock",
TLSCredentials: nil,
},
Transport: "unix",
})
Expand All @@ -176,17 +160,15 @@ func TestBuildOptions_TLSCredentials(t *testing.T) {
NameVal: "IncorrectTLS",
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "willfail",
},
TLSCredentials: &configtls.TLSServerSetting{
TLSSetting: configtls.TLSSetting{
CertFile: "willfail",
},
},
},
}
_, err := cfg.buildOptions()
assert.EqualError(t, err, `error initializing TLS Credentials: failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither`)
assert.EqualError(t, err, `failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither`)

cfg.TLSCredentials = &configtls.TLSServerSetting{}
opt, err := cfg.buildOptions()
Expand Down
5 changes: 1 addition & 4 deletions receiver/otlpreceiver/factory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/config/configprotocol"
"go.opentelemetry.io/collector/consumer"
)

Expand Down Expand Up @@ -51,9 +50,7 @@ func (f *Factory) CreateDefaultConfig() configmodels.Receiver {
NameVal: typeStr,
},
GRPCServerSettings: configgrpc.GRPCServerSettings{
ProtocolServerSettings: configprotocol.ProtocolServerSettings{
Endpoint: "0.0.0.0:55680",
},
Endpoint: "0.0.0.0:55680",
},
Transport: "tcp",
}
Expand Down
Loading

0 comments on commit 1c473fa

Please sign in to comment.