From aa291c08490799c103a8376e79a1a5b5c61c230c Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Wed, 19 Oct 2022 01:15:29 +0100 Subject: [PATCH 01/10] add new env config options for OTLP exporter --- CHANGELOG.md | 10 ++ .../otlp/internal/envconfig/envconfig.go | 46 +++++- .../otlp/internal/envconfig/envconfig_test.go | 141 ++++++++++++++++-- .../otlpmetric/internal/oconf/envconfig.go | 27 +++- .../internal/otlpconfig/envconfig.go | 27 +++- 5 files changed, 225 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34be9d346ae..b7f5c96a595 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Prometheus exporter will register with a prometheus registerer on creation, there are options to control this. (#3239) - Added the `WithAggregationSelector` option to the `go.opentelemetry.io/otel/exporters/prometheus` package to change the `AggregationSelector` used. (#3341) +- OTLP exporters now recognize: + - `OTEL_EXPORTER_OTLP_INSECURE` + - `OTEL_EXPORTER_OTLP_TRACES_INSECURE` + - `OTEL_EXPORTER_OTLP_METRICS_INSECURE` + - `OTEL_EXPORTER_OTLP_CLIENT_KEY` + - `OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY` + - `OTEL_EXPORTER_OTLP_METRICS_CLIENT_KEY` + - `OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE` + - `OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE` + - `OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE` ### Changed diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index 67003c4a2fa..eb5b0a7962c 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -59,6 +59,16 @@ func WithString(n string, fn func(string)) func(e *EnvOptionsReader) { } } +// WithBool retrieves the specified config and passes it to ConfigFn as a boolean. +func WithBool(n string, fn func(bool)) func(e *EnvOptionsReader) { + return func(e *EnvOptionsReader) { + if v, ok := e.GetEnvValue(n); ok { + b := strings.ToLower(v) == "true" + fn(b) + } + } +} + // WithDuration retrieves the specified config and passes it to ConfigFn as a duration. func WithDuration(n string, fn func(time.Duration)) func(e *EnvOptionsReader) { return func(e *EnvOptionsReader) { @@ -90,12 +100,13 @@ func WithURL(n string, fn func(*url.URL)) func(e *EnvOptionsReader) { } } -// WithTLSConfig retrieves the specified config and passes it to ConfigFn as a crypto/tls.Config. -func WithTLSConfig(n string, fn func(*tls.Config)) func(e *EnvOptionsReader) { +// WithCertPool retrieves the specified config and passes it to ConfigFn as a crypto/x509.CertPool +// constructed from reading the certificate at the given path. +func WithCertPool(n string, fn func(*x509.CertPool)) func(e *EnvOptionsReader) { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { if b, err := e.ReadFile(v); err == nil { - if c, err := createTLSConfig(b); err == nil { + if c, err := createCertPool(b); err == nil { fn(c) } } @@ -103,6 +114,28 @@ func WithTLSConfig(n string, fn func(*tls.Config)) func(e *EnvOptionsReader) { } } +// WithCertPool retrieves the specified configs and passes it to ConfigFn as a crypto/tls.Certificate +// constructed from reading the key pair at the given paths. Both files must exist. +func WithClientCert(nc, nk string, fn func(tls.Certificate)) func(e *EnvOptionsReader) { + return func(e *EnvOptionsReader) { + vc, okc := e.GetEnvValue(nc) + vk, okk := e.GetEnvValue(nk) + if !okc || !okk { + return + } + cert, errc := e.ReadFile(vc) + key, errk := e.ReadFile(vk) + if errc != nil && errk != nil { + return + } + crt, err := tls.X509KeyPair(cert, key) + if err != nil { + return + } + fn(crt) + } +} + func keyWithNamespace(ns, key string) string { if ns == "" { return key @@ -136,13 +169,10 @@ func stringToHeader(value string) map[string]string { return headers } -func createTLSConfig(certBytes []byte) (*tls.Config, error) { +func createCertPool(certBytes []byte) (*x509.CertPool, error) { cp := x509.NewCertPool() if ok := cp.AppendCertsFromPEM(certBytes); !ok { return nil, errors.New("failed to append certificate to the cert pool") } - - return &tls.Config{ - RootCAs: cp, - }, nil + return cp, nil } diff --git a/exporters/otlp/internal/envconfig/envconfig_test.go b/exporters/otlp/internal/envconfig/envconfig_test.go index eaf85b1ec7a..3bd13255cff 100644 --- a/exporters/otlp/internal/envconfig/envconfig_test.go +++ b/exporters/otlp/internal/envconfig/envconfig_test.go @@ -16,6 +16,7 @@ package envconfig // import "go.opentelemetry.io/otel/exporters/otlp/internal/en import ( "crypto/tls" + "crypto/x509" "net/url" "testing" "time" @@ -23,22 +24,31 @@ import ( "github.com/stretchr/testify/assert" ) +const WeakKey = ` +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIEbrSPmnlSOXvVzxCyv+VR3a0HDeUTvOcqrdssZ2k4gFoAoGCCqGSM49 +AwEHoUQDQgAEDMTfv75J315C3K9faptS9iythKOMEeV/Eep73nWX531YAkmmwBSB +2dXRD/brsgLnfG57WEpxZuY7dPRbxu33BA== +-----END EC PRIVATE KEY----- +` + const WeakCertificate = ` -----BEGIN CERTIFICATE----- -MIIBhzCCASygAwIBAgIRANHpHgAWeTnLZpTSxCKs0ggwCgYIKoZIzj0EAwIwEjEQ -MA4GA1UEChMHb3RlbC1nbzAeFw0yMTA0MDExMzU5MDNaFw0yMTA0MDExNDU5MDNa -MBIxEDAOBgNVBAoTB290ZWwtZ28wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS9 -nWSkmPCxShxnp43F+PrOtbGV7sNfkbQ/kxzi9Ego0ZJdiXxkmv/C05QFddCW7Y0Z -sJCLHGogQsYnWJBXUZOVo2MwYTAOBgNVHQ8BAf8EBAMCB4AwEwYDVR0lBAwwCgYI -KwYBBQUHAwEwDAYDVR0TAQH/BAIwADAsBgNVHREEJTAjgglsb2NhbGhvc3SHEAAA -AAAAAAAAAAAAAAAAAAGHBH8AAAEwCgYIKoZIzj0EAwIDSQAwRgIhANwZVVKvfvQ/ -1HXsTvgH+xTQswOwSSKYJ1cVHQhqK7ZbAiEAus8NxpTRnp5DiTMuyVmhVNPB+bVH -Lhnm4N/QDk5rek0= +MIIBjjCCATWgAwIBAgIUKQSMC66MUw+kPp954ZYOcyKAQDswCgYIKoZIzj0EAwIw +EjEQMA4GA1UECgwHb3RlbC1nbzAeFw0yMjEwMTkwMDA5MTlaFw0yMzEwMTkwMDA5 +MTlaMBIxEDAOBgNVBAoMB290ZWwtZ28wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNC +AAQMxN+/vknfXkLcr19qm1L2LK2Eo4wR5X8R6nvedZfnfVgCSabAFIHZ1dEP9uuy +Aud8bntYSnFm5jt09FvG7fcEo2kwZzAdBgNVHQ4EFgQUicGuhnTTkYLZwofXMNLK +SHFeCWgwHwYDVR0jBBgwFoAUicGuhnTTkYLZwofXMNLKSHFeCWgwDwYDVR0TAQH/ +BAUwAwEB/zAUBgNVHREEDTALgglsb2NhbGhvc3QwCgYIKoZIzj0EAwIDRwAwRAIg +Lfma8FnnxeSOi6223AsFfYwsNZ2RderNsQrS0PjEHb0CIBkrWacqARUAu7uT4cGu +jVcIxYQqhId5L8p/mAv2PWZS -----END CERTIFICATE----- ` type testOption struct { TestString string + TestBool bool TestDuration time.Duration TestHeaders map[string]string TestURL *url.URL @@ -134,6 +144,56 @@ func TestEnvConfig(t *testing.T) { }, expectedOptions: []testOption{}, }, + { + name: "with a bool config", + reader: EnvOptionsReader{ + GetEnv: func(n string) string { + if n == "HELLO" { + return "true" + } else if n == "WORLD" { + return "false" + } + return "" + }, + }, + configs: []ConfigFn{ + WithBool("HELLO", func(b bool) { + options = append(options, testOption{TestBool: b}) + }), + WithBool("WORLD", func(b bool) { + options = append(options, testOption{TestBool: b}) + }), + }, + expectedOptions: []testOption{ + { + TestBool: true, + }, + { + TestBool: false, + }, + }, + }, + { + name: "with an invalid bool config", + reader: EnvOptionsReader{ + GetEnv: func(n string) string { + if n == "HELLO" { + return "world" + } + return "" + }, + }, + configs: []ConfigFn{ + WithBool("HELLO", func(b bool) { + options = append(options, testOption{TestBool: b}) + }), + }, + expectedOptions: []testOption{ + { + TestBool: false, + }, + }, + }, { name: "with a duration config", reader: EnvOptionsReader{ @@ -265,7 +325,7 @@ func TestEnvConfig(t *testing.T) { } func TestWithTLSConfig(t *testing.T) { - tlsCert, err := createTLSConfig([]byte(WeakCertificate)) + pool, err := createCertPool([]byte(WeakCertificate)) assert.NoError(t, err) reader := EnvOptionsReader{ @@ -285,12 +345,65 @@ func TestWithTLSConfig(t *testing.T) { var option testOption reader.Apply( - WithTLSConfig("CERTIFICATE", func(v *tls.Config) { - option = testOption{TestTLS: v} - })) + WithCertPool("CERTIFICATE", func(cp *x509.CertPool) { + option = testOption{TestTLS: &tls.Config{RootCAs: cp}} + }), + ) // nolint:staticcheck // ignoring tlsCert.RootCAs.Subjects is deprecated ERR because cert does not come from SystemCertPool. - assert.Equal(t, tlsCert.RootCAs.Subjects(), option.TestTLS.RootCAs.Subjects()) + assert.Equal(t, pool.Subjects(), option.TestTLS.RootCAs.Subjects()) +} + +func TestWithClientCert(t *testing.T) { + cert, err := tls.X509KeyPair([]byte(WeakCertificate), []byte(WeakKey)) + assert.NoError(t, err) + + reader := EnvOptionsReader{ + GetEnv: func(n string) string { + switch n { + case "CLIENT_CERTIFICATE": + return "/path/tls.crt" + case "CLIENT_KEY": + return "/path/tls.key" + } + return "" + }, + ReadFile: func(n string) ([]byte, error) { + switch n { + case "/path/tls.crt": + return []byte(WeakCertificate), nil + case "/path/tls.key": + return []byte(WeakKey), nil + } + return []byte{}, nil + }, + } + + var option testOption + reader.Apply( + WithClientCert("CLIENT_CERTIFICATE", "CLIENT_KEY", func(c tls.Certificate) { + option = testOption{TestTLS: &tls.Config{Certificates: []tls.Certificate{c}}} + }), + ) + assert.Equal(t, cert, option.TestTLS.Certificates[0]) + + reader.ReadFile = func(s string) ([]byte, error) { return []byte{}, nil } + option.TestTLS = nil + reader.Apply( + WithClientCert("CLIENT_CERTIFICATE", "CLIENT_KEY", func(c tls.Certificate) { + option = testOption{TestTLS: &tls.Config{Certificates: []tls.Certificate{c}}} + }), + ) + assert.Nil(t, option.TestTLS) + + reader.GetEnv = func(s string) string { return "" } + option.TestTLS = nil + reader.Apply( + WithClientCert("CLIENT_CERTIFICATE", "CLIENT_KEY", func(c tls.Certificate) { + option = testOption{TestTLS: &tls.Config{Certificates: []tls.Certificate{c}}} + }), + ) + assert.Nil(t, option.TestTLS) } func TestStringToHeader(t *testing.T) { diff --git a/exporters/otlp/otlpmetric/internal/oconf/envconfig.go b/exporters/otlp/otlpmetric/internal/oconf/envconfig.go index 96c6fc1753a..93b1209388d 100644 --- a/exporters/otlp/otlpmetric/internal/oconf/envconfig.go +++ b/exporters/otlp/otlpmetric/internal/oconf/envconfig.go @@ -16,6 +16,7 @@ package oconf // import "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/inte import ( "crypto/tls" + "crypto/x509" "net/url" "os" "path" @@ -53,6 +54,7 @@ func ApplyHTTPEnvConfigs(cfg Config) Config { func getOptionsFromEnv() []GenericOption { opts := []GenericOption{} + tlsConf := &tls.Config{} DefaultEnvOptionsReader.Apply( envconfig.WithURL("ENDPOINT", func(u *url.URL) { opts = append(opts, withEndpointScheme(u)) @@ -81,8 +83,13 @@ func getOptionsFromEnv() []GenericOption { return cfg }, withEndpointForGRPC(u))) }), - envconfig.WithTLSConfig("CERTIFICATE", func(c *tls.Config) { opts = append(opts, WithTLSClientConfig(c)) }), - envconfig.WithTLSConfig("METRICS_CERTIFICATE", func(c *tls.Config) { opts = append(opts, WithTLSClientConfig(c)) }), + envconfig.WithCertPool("CERTIFICATE", func(p *x509.CertPool) { tlsConf.RootCAs = p }), + envconfig.WithCertPool("METRICS_CERTIFICATE", func(p *x509.CertPool) { tlsConf.RootCAs = p }), + envconfig.WithClientCert("CLIENT_CERTIFICATE", "CLIENT_KEY", func(c tls.Certificate) { tlsConf.Certificates = []tls.Certificate{c} }), + envconfig.WithClientCert("METRICS_CLIENT_CERTIFICATE", "METRICS_CLIENT_KEY", func(c tls.Certificate) { tlsConf.Certificates = []tls.Certificate{c} }), + envconfig.WithBool("INSECURE", func(b bool) { opts = append(opts, withInsecure(b)) }), + envconfig.WithBool("METRICS_INSECURE", func(b bool) { opts = append(opts, withInsecure(b)) }), + withTLSConfig(tlsConf, func(c *tls.Config) { opts = append(opts, WithTLSClientConfig(c)) }), envconfig.WithHeaders("HEADERS", func(h map[string]string) { opts = append(opts, WithHeaders(h)) }), envconfig.WithHeaders("METRICS_HEADERS", func(h map[string]string) { opts = append(opts, WithHeaders(h)) }), WithEnvCompression("COMPRESSION", func(c Compression) { opts = append(opts, WithCompression(c)) }), @@ -125,3 +132,19 @@ func withEndpointScheme(u *url.URL) GenericOption { return WithSecure() } } + +// revive:disable-next-line:flag-parameter +func withInsecure(b bool) GenericOption { + if b { + return WithInsecure() + } + return WithSecure() +} + +func withTLSConfig(c *tls.Config, fn func(*tls.Config)) func(e *envconfig.EnvOptionsReader) { + return func(e *envconfig.EnvOptionsReader) { + if c.RootCAs != nil || len(c.Certificates) > 0 { + fn(c) + } + } +} diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go index b29f618e3de..62c5029db2a 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go @@ -16,6 +16,7 @@ package otlpconfig // import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/ import ( "crypto/tls" + "crypto/x509" "net/url" "os" "path" @@ -53,6 +54,7 @@ func ApplyHTTPEnvConfigs(cfg Config) Config { func getOptionsFromEnv() []GenericOption { opts := []GenericOption{} + tlsConf := &tls.Config{} DefaultEnvOptionsReader.Apply( envconfig.WithURL("ENDPOINT", func(u *url.URL) { opts = append(opts, withEndpointScheme(u)) @@ -81,8 +83,13 @@ func getOptionsFromEnv() []GenericOption { return cfg }, withEndpointForGRPC(u))) }), - envconfig.WithTLSConfig("CERTIFICATE", func(c *tls.Config) { opts = append(opts, WithTLSClientConfig(c)) }), - envconfig.WithTLSConfig("TRACES_CERTIFICATE", func(c *tls.Config) { opts = append(opts, WithTLSClientConfig(c)) }), + envconfig.WithCertPool("CERTIFICATE", func(p *x509.CertPool) { tlsConf.RootCAs = p }), + envconfig.WithCertPool("TRACES_CERTIFICATE", func(p *x509.CertPool) { tlsConf.RootCAs = p }), + envconfig.WithClientCert("CLIENT_CERTIFICATE", "CLIENT_KEY", func(c tls.Certificate) { tlsConf.Certificates = []tls.Certificate{c} }), + envconfig.WithClientCert("TRACES_CLIENT_CERTIFICATE", "TRACES_CLIENT_KEY", func(c tls.Certificate) { tlsConf.Certificates = []tls.Certificate{c} }), + withTLSConfig(tlsConf, func(c *tls.Config) { opts = append(opts, WithTLSClientConfig(c)) }), + envconfig.WithBool("INSECURE", func(b bool) { opts = append(opts, withInsecure(b)) }), + envconfig.WithBool("TRACES_INSECURE", func(b bool) { opts = append(opts, withInsecure(b)) }), envconfig.WithHeaders("HEADERS", func(h map[string]string) { opts = append(opts, WithHeaders(h)) }), envconfig.WithHeaders("TRACES_HEADERS", func(h map[string]string) { opts = append(opts, WithHeaders(h)) }), WithEnvCompression("COMPRESSION", func(c Compression) { opts = append(opts, WithCompression(c)) }), @@ -125,3 +132,19 @@ func WithEnvCompression(n string, fn func(Compression)) func(e *envconfig.EnvOpt } } } + +// revive:disable-next-line:flag-parameter +func withInsecure(b bool) GenericOption { + if b { + return WithInsecure() + } + return WithSecure() +} + +func withTLSConfig(c *tls.Config, fn func(*tls.Config)) func(e *envconfig.EnvOptionsReader) { + return func(e *envconfig.EnvOptionsReader) { + if c.RootCAs != nil || len(c.Certificates) > 0 { + fn(c) + } + } +} From b4bcc9120f86b1e5c8930c6a4ec964f64aa0721d Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Wed, 19 Oct 2022 13:25:30 +0100 Subject: [PATCH 02/10] error path --- exporters/otlp/internal/envconfig/envconfig_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/internal/envconfig/envconfig_test.go b/exporters/otlp/internal/envconfig/envconfig_test.go index 3bd13255cff..eb2ab8a6806 100644 --- a/exporters/otlp/internal/envconfig/envconfig_test.go +++ b/exporters/otlp/internal/envconfig/envconfig_test.go @@ -17,6 +17,7 @@ package envconfig // import "go.opentelemetry.io/otel/exporters/otlp/internal/en import ( "crypto/tls" "crypto/x509" + "errors" "net/url" "testing" "time" @@ -387,7 +388,7 @@ func TestWithClientCert(t *testing.T) { ) assert.Equal(t, cert, option.TestTLS.Certificates[0]) - reader.ReadFile = func(s string) ([]byte, error) { return []byte{}, nil } + reader.ReadFile = func(s string) ([]byte, error) { return nil, errors.New("oops") } option.TestTLS = nil reader.Apply( WithClientCert("CLIENT_CERTIFICATE", "CLIENT_KEY", func(c tls.Certificate) { From bdf36e165044c3eedb4cc1d38316ed679093fe05 Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 4 Nov 2022 13:25:00 +0000 Subject: [PATCH 03/10] Update exporters/otlp/internal/envconfig/envconfig.go Co-authored-by: Tyler Yahn --- exporters/otlp/internal/envconfig/envconfig.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index eb5b0a7962c..f41fc771626 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -59,7 +59,7 @@ func WithString(n string, fn func(string)) func(e *EnvOptionsReader) { } } -// WithBool retrieves the specified config and passes it to ConfigFn as a boolean. +// WithBool returns a ConfigFn that reads the environment variable n and if it exists passes its parsed bool value to fn. func WithBool(n string, fn func(bool)) func(e *EnvOptionsReader) { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { From 87dffb28f8054d8c47c8e1f0a21384b8077e8fd0 Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 4 Nov 2022 13:25:28 +0000 Subject: [PATCH 04/10] Update exporters/otlp/internal/envconfig/envconfig.go Co-authored-by: Tyler Yahn --- exporters/otlp/internal/envconfig/envconfig.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index f41fc771626..50f5db95c40 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -60,7 +60,7 @@ func WithString(n string, fn func(string)) func(e *EnvOptionsReader) { } // WithBool returns a ConfigFn that reads the environment variable n and if it exists passes its parsed bool value to fn. -func WithBool(n string, fn func(bool)) func(e *EnvOptionsReader) { +func WithBool(n string, fn func(bool)) ConfigFn { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { b := strings.ToLower(v) == "true" From faca69b89a4679a3211351773723dc3ba88341dd Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 4 Nov 2022 13:25:37 +0000 Subject: [PATCH 05/10] Update exporters/otlp/internal/envconfig/envconfig.go Co-authored-by: Tyler Yahn --- exporters/otlp/internal/envconfig/envconfig.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index 50f5db95c40..5f00ba62f13 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -116,7 +116,7 @@ func WithCertPool(n string, fn func(*x509.CertPool)) func(e *EnvOptionsReader) { // WithCertPool retrieves the specified configs and passes it to ConfigFn as a crypto/tls.Certificate // constructed from reading the key pair at the given paths. Both files must exist. -func WithClientCert(nc, nk string, fn func(tls.Certificate)) func(e *EnvOptionsReader) { +func WithClientCert(nc, nk string, fn func(tls.Certificate)) ConfigFn { return func(e *EnvOptionsReader) { vc, okc := e.GetEnvValue(nc) vk, okk := e.GetEnvValue(nk) From 3aaff4e60481021fca5b729cbfa0f92844c28318 Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 4 Nov 2022 13:25:50 +0000 Subject: [PATCH 06/10] Update exporters/otlp/internal/envconfig/envconfig.go Co-authored-by: Tyler Yahn --- exporters/otlp/internal/envconfig/envconfig.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index 5f00ba62f13..3800d86a8a5 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -114,8 +114,7 @@ func WithCertPool(n string, fn func(*x509.CertPool)) func(e *EnvOptionsReader) { } } -// WithCertPool retrieves the specified configs and passes it to ConfigFn as a crypto/tls.Certificate -// constructed from reading the key pair at the given paths. Both files must exist. +// WithClientCert returns a ConfigFn that reads the environment variable nc and nk as filepaths to a client certificate and key pair. If they exists, they are parsed as a crypto/tls.Certificate and it is passed to fn. func WithClientCert(nc, nk string, fn func(tls.Certificate)) ConfigFn { return func(e *EnvOptionsReader) { vc, okc := e.GetEnvValue(nc) From 216f6dea479285771761e4c8045fff950a29fe68 Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 4 Nov 2022 13:25:58 +0000 Subject: [PATCH 07/10] Update exporters/otlp/internal/envconfig/envconfig.go Co-authored-by: Tyler Yahn --- exporters/otlp/internal/envconfig/envconfig.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index 3800d86a8a5..f60010b641e 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -100,8 +100,7 @@ func WithURL(n string, fn func(*url.URL)) func(e *EnvOptionsReader) { } } -// WithCertPool retrieves the specified config and passes it to ConfigFn as a crypto/x509.CertPool -// constructed from reading the certificate at the given path. +// WithCertPool returns a ConfigFn that reads the environment variable n as a filepath to a TLS certificate pool. If it exists, it is parsed as a crypto/x509.CertPool and it is passed to fn. func WithCertPool(n string, fn func(*x509.CertPool)) func(e *EnvOptionsReader) { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { From 026e3b9df7363f8a34bf9921d56da4cdfe06935f Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 4 Nov 2022 13:26:07 +0000 Subject: [PATCH 08/10] Update exporters/otlp/internal/envconfig/envconfig.go Co-authored-by: Tyler Yahn --- exporters/otlp/internal/envconfig/envconfig.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index f60010b641e..a661d1e2228 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -101,7 +101,7 @@ func WithURL(n string, fn func(*url.URL)) func(e *EnvOptionsReader) { } // WithCertPool returns a ConfigFn that reads the environment variable n as a filepath to a TLS certificate pool. If it exists, it is parsed as a crypto/x509.CertPool and it is passed to fn. -func WithCertPool(n string, fn func(*x509.CertPool)) func(e *EnvOptionsReader) { +func WithCertPool(n string, fn func(*x509.CertPool)) ConfigFn { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { if b, err := e.ReadFile(v); err == nil { From 1feaabb7fe2fef509b594effdaebe2199847a2d1 Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 4 Nov 2022 13:41:15 +0000 Subject: [PATCH 09/10] add error logging --- .../otlp/internal/envconfig/envconfig.go | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index a661d1e2228..96532b3b5e1 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -23,6 +23,8 @@ import ( "strconv" "strings" "time" + + "go.opentelemetry.io/otel/internal/global" ) // ConfigFn is the generic function used to set a config. @@ -73,9 +75,11 @@ func WithBool(n string, fn func(bool)) ConfigFn { func WithDuration(n string, fn func(time.Duration)) func(e *EnvOptionsReader) { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { - if d, err := strconv.Atoi(v); err == nil { - fn(time.Duration(d) * time.Millisecond) + d, err := strconv.Atoi(v) + if err != nil { + global.Error(err, "parse duration", "input", v) } + fn(time.Duration(d) * time.Millisecond) } } } @@ -93,9 +97,11 @@ func WithHeaders(n string, fn func(map[string]string)) func(e *EnvOptionsReader) func WithURL(n string, fn func(*url.URL)) func(e *EnvOptionsReader) { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { - if u, err := url.Parse(v); err == nil { - fn(u) + u, err := url.Parse(v) + if err != nil { + global.Error(err, "parse url", "input", v) } + fn(u) } } } @@ -104,11 +110,17 @@ func WithURL(n string, fn func(*url.URL)) func(e *EnvOptionsReader) { func WithCertPool(n string, fn func(*x509.CertPool)) ConfigFn { return func(e *EnvOptionsReader) { if v, ok := e.GetEnvValue(n); ok { - if b, err := e.ReadFile(v); err == nil { - if c, err := createCertPool(b); err == nil { - fn(c) - } + b, err := e.ReadFile(v) + if err != nil { + global.Error(err, "read tls ca cert file", "file", v) + return + } + c, err := createCertPool(b) + if err != nil { + global.Error(err, "create tls cert pool") + return } + fn(c) } } } @@ -121,13 +133,19 @@ func WithClientCert(nc, nk string, fn func(tls.Certificate)) ConfigFn { if !okc || !okk { return } - cert, errc := e.ReadFile(vc) - key, errk := e.ReadFile(vk) - if errc != nil && errk != nil { + cert, err := e.ReadFile(vc) + if err != nil { + global.Error(err, "read tls client cert", "file", vc) + return + } + key, err := e.ReadFile(vk) + if err != nil { + global.Error(err, "read tls client key", "file", vk) return } crt, err := tls.X509KeyPair(cert, key) if err != nil { + global.Error(err, "create tls client key pair") return } fn(crt) @@ -148,15 +166,18 @@ func stringToHeader(value string) map[string]string { for _, header := range headersPairs { nameValue := strings.SplitN(header, "=", 2) if len(nameValue) < 2 { + global.Error(errors.New("missing '="), "parse headers", "input", nameValue) continue } name, err := url.QueryUnescape(nameValue[0]) if err != nil { + global.Error(err, "escape header key", "key", nameValue[0]) continue } trimmedName := strings.TrimSpace(name) value, err := url.QueryUnescape(nameValue[1]) if err != nil { + global.Error(err, "escape header value", "value", nameValue[1]) continue } trimmedValue := strings.TrimSpace(value) From 4cf95efd17d72f745da1d3d62c743a1a5234e300 Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Mon, 7 Nov 2022 16:15:48 +0000 Subject: [PATCH 10/10] add missing early returns --- exporters/otlp/internal/envconfig/envconfig.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exporters/otlp/internal/envconfig/envconfig.go b/exporters/otlp/internal/envconfig/envconfig.go index 96532b3b5e1..53ff3126b6e 100644 --- a/exporters/otlp/internal/envconfig/envconfig.go +++ b/exporters/otlp/internal/envconfig/envconfig.go @@ -78,6 +78,7 @@ func WithDuration(n string, fn func(time.Duration)) func(e *EnvOptionsReader) { d, err := strconv.Atoi(v) if err != nil { global.Error(err, "parse duration", "input", v) + return } fn(time.Duration(d) * time.Millisecond) } @@ -100,6 +101,7 @@ func WithURL(n string, fn func(*url.URL)) func(e *EnvOptionsReader) { u, err := url.Parse(v) if err != nil { global.Error(err, "parse url", "input", v) + return } fn(u) }