Skip to content

Commit d843e0e

Browse files
authored
[processor/resourcedetection] Fix consul TokenFile using path as token value (#46746)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description When `token_file` was configured, the file path string was assigned to `api.Config.Token` instead of `api.Config.TokenFile`, causing the consul API client to use the path as the authentication token. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #46745 <!--Describe what testing was performed and which tests were added.--> #### Testing Added unit tests. Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
1 parent aae5636 commit d843e0e

3 files changed

Lines changed: 105 additions & 4 deletions

File tree

.chloggen/fix_46745.yaml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
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. receiver/filelog)
7+
component: "processor/resourcedetection"
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Fix consul detector `token_file` setting the file path as the literal token value instead of configuring the consul SDK to read the file"
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: [46745]
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+
When `token_file` was configured, the file path string
20+
was assigned to `api.Config.Token` instead of `api.Config.TokenFile`,
21+
causing the consul API client to use the path as the authentication
22+
token (always resulting in 403 Forbidden)."
23+
24+
# If your change doesn't affect end users or the exported elements of any package,
25+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
26+
# Optional: The change log or logs in which this entry should be included.
27+
# e.g. '[user]' or '[user, api]'
28+
# Include 'user' if the change is relevant to end users.
29+
# Include 'api' if there is a change to a library API.
30+
# Default: '[user]'
31+
change_logs: [user]

processor/resourcedetectionprocessor/internal/consul/consul.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ type Detector struct {
3232
rb *metadata.ResourceBuilder
3333
}
3434

35-
// NewDetector creates a new system metadata detector
36-
func NewDetector(p processor.Settings, dcfg internal.DetectorConfig) (internal.Detector, error) {
37-
userCfg := dcfg.(Config)
35+
// buildConsulAPIConfig translates the detector Config into a hashicorp consul api.Config.
36+
func buildConsulAPIConfig(userCfg Config) *api.Config {
3837
cfg := api.DefaultConfig()
3938

4039
if userCfg.Address != "" {
@@ -50,9 +49,16 @@ func NewDetector(p processor.Settings, dcfg internal.DetectorConfig) (internal.D
5049
cfg.Token = string(userCfg.Token)
5150
}
5251
if userCfg.TokenFile != "" {
53-
cfg.Token = userCfg.TokenFile
52+
cfg.TokenFile = userCfg.TokenFile
5453
}
5554

55+
return cfg
56+
}
57+
58+
func NewDetector(p processor.Settings, dcfg internal.DetectorConfig) (internal.Detector, error) {
59+
userCfg := dcfg.(Config)
60+
cfg := buildConsulAPIConfig(userCfg)
61+
5662
client, err := api.NewClient(cfg)
5763
if err != nil {
5864
return nil, fmt.Errorf("failed creating consul client: %w", err)

processor/resourcedetectionprocessor/internal/consul/consul_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ package consul
55

66
import (
77
"context"
8+
"os"
9+
"path/filepath"
810
"testing"
911

12+
"github.com/hashicorp/consul/api"
1013
"github.com/stretchr/testify/assert"
1114
"github.com/stretchr/testify/mock"
1215
"github.com/stretchr/testify/require"
@@ -58,3 +61,64 @@ func TestDetect(t *testing.T) {
5861

5962
assert.Equal(t, expected, res.Attributes().AsRaw())
6063
}
64+
65+
func TestBuildConsulAPIConfig_TokenFile(t *testing.T) {
66+
tokenFile := filepath.Join(t.TempDir(), "token")
67+
require.NoError(t, os.WriteFile(tokenFile, []byte("secret-token-value"), 0o600))
68+
69+
userCfg := Config{
70+
TokenFile: tokenFile,
71+
ResourceAttributes: metadata.DefaultResourceAttributesConfig(),
72+
}
73+
74+
apiCfg := buildConsulAPIConfig(userCfg)
75+
76+
// TokenFile must be set on api.Config.TokenFile so the consul SDK reads the file.
77+
// It must NOT be set on api.Config.Token (which would use the path as a literal token).
78+
require.Equal(t, tokenFile, apiCfg.TokenFile,
79+
"TokenFile should be passed to api.Config.TokenFile")
80+
require.Empty(t, apiCfg.Token,
81+
"api.Config.Token should not be set when only TokenFile is configured")
82+
}
83+
84+
func TestBuildConsulAPIConfig_Token(t *testing.T) {
85+
userCfg := Config{
86+
Token: "my-secret-token",
87+
ResourceAttributes: metadata.DefaultResourceAttributesConfig(),
88+
}
89+
90+
apiCfg := buildConsulAPIConfig(userCfg)
91+
92+
require.Equal(t, "my-secret-token", apiCfg.Token)
93+
require.Empty(t, apiCfg.TokenFile)
94+
}
95+
96+
func TestBuildConsulAPIConfig_AllFields(t *testing.T) {
97+
userCfg := Config{
98+
Address: "http://consul.local:8500",
99+
Datacenter: "dc2",
100+
Namespace: "team-a",
101+
Token: "direct-token",
102+
ResourceAttributes: metadata.DefaultResourceAttributesConfig(),
103+
}
104+
105+
apiCfg := buildConsulAPIConfig(userCfg)
106+
107+
require.Equal(t, "http://consul.local:8500", apiCfg.Address)
108+
require.Equal(t, "dc2", apiCfg.Datacenter)
109+
require.Equal(t, "team-a", apiCfg.Namespace)
110+
require.Equal(t, "direct-token", apiCfg.Token)
111+
}
112+
113+
func TestBuildConsulAPIConfig_Defaults(t *testing.T) {
114+
userCfg := Config{
115+
ResourceAttributes: metadata.DefaultResourceAttributesConfig(),
116+
}
117+
118+
apiCfg := buildConsulAPIConfig(userCfg)
119+
120+
// When no fields are set, api.DefaultConfig() defaults should be preserved
121+
defaultCfg := api.DefaultConfig()
122+
require.Equal(t, defaultCfg.Address, apiCfg.Address)
123+
require.Equal(t, defaultCfg.Datacenter, apiCfg.Datacenter)
124+
}

0 commit comments

Comments
 (0)