Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS naming cleanup #166

Merged
merged 3 commits into from
Feb 23, 2023
Merged

TLS naming cleanup #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a574a4
rename some TLS keys and variables to be consistent
Feb 22, 2023
829edbc
rename test template variables to match config keys & flags
Feb 22, 2023
1b47851
add TLS settings to README.md
Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,10 @@ then config file, then environment variables.
| --tp-ignore-env | OTEL_CLI_IGNORE_ENV | traceparent_ignore_env | false |
| --tp-print | OTEL_CLI_PRINT_TRACEPARENT | traceparent_print | false |
| --tp-export | OTEL_CLI_EXPORT_TRACEPARENT | traceparent_print_export | false |
| --no-tls-verify | OTEL_CLI_NO_TLS_VERIFY | no_tls_verify | false |
| --tls-no-verify | OTEL_CLI_TLS_NO_VERIFY | tls_no_verify | false |
| --tls-ca-cert | OTEL_EXPORTER_OTLP_CERTIFICATE | tls_ca_cert | |
| --tls-client-key | OTEL_EXPORTER_OTLP_CLIENT_KEY | tls_client_key | |
| --tls-client-cert | OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE | tls_client_cert | |
Comment on lines +124 to +127
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do you care that the second column isn't aligned here now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I care, but chose to do that so the whole table wouldn't reformat in the diff. The MD will render the table aligned in the browser so it felt like an ok tradeoff for now.


