Skip to content

Commit 0271012

Browse files
jackgopack4mx-psi
authored and
zeck-ops
committed
[exporter/datadog] add basic API key validation on startup (open-telemetry#36510)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description adds basic hexadecimal character validation to Datadog API key on startup <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#36509 <!--Describe what testing was performed and which tests were added.--> #### Testing new "invalid API Key" test <!--Describe the documentation added.--> #### Documentation changelog file <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Pablo Baeyens <[email protected]>
1 parent ee8441d commit 0271012

13 files changed

+74
-28
lines changed

‎.chloggen/dd-config-api-key-fix.yaml

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: connector/datadog, exporter/datadog, pkg/datadog
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: throw error if datadog API key contains invalid characters
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [36509]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

‎connector/datadogconnector/example_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
)
2323

2424
func TestExamples(t *testing.T) {
25-
t.Setenv("DD_API_KEY", "testvalue")
25+
t.Setenv("DD_API_KEY", "aaaaaaaaa")
2626
factories := newTestComponents(t)
2727
const configFile = "./examples/config.yaml"
2828
// https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33594

‎exporter/datadogexporter/examples/k8s-chart/configmap.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ data:
5656
exporters:
5757
datadog:
5858
api:
59-
key: <YOUR_API_KEY_HERE>
59+
key: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" # Change this to your Datadog API key
6060
site: datadoghq.com # Change this to your site if not using the default
6161
processors:
6262
resourcedetection:

‎exporter/datadogexporter/examples_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestExamples(t *testing.T) {
5353
continue
5454
}
5555
t.Run(filepath.Base(f.Name()), func(t *testing.T) {
56-
t.Setenv("DD_API_KEY", "testvalue")
56+
t.Setenv("DD_API_KEY", "aaaaaaaaa")
5757
name := filepath.Join(folder, f.Name())
5858
// https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33594
5959
// nolint:staticcheck

‎exporter/datadogexporter/factory_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ func TestOnlyMetadata(t *testing.T) {
300300
BackOffConfig: configretry.NewDefaultBackOffConfig(),
301301
QueueSettings: exporterhelper.NewDefaultQueueConfig(),
302302

303-
API: APIConfig{Key: "notnull"},
303+
API: APIConfig{Key: "aaaaaaa"},
304304
Metrics: MetricsConfig{TCPAddrConfig: confignet.TCPAddrConfig{Endpoint: server.URL}},
305305
Traces: TracesConfig{TCPAddrConfig: confignet.TCPAddrConfig{Endpoint: server.URL}},
306306
OnlyMetadata: true,

‎exporter/datadogexporter/integrationtest/integration_test_config.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ exporters:
3030
verbosity: detailed
3131
datadog:
3232
api:
33-
key: "key"
33+
key: "aaa"
3434
tls:
3535
insecure_skip_verify: true
3636
host_metadata:

‎exporter/datadogexporter/integrationtest/integration_test_internal_metrics_config.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ receivers:
1717
exporters:
1818
datadog:
1919
api:
20-
key: "key"
20+
key: "aaa"
2121
tls:
2222
insecure_skip_verify: true
2323
host_metadata:

‎exporter/datadogexporter/integrationtest/integration_test_logs_config.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ receivers:
2121
exporters:
2222
datadog:
2323
api:
24-
key: "key"
24+
key: "aaa"
2525
tls:
2626
insecure_skip_verify: true
2727
host_metadata:

‎exporter/datadogexporter/integrationtest/integration_test_toplevel_config.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ exporters:
1717
verbosity: detailed
1818
datadog:
1919
api:
20-
key: "key"
20+
key: "aaa"
2121
tls:
2222
insecure_skip_verify: true
2323
host_metadata:

‎exporter/datadogexporter/metrics_exporter_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func TestNewExporter(t *testing.T) {
4343

4444
cfg := &Config{
4545
API: APIConfig{
46-
Key: "ddog_32_characters_long_api_key1",
46+
Key: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
4747
},
4848
Metrics: MetricsConfig{
4949
TCPAddrConfig: confignet.TCPAddrConfig{
@@ -424,7 +424,7 @@ func TestNewExporter_Zorkian(t *testing.T) {
424424

425425
cfg := &Config{
426426
API: APIConfig{
427-
Key: "ddog_32_characters_long_api_key1",
427+
Key: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
428428
},
429429
Metrics: MetricsConfig{
430430
TCPAddrConfig: confignet.TCPAddrConfig{

‎pkg/datadog/config/config.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package config // import "github.com/open-telemetry/opentelemetry-collector-cont
66
import (
77
"errors"
88
"fmt"
9+
"regexp"
910
"strings"
1011
"time"
1112

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

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

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

123-
if c.API.Key == "" {
130+
if string(c.API.Key) == "" {
124131
return ErrUnsetAPIKey
125132
}
126133

134+
invalidAPIKeyChars := NonHexRegex.FindAllString(string(c.API.Key), -1)
135+
if len(invalidAPIKeyChars) > 0 {
136+
return fmt.Errorf("%w: invalid characters: %s", ErrAPIKeyFormat, strings.Join(invalidAPIKeyChars, ", "))
137+
}
138+
127139
if err := c.Traces.Validate(); err != nil {
128140
return err
129141
}

‎pkg/datadog/config/config_test.go

+23-16
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,17 @@ func TestValidate(t *testing.T) {
4444
},
4545
err: ErrUnsetAPIKey.Error(),
4646
},
47+
{
48+
name: "invalid format api::key",
49+
cfg: &Config{
50+
API: APIConfig{Key: "'aaaaaaa"},
51+
},
52+
err: ErrAPIKeyFormat.Error(),
53+
},
4754
{
4855
name: "invalid hostname",
4956
cfg: &Config{
50-
API: APIConfig{Key: "notnull"},
57+
API: APIConfig{Key: "aaaaaaa"},
5158
TagsConfig: TagsConfig{Hostname: "invalid_host"},
5259
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
5360
},
@@ -56,7 +63,7 @@ func TestValidate(t *testing.T) {
5663
{
5764
name: "no metadata",
5865
cfg: &Config{
59-
API: APIConfig{Key: "notnull"},
66+
API: APIConfig{Key: "aaaaaaa"},
6067
OnlyMetadata: true,
6168
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
6269
},
@@ -65,15 +72,15 @@ func TestValidate(t *testing.T) {
6572
{
6673
name: "span name remapping valid",
6774
cfg: &Config{
68-
API: APIConfig{Key: "notnull"},
75+
API: APIConfig{Key: "aaaaaaa"},
6976
Traces: TracesExporterConfig{TracesConfig: TracesConfig{SpanNameRemappings: map[string]string{"old.opentelemetryspan.name": "updated.name"}}},
7077
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
7178
},
7279
},
7380
{
7481
name: "span name remapping empty val",
7582
cfg: &Config{
76-
API: APIConfig{Key: "notnull"},
83+
API: APIConfig{Key: "aaaaaaa"},
7784
Traces: TracesExporterConfig{TracesConfig: TracesConfig{SpanNameRemappings: map[string]string{"oldname": ""}}},
7885
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
7986
},
@@ -82,7 +89,7 @@ func TestValidate(t *testing.T) {
8289
{
8390
name: "span name remapping empty key",
8491
cfg: &Config{
85-
API: APIConfig{Key: "notnull"},
92+
API: APIConfig{Key: "aaaaaaa"},
8693
Traces: TracesExporterConfig{TracesConfig: TracesConfig{SpanNameRemappings: map[string]string{"": "newname"}}},
8794
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
8895
},
@@ -91,15 +98,15 @@ func TestValidate(t *testing.T) {
9198
{
9299
name: "ignore resources valid",
93100
cfg: &Config{
94-
API: APIConfig{Key: "notnull"},
101+
API: APIConfig{Key: "aaaaaaa"},
95102
Traces: TracesExporterConfig{TracesConfig: TracesConfig{IgnoreResources: []string{"[123]"}}},
96103
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
97104
},
98105
},
99106
{
100107
name: "ignore resources missing bracket",
101108
cfg: &Config{
102-
API: APIConfig{Key: "notnull"},
109+
API: APIConfig{Key: "aaaaaaa"},
103110
Traces: TracesExporterConfig{TracesConfig: TracesConfig{IgnoreResources: []string{"[123"}}},
104111
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
105112
},
@@ -108,7 +115,7 @@ func TestValidate(t *testing.T) {
108115
{
109116
name: "invalid histogram settings",
110117
cfg: &Config{
111-
API: APIConfig{Key: "notnull"},
118+
API: APIConfig{Key: "aaaaaaa"},
112119
Metrics: MetricsConfig{
113120
HistConfig: HistogramConfig{
114121
Mode: HistogramModeNoBuckets,
@@ -122,7 +129,7 @@ func TestValidate(t *testing.T) {
122129
{
123130
name: "TLS settings are valid",
124131
cfg: &Config{
125-
API: APIConfig{Key: "notnull"},
132+
API: APIConfig{Key: "aaaaaaa"},
126133
ClientConfig: confighttp.ClientConfig{
127134
TLSSetting: configtls.ClientConfig{
128135
InsecureSkipVerify: true,
@@ -134,15 +141,15 @@ func TestValidate(t *testing.T) {
134141
{
135142
name: "With trace_buffer",
136143
cfg: &Config{
137-
API: APIConfig{Key: "notnull"},
144+
API: APIConfig{Key: "aaaaaaa"},
138145
Traces: TracesExporterConfig{TraceBuffer: 10},
139146
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 10 * time.Minute},
140147
},
141148
},
142149
{
143150
name: "With peer_tags",
144151
cfg: &Config{
145-
API: APIConfig{Key: "notnull"},
152+
API: APIConfig{Key: "aaaaaaa"},
146153
Traces: TracesExporterConfig{
147154
TracesConfig: TracesConfig{
148155
PeerTags: []string{"tag1", "tag2"},
@@ -154,7 +161,7 @@ func TestValidate(t *testing.T) {
154161
{
155162
name: "With confighttp client configs",
156163
cfg: &Config{
157-
API: APIConfig{Key: "notnull"},
164+
API: APIConfig{Key: "aaaaaaa"},
158165
ClientConfig: confighttp.ClientConfig{
159166
ReadBufferSize: 100,
160167
WriteBufferSize: 200,
@@ -173,7 +180,7 @@ func TestValidate(t *testing.T) {
173180
{
174181
name: "unsupported confighttp client configs",
175182
cfg: &Config{
176-
API: APIConfig{Key: "notnull"},
183+
API: APIConfig{Key: "aaaaaaa"},
177184
ClientConfig: confighttp.ClientConfig{
178185
Endpoint: "endpoint",
179186
Compression: "gzip",
@@ -189,7 +196,7 @@ func TestValidate(t *testing.T) {
189196
{
190197
name: "Invalid reporter_period",
191198
cfg: &Config{
192-
API: APIConfig{Key: "notnull"},
199+
API: APIConfig{Key: "abcdef0"},
193200
HostMetadata: HostMetadataConfig{Enabled: true, ReporterPeriod: 4 * time.Minute},
194201
},
195202
err: "reporter_period must be 5 minutes or higher",
@@ -199,7 +206,7 @@ func TestValidate(t *testing.T) {
199206
t.Run(testInstance.name, func(t *testing.T) {
200207
err := testInstance.cfg.Validate()
201208
if testInstance.err != "" {
202-
assert.EqualError(t, err, testInstance.err)
209+
assert.ErrorContains(t, err, testInstance.err)
203210
} else {
204211
assert.NoError(t, err)
205212
}
@@ -649,7 +656,7 @@ func TestLoadConfig(t *testing.T) {
649656
BackOffConfig: configretry.NewDefaultBackOffConfig(),
650657
QueueSettings: exporterhelper.NewDefaultQueueConfig(),
651658
API: APIConfig{
652-
Key: "key",
659+
Key: "abc",
653660
Site: "datadoghq.com",
654661
FailOnInvalidKey: false,
655662
},

‎pkg/datadog/config/testdata/config.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ datadog/default:
4040

4141
datadog/customReporterPeriod:
4242
api:
43-
key: key
43+
key: abc
4444
host_metadata:
4545
enabled: true
4646
reporter_period: 10m

0 commit comments

Comments
 (0)