Use host root CA set by default for OTLP exporter (#2432)

* Use host CA set by default for otlptrace * Remove test for invalid default certs * Default to host CA for otlpmetric * Add changes to changelog * Update CHANGELOG.md Co-authored-by: Anthony Mirabella <a9@aneurysm9.com> Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
open-telemetry · Dec 9, 2021 · b177541 · b177541
1 parent 2b7c650
commit b177541
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Changed the project minimum supported Go version from 1.15 to 1.16. (#2412)
 - The `"go.opentelemetry.io/otel/exporter/otel/otlpmetric/otlpmetricgrpc".Client` now uses the underlying gRPC `ClientConn` to handle name resolution, TCP connection establishment (with retries and backoff) and TLS handshakes, and handling errors on established connections by re-resolving the name and reconnecting. (#2425)
 - The `"go.opentelemetry.io/otel/exporter/otel/otlpmetric/otlpmetricgrpc".RetrySettings` type is renamed to `RetryConfig`. (#2425)
+- The `go.opentelemetry.io/otel/exporter/otel/*` gRPC exporters now default to using the host's root CA set if none are provided by the user and `WithInsecure` is not specified. (#1584, #2432)
 
 ### Deprecated
 

diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go b/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go
@@ -28,24 +28,21 @@ import (
 	"go.opentelemetry.io/otel"
 )
 
-var httpSchemeRegexp = regexp.MustCompile(`(?i)^http://|https://`)
+var (
+	httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`)
 
-func ApplyGRPCEnvConfigs(cfg *Config) {
-	e := EnvOptionsReader{
+	DefaultEnvOptionsReader = EnvOptionsReader{
 		GetEnv:   os.Getenv,
 		ReadFile: ioutil.ReadFile,
 	}
+)
 
-	e.ApplyGRPCEnvConfigs(cfg)
+func ApplyGRPCEnvConfigs(cfg *Config) {
+	DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg)
 }
 
 func ApplyHTTPEnvConfigs(cfg *Config) {
-	e := EnvOptionsReader{
-		GetEnv:   os.Getenv,
-		ReadFile: ioutil.ReadFile,
-	}
-
-	e.ApplyHTTPEnvConfigs(cfg)
+	DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg)
 }
 
 type EnvOptionsReader struct {

diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options.go
@@ -97,10 +97,16 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
 	if cfg.ServiceConfig != "" {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
 	}
+	// Priroritize GRPCCredentials over Insecure (passing both is an error).
 	if cfg.Metrics.GRPCCredentials != nil {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials))
 	} else if cfg.Metrics.Insecure {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithInsecure())
+	} else {
+		// Default to using the host's root CA.
+		creds := credentials.NewTLS(nil)
+		cfg.Metrics.GRPCCredentials = creds
+		cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
 	}
 	if cfg.Metrics.Compression == GzipCompression {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))

diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go
@@ -189,6 +189,16 @@ func TestConfigs(t *testing.T) {
 		},
 
 		// Certificate tests
+		{
+			name: "Test Default Certificate",
+			asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
+				if grpcOption {
+					assert.NotNil(t, c.Metrics.GRPCCredentials)
+				} else {
+					assert.Nil(t, c.Metrics.TLSCfg)
+				}
+			},
+		},
 		{
 			name: "Test With Certificate",
 			opts: []otlpconfig.GenericOption{
@@ -380,27 +390,32 @@ func TestConfigs(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-
-			e := otlpconfig.EnvOptionsReader{
+			origEOR := otlpconfig.DefaultEnvOptionsReader
+			otlpconfig.DefaultEnvOptionsReader = otlpconfig.EnvOptionsReader{
 				GetEnv:   tt.env.getEnv,
 				ReadFile: tt.fileReader.readFile,
 			}
+			t.Cleanup(func() { otlpconfig.DefaultEnvOptionsReader = origEOR })
 
 			// Tests Generic options as HTTP Options
 			cfg := otlpconfig.NewDefaultConfig()
-			e.ApplyHTTPEnvConfigs(&cfg)
+			otlpconfig.ApplyHTTPEnvConfigs(&cfg)
 			for _, opt := range tt.opts {
 				opt.ApplyHTTPOption(&cfg)
 			}
 			tt.asserts(t, &cfg, false)
 
 			// Tests Generic options as gRPC Options
-			cfg = otlpconfig.NewDefaultConfig()
-			e.ApplyGRPCEnvConfigs(&cfg)
-			for _, opt := range tt.opts {
-				opt.ApplyGRPCOption(&cfg)
-			}
+			cfg = otlpconfig.NewGRPCConfig(asGRPCOptions(tt.opts)...)
 			tt.asserts(t, &cfg, true)
 		})
 	}
 }
+
+func asGRPCOptions(opts []otlpconfig.GenericOption) []otlpconfig.GRPCOption {
+	converted := make([]otlpconfig.GRPCOption, len(opts))
+	for i, o := range opts {
+		converted[i] = otlpconfig.NewGRPCOption(o.ApplyGRPCOption)
+	}
+	return converted
+}
diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go
@@ -280,18 +280,6 @@ func TestNewExporter_WithTimeout(t *testing.T) {
 	}
 }
 
-func TestStartErrorInvalidSecurityConfiguration(t *testing.T) {
-	mc := runMockCollector(t)
-	defer func() {
-		_ = mc.stop()
-	}()
-
-	client := otlpmetricgrpc.NewClient(otlpmetricgrpc.WithEndpoint(mc.endpoint))
-	err := client.Start(context.Background())
-	// https://github.com/grpc/grpc-go/blob/a671967dfbaab779d37fd7e597d9248f13806087/clientconn.go#L82
-	assert.EqualError(t, err, "grpc: no transport security set (use grpc.WithInsecure() explicitly or set credentials)")
-}
-
 func TestStartErrorInvalidAddress(t *testing.T) {
 	client := otlpmetricgrpc.NewClient(
 		otlpmetricgrpc.WithInsecure(),

diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go
@@ -28,24 +28,21 @@ import (
 	"go.opentelemetry.io/otel"
 )
 
-var httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`)
+var (
+	httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`)
 
-func ApplyGRPCEnvConfigs(cfg *Config) {
-	e := EnvOptionsReader{
+	DefaultEnvOptionsReader = EnvOptionsReader{
 		GetEnv:   os.Getenv,
 		ReadFile: ioutil.ReadFile,
 	}
+)
 
-	e.ApplyGRPCEnvConfigs(cfg)
+func ApplyGRPCEnvConfigs(cfg *Config) {
+	DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg)
 }
 
 func ApplyHTTPEnvConfigs(cfg *Config) {
-	e := EnvOptionsReader{
-		GetEnv:   os.Getenv,
-		ReadFile: ioutil.ReadFile,
-	}
-
-	e.ApplyHTTPEnvConfigs(cfg)
+	DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg)
 }
 
 type EnvOptionsReader struct {

diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options.go b/exporters/otlp/otlptrace/internal/otlpconfig/options.go
@@ -90,10 +90,16 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
 	if cfg.ServiceConfig != "" {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
 	}
+	// Priroritize GRPCCredentials over Insecure (passing both is an error).
 	if cfg.Traces.GRPCCredentials != nil {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials))
 	} else if cfg.Traces.Insecure {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithInsecure())
+	} else {
+		// Default to using the host's root CA.
+		creds := credentials.NewTLS(nil)
+		cfg.Traces.GRPCCredentials = creds
+		cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
 	}
 	if cfg.Traces.Compression == GzipCompression {
 		cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))

diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go
@@ -189,6 +189,16 @@ func TestConfigs(t *testing.T) {
 		},
 
 		// Certificate tests
+		{
+			name: "Test Default Certificate",
+			asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
+				if grpcOption {
+					assert.NotNil(t, c.Traces.GRPCCredentials)
+				} else {
+					assert.Nil(t, c.Traces.TLSCfg)
+				}
+			},
+		},
 		{
 			name: "Test With Certificate",
 			opts: []otlpconfig.GenericOption{
@@ -378,27 +388,32 @@ func TestConfigs(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-
-			e := otlpconfig.EnvOptionsReader{
+			origEOR := otlpconfig.DefaultEnvOptionsReader
+			otlpconfig.DefaultEnvOptionsReader = otlpconfig.EnvOptionsReader{
 				GetEnv:   tt.env.getEnv,
 				ReadFile: tt.fileReader.readFile,
 			}
+			t.Cleanup(func() { otlpconfig.DefaultEnvOptionsReader = origEOR })
 
 			// Tests Generic options as HTTP Options
 			cfg := otlpconfig.NewDefaultConfig()
-			e.ApplyHTTPEnvConfigs(&cfg)
+			otlpconfig.ApplyHTTPEnvConfigs(&cfg)
 			for _, opt := range tt.opts {
 				opt.ApplyHTTPOption(&cfg)
 			}
 			tt.asserts(t, &cfg, false)
 
 			// Tests Generic options as gRPC Options
-			cfg = otlpconfig.NewDefaultConfig()
-			e.ApplyGRPCEnvConfigs(&cfg)
-			for _, opt := range tt.opts {
-				opt.ApplyGRPCOption(&cfg)
-			}
+			cfg = otlpconfig.NewGRPCConfig(asGRPCOptions(tt.opts)...)
 			tt.asserts(t, &cfg, true)
 		})
 	}
 }
+
+func asGRPCOptions(opts []otlpconfig.GenericOption) []otlpconfig.GRPCOption {
+	converted := make([]otlpconfig.GRPCOption, len(opts))
+	for i, o := range opts {
+		converted[i] = otlpconfig.NewGRPCOption(o.ApplyGRPCOption)
+	}
+	return converted
+}
diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client_test.go b/exporters/otlp/otlptrace/otlptracegrpc/client_test.go
@@ -238,16 +238,6 @@ func TestExportSpansTimeoutHonored(t *testing.T) {
 	require.Equal(t, codes.DeadlineExceeded, status.Convert(err).Code())
 }
 
-func TestStartErrorInvalidSecurityConfiguration(t *testing.T) {
-	mc := runMockCollector(t)
-	t.Cleanup(func() { require.NoError(t, mc.stop()) })
-
-	client := otlptracegrpc.NewClient(otlptracegrpc.WithEndpoint(mc.endpoint))
-	err := client.Start(context.Background())
-	// https://github.com/grpc/grpc-go/blob/a671967dfbaab779d37fd7e597d9248f13806087/clientconn.go#L82
-	assert.EqualError(t, err, "grpc: no transport security set (use grpc.WithInsecure() explicitly or set credentials)")
-}
-
 func TestNew_withMultipleAttributeTypes(t *testing.T) {
 	mc := runMockCollector(t)