[Valid timeout units](https://pkg.go.dev/time#ParseDuration) are "ns", "us"/"µs", "ms", "s", "m", "h".

Expand Down
28 changes: 19 additions & 9 deletions data_for_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ var suites = []FixtureSuite{
"status",
"--endpoint", "https://{{endpoint}}",
"--protocol", "grpc",
// TODO: switch to --tls-no-verify before 1.0, for now keep testing it
"--verbose", "--fail", "--no-tls-verify",
},
TestTimeoutMs: 1000,
Expand All @@ -161,7 +162,7 @@ var suites = []FixtureSuite{
WithEndpoint("https://{{endpoint}}").
WithProtocol("grpc").
WithVerbose(true).
WithNoTlsVerify(true),
WithTlsNoVerify(true),
Diagnostics: otelcli.Diagnostics{
IsRecording: true,
NumArgs: 8,
Expand All @@ -183,7 +184,7 @@ var suites = []FixtureSuite{
Expect: Results{
// otel-cli should NOT set insecure when it auto-detects localhost
Config: otelcli.DefaultConfig().
WithNoTlsVerify(true).
WithTlsNoVerify(true).
WithEndpoint("https://{{endpoint}}"),
Diagnostics: otelcli.Diagnostics{
IsRecording: true,
Expand All @@ -203,9 +204,9 @@ var suites = []FixtureSuite{
"--endpoint", "https://{{endpoint}}",
"--protocol", "grpc",
"--verbose", "--fail",
"--tls-ca-cert", "{{cacert}}",
"--tls-client-cert", "{{client_cert}}",
"--tls-client-key", "{{client_key}}",
"--tls-ca-cert", "{{tls_ca_cert}}",
"--tls-client-cert", "{{tls_client_cert}}",
"--tls-client-key", "{{tls_client_key}}",
},
TestTimeoutMs: 1000,
ServerTLSEnabled: true,
Expand All @@ -215,6 +216,9 @@ var suites = []FixtureSuite{
Config: otelcli.DefaultConfig().
WithEndpoint("https://{{endpoint}}").
WithProtocol("grpc").
WithTlsCACert("{{tls_ca_cert}}").
WithTlsClientKey("{{tls_client_key}}").
WithTlsClientCert("{{tls_client_cert}}").
WithVerbose(true),
Diagnostics: otelcli.Diagnostics{
IsRecording: true,
Expand All @@ -234,9 +238,9 @@ var suites = []FixtureSuite{
"status",
"--endpoint", "https://{{endpoint}}",
"--verbose", "--fail",
"--tls-ca-cert", "{{cacert}}",
"--tls-client-cert", "{{client_cert}}",
"--tls-client-key", "{{client_key}}",
"--tls-ca-cert", "{{tls_ca_cert}}",
"--tls-client-cert", "{{tls_client_cert}}",
"--tls-client-key", "{{tls_client_key}}",
},
TestTimeoutMs: 2000,
ServerTLSEnabled: true,
Expand All @@ -245,6 +249,9 @@ var suites = []FixtureSuite{
Expect: Results{
Config: otelcli.DefaultConfig().
WithEndpoint("https://{{endpoint}}").
WithTlsCACert("{{tls_ca_cert}}").
WithTlsClientKey("{{tls_client_key}}").
WithTlsClientCert("{{tls_client_cert}}").
WithVerbose(true),
Diagnostics: otelcli.Diagnostics{
IsRecording: true,
Expand Down Expand Up @@ -352,7 +359,10 @@ var suites = []FixtureSuite{
WithHeaders(map[string]string{"header1": "header1-value"}).
WithInsecure(true).
WithBlocking(false).
WithNoTlsVerify(true).
WithTlsNoVerify(true).
WithTlsCACert("/dev/null").
WithTlsClientKey("/dev/null").
WithTlsClientCert("/dev/null").
WithServiceName("configured_in_config_file").
WithSpanName("config_file_span").
WithKind("server").
Expand Down
8 changes: 4 additions & 4 deletions example-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
"otlp_blocking" : false,

"insecure" : true,
"no_tls_verify" : true,
"ca_file": "/dev/null",
"cert_file": "/dev/null",
"key_file": "/dev/null",
"tls_no_verify" : true,
"tls_ca_cert": "/dev/null",
"tls_client_key": "/dev/null",
"tls_client_cert": "/dev/null",

"service_name" : "configured_in_config_file",

Expand Down
13 changes: 7 additions & 6 deletions main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,23 +492,24 @@ func mkEnviron(endpoint string, env map[string]string, tlsData tlsHelpers) []str
}

// injectMapVars iterates over the map and updates the values, replacing all instances
// of {{endpoint}}, {{cacert}}, {{client_cert}}, and {{client_key}} with test values.
// of {{endpoint}}, {{tls_ca_cert}}, {{tls_client_cert}}, and {{tls_client_key}} with
// test values.
func injectMapVars(endpoint string, target map[string]string, tlsData tlsHelpers) {
for k, v := range target {
target[k] = injectVars(v, endpoint, tlsData)
}
}

// injectVars replaces all instances of {{endpoint}}, {{cacert}}, {{client_cert}}, and
// {{client_key}} with test values.
// injectVars replaces all instances of {{endpoint}}, {{tls_ca_cert}},
// {{tls_client_cert}}, and {{tls_client_key}} with test values.
// This is needed because the otlpserver is configured to listen on :0 which has it
// grab a random port. Once that's generated we need to inject it into all the values
// so the test comparisons work as expected. Similarly for TLS testing, a temp CA and
// certs are created and need to be injected.
func injectVars(in, endpoint string, tlsData tlsHelpers) string {
out := strings.ReplaceAll(in, "{{endpoint}}", endpoint)
out = strings.ReplaceAll(out, "{{cacert}}", tlsData.caFile)
out = strings.ReplaceAll(out, "{{client_cert}}", tlsData.clientFile)
out = strings.ReplaceAll(out, "{{client_key}}", tlsData.clientPrivKeyFile)
out = strings.ReplaceAll(out, "{{tls_ca_cert}}", tlsData.caFile)
out = strings.ReplaceAll(out, "{{tls_client_cert}}", tlsData.clientFile)
out = strings.ReplaceAll(out, "{{tls_client_key}}", tlsData.clientPrivKeyFile)
return out
}
58 changes: 41 additions & 17 deletions otelcli/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ func DefaultConfig() Config {
Headers: map[string]string{},
Insecure: false,
Blocking: false,
NoTlsVerify: false,
TlsNoVerify: false,
TlsCACert: "",
TlsClientKey: "",
TlsClientCert: "",
ServiceName: "otel-cli",
SpanName: "todo-generate-default-span-names",
Kind: "client",
Expand Down Expand Up @@ -59,18 +62,18 @@ func DefaultConfig() Config {
// This is used as a singleton as "config" and accessed from many other files.
// Data structure is public so that it can serialize to json easily.
type Config struct {
Endpoint string `json:"endpoint" env:"OTEL_EXPORTER_OTLP_ENDPOINT,OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"`
Protocol string `json:"protocol" env:"OTEL_EXPORTER_OTLP_PROTOCOL,OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"`
Timeout string `json:"timeout" env:"OTEL_EXPORTER_OTLP_TIMEOUT,OTEL_EXPORTER_OTLP_TRACES_TIMEOUT"`
Headers map[string]string `json:"otlp_headers" env:"OTEL_EXPORTER_OTLP_HEADERS"` // TODO: needs json marshaler hook to mask tokens
Insecure bool `json:"insecure" env:"OTEL_EXPORTER_OTLP_INSECURE"`
Blocking bool `json:"otlp_blocking" env:"OTEL_EXPORTER_OTLP_BLOCKING"`
NoTlsVerify bool `json:"no_tls_verify" env:"OTEL_CLI_NO_TLS_VERIFY"`

// json keys match the otel collector yaml
CACert string `json:"ca_file" env:"OTEL_EXPORTER_OTLP_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE"`
ClientKey string `json:"cert_file" env:"OTEL_EXPORTER_OTLP_CLIENT_KEY,OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY"`
ClientCert string `json:"key_file" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE"`
Endpoint string `json:"endpoint" env:"OTEL_EXPORTER_OTLP_ENDPOINT,OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"`
Protocol string `json:"protocol" env:"OTEL_EXPORTER_OTLP_PROTOCOL,OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"`
Timeout string `json:"timeout" env:"OTEL_EXPORTER_OTLP_TIMEOUT,OTEL_EXPORTER_OTLP_TRACES_TIMEOUT"`
Headers map[string]string `json:"otlp_headers" env:"OTEL_EXPORTER_OTLP_HEADERS"` // TODO: needs json marshaler hook to mask tokens
Insecure bool `json:"insecure" env:"OTEL_EXPORTER_OTLP_INSECURE"`
Blocking bool `json:"otlp_blocking" env:"OTEL_EXPORTER_OTLP_BLOCKING"`

TlsCACert string `json:"tls_ca_cert" env:"OTEL_EXPORTER_OTLP_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE"`
TlsClientKey string `json:"tls_client_key" env:"OTEL_EXPORTER_OTLP_CLIENT_KEY,OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY"`
TlsClientCert string `json:"tls_client_cert" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE"`
// OTEL_CLI_NO_TLS_VERIFY is deprecated and will be removed for 1.0
TlsNoVerify bool `json:"tls_no_verify" env:"OTEL_CLI_TLS_NO_VERIFY,OTEL_CLI_NO_TLS_VERIFY"`

ServiceName string `json:"service_name" env:"OTEL_CLI_SERVICE_NAME,OTEL_SERVICE_NAME"`
SpanName string `json:"span_name" env:"OTEL_CLI_SPAN_NAME"`
Expand Down Expand Up @@ -188,7 +191,10 @@ func (c Config) ToStringMap() map[string]string {
"headers": flattenStringMap(c.Headers, "{}"),
"insecure": strconv.FormatBool(c.Insecure),
"blocking": strconv.FormatBool(c.Blocking),
"no_tls_verify": strconv.FormatBool(c.NoTlsVerify),
"tls_no_verify": strconv.FormatBool(c.TlsNoVerify),
"tls_ca_cert": c.TlsCACert,
"tls_client_key": c.TlsClientKey,
"tls_client_cert": c.TlsClientCert,
"service_name": c.ServiceName,
"span_name": c.SpanName,
"span_kind": c.Kind,
Expand Down Expand Up @@ -249,9 +255,27 @@ func (c Config) WithBlocking(with bool) Config {
return c
}

// WithNoTlsVerify returns the config with NoTlsVerify set to the provided value.
func (c Config) WithNoTlsVerify(with bool) Config {
c.NoTlsVerify = with
// WithTlsNoVerify returns the config with NoTlsVerify set to the provided value.
func (c Config) WithTlsNoVerify(with bool) Config {
c.TlsNoVerify = with
return c
}

// WithTlsCACert returns the config with TlsCACert set to the provided value.
func (c Config) WithTlsCACert(with string) Config {
c.TlsCACert = with
return c
}

// WithTlsClientKey returns the config with NoTlsClientKey set to the provided value.
func (c Config) WithTlsClientKey(with string) Config {
c.TlsClientKey = with
return c
}

// WithTlsClientCert returns the config with NoTlsClientCert set to the provided value.
func (c Config) WithTlsClientCert(with string) Config {
c.TlsClientCert = with
return c
}

Expand Down
19 changes: 17 additions & 2 deletions otelcli/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,23 @@ func TestWithBlocking(t *testing.T) {
t.Fail()
}
}
func TestWithNoTlsVerify(t *testing.T) {
if DefaultConfig().WithNoTlsVerify(true).NoTlsVerify != true {
func TestWithTlsNoVerify(t *testing.T) {
if DefaultConfig().WithTlsNoVerify(true).TlsNoVerify != true {
t.Fail()
}
}
func TestWithTlsCACert(t *testing.T) {
if DefaultConfig().WithTlsCACert("/a/b/c").TlsCACert != "/a/b/c" {
t.Fail()
}
}
func TestWithTlsClientKey(t *testing.T) {
if DefaultConfig().WithTlsClientKey("/c/b/a").TlsClientKey != "/c/b/a" {
t.Fail()
}
}
func TestWithTlsClientCert(t *testing.T) {
if DefaultConfig().WithTlsClientCert("/b/c/a").TlsClientCert != "/b/c/a" {
t.Fail()
}
}
Expand Down
20 changes: 10 additions & 10 deletions otelcli/plumbing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,15 @@ func initTracer() (context.Context, func()) {
func tlsConfig() *tls.Config {
tlsConfig := &tls.Config{}

if config.NoTlsVerify {
if config.TlsNoVerify {
diagnostics.InsecureSkipVerify = true
tlsConfig.InsecureSkipVerify = true
}

// puts the provided CA certificate into the root pool
// when not provided, Go TLS will automatically load the system CA pool
if config.CACert != "" {
data, err := os.ReadFile(config.CACert)
if config.TlsCACert != "" {
data, err := os.ReadFile(config.TlsCACert)
if err != nil {
softFail("failed to load CA certificate: %s", err)
}
Expand All @@ -124,23 +124,23 @@ func tlsConfig() *tls.Config {
}

// client certificate authentication
if config.ClientCert != "" && config.ClientKey != "" {
clientPEM, err := os.ReadFile(config.ClientCert)
if config.TlsClientCert != "" && config.TlsClientKey != "" {
clientPEM, err := os.ReadFile(config.TlsClientCert)
if err != nil {
softFail("failed to read client certificate file %s: %s", config.ClientCert, err)
softFail("failed to read client certificate file %s: %s", config.TlsClientCert, err)
}
clientKeyPEM, err := os.ReadFile(config.ClientKey)
clientKeyPEM, err := os.ReadFile(config.TlsClientKey)
if err != nil {
softFail("failed to read client key file %s: %s", config.ClientKey, err)
softFail("failed to read client key file %s: %s", config.TlsClientKey, err)
}
certPair, err := tls.X509KeyPair(clientPEM, clientKeyPEM)
if err != nil {
softFail("failed to parse client cert pair: %s", err)
}
tlsConfig.Certificates = []tls.Certificate{certPair}
} else if config.ClientCert != "" {
} else if config.TlsClientCert != "" {
softFail("client cert and key must be specified together")
} else if config.ClientKey != "" {
} else if config.TlsClientKey != "" {
softFail("client cert and key must be specified together")
}

Expand Down
10 changes: 5 additions & 5 deletions otelcli/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ func addClientParams(cmd *cobra.Command) {
cmd.Flags().BoolVar(&config.Blocking, "otlp-blocking", defaults.Blocking, "block on connecting to the OTLP server before proceeding")

cmd.Flags().BoolVar(&config.Insecure, "insecure", defaults.Insecure, "allow connecting to cleartext endpoints")
cmd.Flags().StringVar(&config.CACert, "tls-ca-cert", defaults.CACert, "a file containing the certificate authority bundle")
cmd.Flags().StringVar(&config.ClientCert, "tls-client-cert", defaults.ClientCert, "a file containing the client certificate")
cmd.Flags().StringVar(&config.ClientKey, "tls-client-key", defaults.ClientKey, "a file containing the client certificate key")
cmd.Flags().BoolVar(&config.NoTlsVerify, "tls-no-verify", defaults.NoTlsVerify, "insecure! disables verification of the server certificate and name, mostly for self-signed CAs")
cmd.Flags().StringVar(&config.TlsCACert, "tls-ca-cert", defaults.TlsCACert, "a file containing the certificate authority bundle")
cmd.Flags().StringVar(&config.TlsClientCert, "tls-client-cert", defaults.TlsClientCert, "a file containing the client certificate")
cmd.Flags().StringVar(&config.TlsClientKey, "tls-client-key", defaults.TlsClientKey, "a file containing the client certificate key")
cmd.Flags().BoolVar(&config.TlsNoVerify, "tls-no-verify", defaults.TlsNoVerify, "insecure! disables verification of the server certificate and name, mostly for self-signed CAs")
// --no-tls-verify is deprecated, will remove before 1.0
cmd.Flags().BoolVar(&config.NoTlsVerify, "no-tls-verify", defaults.NoTlsVerify, "(deprecated) same as --tls-no-verify")
cmd.Flags().BoolVar(&config.TlsNoVerify, "no-tls-verify", defaults.TlsNoVerify, "(deprecated) same as --tls-no-verify")

// OTEL_CLI trace propagation options
cmd.Flags().BoolVar(&config.TraceparentRequired, "tp-required", defaults.TraceparentRequired, "when set to true, fail and log if a traceparent can't be picked up from TRACEPARENT ennvar or a carrier file")
Expand Down