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.

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

[exporter/datadog] add basic API key validation on startup #36510

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 27 additions & 0 deletions .chloggen/dd-config-api-key-fix.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: connector/datadog, exporter/datadog, pkg/datadog

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: throw error if datadog API key contains invalid characters

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [36509]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
2 changes: 1 addition & 1 deletion connector/datadogconnector/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
)

func TestExamples(t *testing.T) {
t.Setenv("DD_API_KEY", "testvalue")
t.Setenv("DD_API_KEY", "aaaaaaaaa")
factories := newTestComponents(t)
const configFile = "./examples/config.yaml"
// https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33594
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ data:
exporters:
datadog:
api:
key: <YOUR_API_KEY_HERE>
key: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # Change this to your Datadog API key
site: datadoghq.com # Change this to your site if not using the default
processors:
resourcedetection:
Expand Down
2 changes: 1 addition & 1 deletion exporter/datadogexporter/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestExamples(t *testing.T) {
continue
}
t.Run(filepath.Base(f.Name()), func(t *testing.T) {
t.Setenv("DD_API_KEY", "testvalue")
t.Setenv("DD_API_KEY", "aaaaaaaaa")
name := filepath.Join(folder, f.Name())
// https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33594
// nolint:staticcheck
Expand Down
2 changes: 1 addition & 1 deletion exporter/datadogexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func TestOnlyMetadata(t *testing.T) {
BackOffConfig: configretry.NewDefaultBackOffConfig(),
QueueSettings: exporterhelper.NewDefaultQueueConfig(),

API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Metrics: MetricsConfig{TCPAddrConfig: confignet.TCPAddrConfig{Endpoint: server.URL}},
Traces: TracesConfig{TCPAddrConfig: confignet.TCPAddrConfig{Endpoint: server.URL}},
OnlyMetadata: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ exporters:
verbosity: detailed
datadog:
api:
key: "key"
key: "aaa"
tls:
insecure_skip_verify: true
host_metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ receivers:
exporters:
datadog:
api:
key: "key"
key: "aaa"
tls:
insecure_skip_verify: true
host_metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ receivers:
exporters:
datadog:
api:
key: "key"
key: "aaa"
tls:
insecure_skip_verify: true
host_metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ exporters:
verbosity: detailed
datadog:
api:
key: "key"
key: "aaa"
tls:
insecure_skip_verify: true
host_metadata:
Expand Down
4 changes: 2 additions & 2 deletions exporter/datadogexporter/metrics_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestNewExporter(t *testing.T) {

cfg := &Config{
API: APIConfig{
Key: "ddog_32_characters_long_api_key1",
Key: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
},
Metrics: MetricsConfig{
TCPAddrConfig: confignet.TCPAddrConfig{
Expand Down Expand Up @@ -424,7 +424,7 @@ func TestNewExporter_Zorkian(t *testing.T) {

cfg := &Config{
API: APIConfig{
Key: "ddog_32_characters_long_api_key1",
Key: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
},
Metrics: MetricsConfig{
TCPAddrConfig: confignet.TCPAddrConfig{
Expand Down
14 changes: 13 additions & 1 deletion pkg/datadog/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package config // import "github.com/open-telemetry/opentelemetry-collector-cont
import (
"errors"
"fmt"
"regexp"
"strings"
"time"

Expand All @@ -27,11 +28,17 @@ var (
ErrNoMetadata = errors.New("only_metadata can't be enabled when host_metadata::enabled = false or host_metadata::hostname_source != first_resource")
// ErrInvalidHostname is returned when the hostname is invalid.
ErrEmptyEndpoint = errors.New("endpoint cannot be empty")
// ErrAPIKeyFormat is returned if API key contains invalid characters
ErrAPIKeyFormat = errors.New("api::key contains invalid characters")
// NonHexRegex is a regex of characters that are always invalid in a Datadog API key
NonHexRegex = regexp.MustCompile(NonHexChars)
)

const (
// DefaultSite is the default site of the Datadog intake to send data to
DefaultSite = "datadoghq.com"
// NonHexChars is a regex of characters that are always invalid in a Datadog API key
NonHexChars = "[^0-9a-fA-F]"
)

// APIConfig defines the API configuration options
Expand Down Expand Up @@ -120,10 +127,15 @@ func (c *Config) Validate() error {
return fmt.Errorf("hostname field is invalid: %w", err)
}

if c.API.Key == "" {
if string(c.API.Key) == "" {
return ErrUnsetAPIKey
}

invalidAPIKeyChars := NonHexRegex.FindAllString(string(c.API.Key), -1)
if len(invalidAPIKeyChars) > 0 {
return fmt.Errorf("%w: invalid characters: %s", ErrAPIKeyFormat, strings.Join(invalidAPIKeyChars, ", "))
}

if err := c.Traces.Validate(); err != nil {
return err
}
Expand Down
39 changes: 23 additions & 16 deletions pkg/datadog/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,17 @@ func TestValidate(t *testing.T) {
},
err: ErrUnsetAPIKey.Error(),
},
{
name: "invalid format api::key",
jackgopack4 marked this conversation as resolved.
Show resolved Hide resolved
cfg: &Config{
API: APIConfig{Key: "'aaaaaaa"},
},
err: ErrAPIKeyFormat.Error(),
},
{
name: "invalid hostname",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
TagsConfig: TagsConfig{Hostname: "invalid_host"},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
Expand All @@ -56,7 +63,7 @@ func TestValidate(t *testing.T) {
{
name: "no metadata",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
OnlyMetadata: true,
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
Expand All @@ -65,15 +72,15 @@ func TestValidate(t *testing.T) {
{
name: "span name remapping valid",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Traces: TracesExporterConfig{TracesConfig: TracesConfig{SpanNameRemappings: map[string]string{"old.opentelemetryspan.name": "updated.name"}}},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
},
{
name: "span name remapping empty val",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Traces: TracesExporterConfig{TracesConfig: TracesConfig{SpanNameRemappings: map[string]string{"oldname": ""}}},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
Expand All @@ -82,7 +89,7 @@ func TestValidate(t *testing.T) {
{
name: "span name remapping empty key",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Traces: TracesExporterConfig{TracesConfig: TracesConfig{SpanNameRemappings: map[string]string{"": "newname"}}},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
Expand All @@ -91,15 +98,15 @@ func TestValidate(t *testing.T) {
{
name: "ignore resources valid",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Traces: TracesExporterConfig{TracesConfig: TracesConfig{IgnoreResources: []string{"[123]"}}},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
},
{
name: "ignore resources missing bracket",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Traces: TracesExporterConfig{TracesConfig: TracesConfig{IgnoreResources: []string{"[123"}}},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
Expand All @@ -108,7 +115,7 @@ func TestValidate(t *testing.T) {
{
name: "invalid histogram settings",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Metrics: MetricsConfig{
HistConfig: HistogramConfig{
Mode: HistogramModeNoBuckets,
Expand All @@ -122,7 +129,7 @@ func TestValidate(t *testing.T) {
{
name: "TLS settings are valid",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
ClientConfig: confighttp.ClientConfig{
TLSSetting: configtls.ClientConfig{
InsecureSkipVerify: true,
Expand All @@ -134,15 +141,15 @@ func TestValidate(t *testing.T) {
{
name: "With trace_buffer",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Traces: TracesExporterConfig{TraceBuffer: 10},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
},
},
{
name: "With peer_tags",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
Traces: TracesExporterConfig{
TracesConfig: TracesConfig{
PeerTags: []string{"tag1", "tag2"},
Expand All @@ -154,7 +161,7 @@ func TestValidate(t *testing.T) {
{
name: "With confighttp client configs",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
ClientConfig: confighttp.ClientConfig{
ReadBufferSize: 100,
WriteBufferSize: 200,
Expand All @@ -173,7 +180,7 @@ func TestValidate(t *testing.T) {
{
name: "unsupported confighttp client configs",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "aaaaaaa"},
ClientConfig: confighttp.ClientConfig{
Endpoint: "endpoint",
Compression: "gzip",
Expand All @@ -189,7 +196,7 @@ func TestValidate(t *testing.T) {
{
name: "Invalid reporter_period",
cfg: &Config{
API: APIConfig{Key: "notnull"},
API: APIConfig{Key: "abcdef0"},
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 4 * time.Minute},
},
err: "reporter_period must be 5 minutes or higher",
Expand All @@ -199,7 +206,7 @@ func TestValidate(t *testing.T) {
t.Run(testInstance.name, func(t *testing.T) {
err := testInstance.cfg.Validate()
if testInstance.err != "" {
assert.EqualError(t, err, testInstance.err)
assert.ErrorContains(t, err, testInstance.err)
} else {
assert.NoError(t, err)
}
Expand Down Expand Up @@ -649,7 +656,7 @@ func TestLoadConfig(t *testing.T) {
BackOffConfig: configretry.NewDefaultBackOffConfig(),
QueueSettings: exporterhelper.NewDefaultQueueConfig(),
API: APIConfig{
Key: "key",
Key: "abc",
Site: "datadoghq.com",
FailOnInvalidKey: false,
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/datadog/config/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ datadog/default:

datadog/customReporterPeriod:
api:
key: key
key: abc
host_metadata:
enabled: true
reporter_period: 10m
Loading