Revert "Make TLSClient config to pointer type (#4104)" (#4196)

This reverts commit ebb0fbd.
open-telemetry · Oct 13, 2021 · 028fc46 · 028fc46
1 parent 5570093
commit 028fc46
Show file tree

Hide file tree

Showing 9 changed files with 46 additions and 57 deletions.
diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go
@@ -73,7 +73,7 @@ type GRPCClientSettings struct {
 	Compression string `mapstructure:"compression"`
 
 	// TLSSetting struct exposes TLS client configuration.
-	TLSSetting *configtls.TLSClientSetting `mapstructure:"tls,omitempty"`
+	TLSSetting configtls.TLSClientSetting `mapstructure:"tls,omitempty"`
 
 	// The keepalive parameters for gRPC client. See grpc.WithKeepaliveParams.
 	// (https://godoc.org/google.golang.org/grpc#WithKeepaliveParams).
@@ -181,28 +181,24 @@ func (gcs *GRPCClientSettings) isSchemeHTTPS() bool {
 // ToDialOptions maps configgrpc.GRPCClientSettings to a slice of dial options for gRPC.
 func (gcs *GRPCClientSettings) ToDialOptions(host component.Host) ([]grpc.DialOption, error) {
 	var opts []grpc.DialOption
-	var tlsCfg *tls.Config
-	var err error
 	if gcs.Compression != "" {
 		if compressionKey := GetGRPCCompressionKey(gcs.Compression); compressionKey != CompressionUnsupported {
 			opts = append(opts, grpc.WithDefaultCallOptions(grpc.UseCompressor(compressionKey)))
 		} else {
 			return nil, fmt.Errorf("unsupported compression type %q", gcs.Compression)
 		}
 	}
-	tlsDialOption := grpc.WithInsecure()
-	if gcs.TLSSetting != nil {
-		tlsCfg, err = gcs.TLSSetting.LoadTLSConfig()
-		if err != nil {
-			return nil, err
-		}
+
+	tlsCfg, err := gcs.TLSSetting.LoadTLSConfig()
+	if err != nil {
+		return nil, err
 	}
+	tlsDialOption := grpc.WithInsecure()
 	if tlsCfg != nil {
 		tlsDialOption = grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg))
 	} else if gcs.isSchemeHTTPS() {
 		tlsDialOption = grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{}))
 	}
-
 	opts = append(opts, tlsDialOption)
 
 	if gcs.ReadBufferSize > 0 {
@@ -239,7 +235,7 @@ func (gcs *GRPCClientSettings) ToDialOptions(host component.Host) ([]grpc.DialOp
 
 		perRPCCredentials, perr := grpcAuthenticator.PerRPCCredentials()
 		if perr != nil {
-			return nil, perr
+			return nil, err
 		}
 		opts = append(opts, grpc.WithPerRPCCredentials(perRPCCredentials))
 	}

diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go
@@ -38,7 +38,7 @@ import (
 
 func TestDefaultGrpcClientSettings(t *testing.T) {
 	gcs := &GRPCClientSettings{
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: true,
 		},
 	}
@@ -54,7 +54,7 @@ func TestAllGrpcClientSettings(t *testing.T) {
 		},
 		Endpoint:    "localhost:1234",
 		Compression: "gzip",
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: false,
 		},
 		Keepalive: &KeepaliveClientConfig{
@@ -160,7 +160,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 				Headers:     nil,
 				Endpoint:    "",
 				Compression: "",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					TLSSetting: configtls.TLSSetting{
 						CAFile: "/doesnt/exist",
 					},
@@ -176,7 +176,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 				Headers:     nil,
 				Endpoint:    "",
 				Compression: "",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					TLSSetting: configtls.TLSSetting{
 						CertFile: "/doesnt/exist",
 					},
@@ -194,7 +194,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 				},
 				Endpoint:    "localhost:1234",
 				Compression: "gzip",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					Insecure: false,
 				},
 				Keepalive: &KeepaliveClientConfig{
@@ -250,7 +250,7 @@ func TestUseSecure(t *testing.T) {
 		Headers:     nil,
 		Endpoint:    "",
 		Compression: "",
-		TLSSetting:  &configtls.TLSClientSetting{},
+		TLSSetting:  configtls.TLSClientSetting{},
 		Keepalive:   nil,
 	}
 	dialOpts, err := gcs.ToDialOptions(componenttest.NewNopHost())
@@ -472,7 +472,7 @@ func TestHttpReception(t *testing.T) {
 
 			gcs := &GRPCClientSettings{
 				Endpoint:   ln.Addr().String(),
-				TLSSetting: tt.tlsClientCreds,
+				TLSSetting: *tt.tlsClientCreds,
 			}
 			clientOpts, errClient := gcs.ToDialOptions(componenttest.NewNopHost())
 			assert.NoError(t, errClient)
@@ -517,7 +517,7 @@ func TestReceiveOnUnixDomainSocket(t *testing.T) {
 
 	gcs := &GRPCClientSettings{
 		Endpoint: "unix://" + ln.Addr().String(),
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: true,
 		},
 	}

diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go
@@ -38,7 +38,7 @@ type HTTPClientSettings struct {
 	Endpoint string `mapstructure:"endpoint"`
 
 	// TLSSetting struct exposes TLS client configuration.
-	TLSSetting *configtls.TLSClientSetting `mapstructure:"tls,omitempty"`
+	TLSSetting configtls.TLSClientSetting `mapstructure:"tls,omitempty"`
 
 	// ReadBufferSize for HTTP client. See http.Transport.ReadBufferSize.
 	ReadBufferSize int `mapstructure:"read_buffer_size"`
@@ -62,19 +62,14 @@ type HTTPClientSettings struct {
 
 // ToClient creates an HTTP client.
 func (hcs *HTTPClientSettings) ToClient(ext map[config.ComponentID]component.Extension) (*http.Client, error) {
-	var err error
+	tlsCfg, err := hcs.TLSSetting.LoadTLSConfig()
+	if err != nil {
+		return nil, err
+	}
 	transport := http.DefaultTransport.(*http.Transport).Clone()
-
-	if hcs.TLSSetting != nil {
-		tlsCfg, terr := hcs.TLSSetting.LoadTLSConfig()
-		if terr != nil {
-			return nil, terr
-		}
-		if tlsCfg != nil {
-			transport.TLSClientConfig = tlsCfg
-		}
+	if tlsCfg != nil {
+		transport.TLSClientConfig = tlsCfg
 	}
-
 	if hcs.ReadBufferSize > 0 {
 		transport.ReadBufferSize = hcs.ReadBufferSize
 	}

diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go
@@ -57,7 +57,7 @@ func TestAllHTTPClientSettings(t *testing.T) {
 			name: "all_valid_settings",
 			settings: HTTPClientSettings{
 				Endpoint: "localhost:1234",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					Insecure: false,
 				},
 				ReadBufferSize:     1024,
@@ -70,7 +70,7 @@ func TestAllHTTPClientSettings(t *testing.T) {
 			name: "error_round_tripper_returned",
 			settings: HTTPClientSettings{
 				Endpoint: "localhost:1234",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					Insecure: false,
 				},
 				ReadBufferSize:     1024,
@@ -105,7 +105,7 @@ func TestHTTPClientSettingsError(t *testing.T) {
 			err: "^failed to load TLS config: failed to load CA CertPool: failed to load CA /doesnt/exist:",
 			settings: HTTPClientSettings{
 				Endpoint: "",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					TLSSetting: configtls.TLSSetting{
 						CAFile: "/doesnt/exist",
 					},
@@ -118,7 +118,7 @@ func TestHTTPClientSettingsError(t *testing.T) {
 			err: "^failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither",
 			settings: HTTPClientSettings{
 				Endpoint: "",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					TLSSetting: configtls.TLSSetting{
 						CertFile: "/doesnt/exist",
 					},
@@ -407,7 +407,7 @@ func TestHttpReception(t *testing.T) {
 
 			hcs := &HTTPClientSettings{
 				Endpoint:   prefix + ln.Addr().String(),
-				TLSSetting: tt.tlsClientCreds,
+				TLSSetting: *tt.tlsClientCreds,
 			}
 			client, errClient := hcs.ToClient(map[config.ComponentID]component.Extension{})
 			assert.NoError(t, errClient)
@@ -582,7 +582,7 @@ func TestHttpHeaders(t *testing.T) {
 			serverURL, _ := url.Parse(server.URL)
 			setting := HTTPClientSettings{
 				Endpoint:        serverURL.String(),
-				TLSSetting:      &configtls.TLSClientSetting{},
+				TLSSetting:      configtls.TLSClientSetting{},
 				ReadBufferSize:  0,
 				WriteBufferSize: 0,
 				Timeout:         0,

diff --git a/exporter/otlpexporter/config_test.go b/exporter/otlpexporter/config_test.go
@@ -72,7 +72,7 @@ func TestLoadConfig(t *testing.T) {
 				},
 				Endpoint:    "1.2.3.4:1234",
 				Compression: "on",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					TLSSetting: configtls.TLSSetting{
 						CAFile: "/var/lib/mycert.pem",
 					},

diff --git a/exporter/otlpexporter/factory_test.go b/exporter/otlpexporter/factory_test.go
@@ -78,7 +78,7 @@ func TestCreateTracesExporter(t *testing.T) {
 				ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
 				GRPCClientSettings: configgrpc.GRPCClientSettings{
 					Endpoint: endpoint,
-					TLSSetting: &configtls.TLSClientSetting{
+					TLSSetting: configtls.TLSClientSetting{
 						Insecure: false,
 					},
 				},
@@ -147,7 +147,7 @@ func TestCreateTracesExporter(t *testing.T) {
 				ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
 				GRPCClientSettings: configgrpc.GRPCClientSettings{
 					Endpoint: endpoint,
-					TLSSetting: &configtls.TLSClientSetting{
+					TLSSetting: configtls.TLSClientSetting{
 						TLSSetting: configtls.TLSSetting{
 							CAFile: "testdata/test_cert.pem",
 						},
@@ -161,7 +161,7 @@ func TestCreateTracesExporter(t *testing.T) {
 				ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
 				GRPCClientSettings: configgrpc.GRPCClientSettings{
 					Endpoint: endpoint,
-					TLSSetting: &configtls.TLSClientSetting{
+					TLSSetting: configtls.TLSClientSetting{
 						TLSSetting: configtls.TLSSetting{
 							CAFile: "nosuchfile",
 						},

diff --git a/exporter/otlpexporter/otlp_test.go b/exporter/otlpexporter/otlp_test.go
@@ -195,7 +195,7 @@ func TestSendTraces(t *testing.T) {
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.GRPCClientSettings = configgrpc.GRPCClientSettings{
 		Endpoint: ln.Addr().String(),
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: true,
 		},
 		Headers: map[string]string{
@@ -258,19 +258,17 @@ func TestSendTracesWhenEndpointHasHttpScheme(t *testing.T) {
 		gRPCClientSettings configgrpc.GRPCClientSettings
 	}{
 		{
-			name:   "Use https scheme",
-			useTLS: true,
-			scheme: "https://",
-			gRPCClientSettings: configgrpc.GRPCClientSettings{
-				TLSSetting: &configtls.TLSClientSetting{},
-			},
+			name:               "Use https scheme",
+			useTLS:             true,
+			scheme:             "https://",
+			gRPCClientSettings: configgrpc.GRPCClientSettings{},
 		},
 		{
 			name:   "Use http scheme",
 			useTLS: false,
 			scheme: "http://",
 			gRPCClientSettings: configgrpc.GRPCClientSettings{
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					Insecure: true,
 				},
 			},
@@ -338,7 +336,7 @@ func TestSendMetrics(t *testing.T) {
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.GRPCClientSettings = configgrpc.GRPCClientSettings{
 		Endpoint: ln.Addr().String(),
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: true,
 		},
 		Headers: map[string]string{
@@ -406,7 +404,7 @@ func TestSendTraceDataServerDownAndUp(t *testing.T) {
 	cfg.QueueSettings.Enabled = false
 	cfg.GRPCClientSettings = configgrpc.GRPCClientSettings{
 		Endpoint: ln.Addr().String(),
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: true,
 		},
 		// Need to wait for every request blocking until either request timeouts or succeed.
@@ -466,7 +464,7 @@ func TestSendTraceDataServerStartWhileRequest(t *testing.T) {
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.GRPCClientSettings = configgrpc.GRPCClientSettings{
 		Endpoint: ln.Addr().String(),
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: true,
 		},
 	}
@@ -542,7 +540,7 @@ func TestSendLogData(t *testing.T) {
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.GRPCClientSettings = configgrpc.GRPCClientSettings{
 		Endpoint: ln.Addr().String(),
-		TLSSetting: &configtls.TLSClientSetting{
+		TLSSetting: configtls.TLSClientSetting{
 			Insecure: true,
 		},
 	}

diff --git a/exporter/otlphttpexporter/config_test.go b/exporter/otlphttpexporter/config_test.go
@@ -66,7 +66,7 @@ func TestLoadConfig(t *testing.T) {
 					"another":                "somevalue",
 				},
 				Endpoint: "https://1.2.3.4:1234",
-				TLSSetting: &configtls.TLSClientSetting{
+				TLSSetting: configtls.TLSClientSetting{
 					TLSSetting: configtls.TLSSetting{
 						CAFile:   "/var/lib/mycert.pem",
 						CertFile: "certfile",

diff --git a/exporter/otlphttpexporter/factory_test.go b/exporter/otlphttpexporter/factory_test.go
@@ -82,7 +82,7 @@ func TestCreateTracesExporter(t *testing.T) {
 				ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
 				HTTPClientSettings: confighttp.HTTPClientSettings{
 					Endpoint: endpoint,
-					TLSSetting: &configtls.TLSClientSetting{
+					TLSSetting: configtls.TLSClientSetting{
 						Insecure: false,
 					},
 				},
@@ -107,7 +107,7 @@ func TestCreateTracesExporter(t *testing.T) {
 				ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
 				HTTPClientSettings: confighttp.HTTPClientSettings{
 					Endpoint: endpoint,
-					TLSSetting: &configtls.TLSClientSetting{
+					TLSSetting: configtls.TLSClientSetting{
 						TLSSetting: configtls.TLSSetting{
 							CAFile: "testdata/test_cert.pem",
 						},
@@ -121,7 +121,7 @@ func TestCreateTracesExporter(t *testing.T) {
 				ExporterSettings: config.NewExporterSettings(config.NewComponentID(typeStr)),
 				HTTPClientSettings: confighttp.HTTPClientSettings{
 					Endpoint: endpoint,
-					TLSSetting: &configtls.TLSClientSetting{
+					TLSSetting: configtls.TLSClientSetting{
 						TLSSetting: configtls.TLSSetting{
 							CAFile: "nosuchfile",
 						},