Skip to content

Commit

Permalink
Add auth token requirement for trace-agent set log_level (APMSP-1204) (
Browse files Browse the repository at this point in the history
…#31448)

Co-authored-by: Jen Gilbert <[email protected]>
  • Loading branch information
ajgajg1134 and jhgilbert authored Dec 3, 2024
1 parent dbbc50e commit e6017fd
Show file tree
Hide file tree
Showing 19 changed files with 130 additions and 49 deletions.
3 changes: 3 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@
# Additional notification to @iglendd about Agent Telemetry changes for optional approval and governance acknowledgement
/comp/core/agenttelemetry @DataDog/agent-shared-components @iglendd

# trace-agent logging implementation should also notify agent-apm
/comp/core/log/impl-trace @DataDog/agent-apm

# pkg
/pkg/ @DataDog/agent-shared-components
/pkg/api/ @DataDog/agent-shared-components
Expand Down
10 changes: 4 additions & 6 deletions cmd/trace-agent/test/testsuite/config_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import (
"testing"
"time"

"github.com/DataDog/datadog-agent/cmd/trace-agent/test"

"github.com/cihub/seelog"
"github.com/stretchr/testify/assert"

"github.com/DataDog/datadog-agent/cmd/trace-agent/test"
)

func TestConfigSetHandler(t *testing.T) {
func TestConfigSetHandlerUnauthenticated(t *testing.T) {
var r test.Runner
if err := r.Start(); err != nil {
t.Fatal(err)
Expand All @@ -41,8 +41,6 @@ func TestConfigSetHandler(t *testing.T) {
t.Fatal(err)
}

logstr = r.AgentLog()
assert.NotContains(t, logstr, "Switched log level to")
assert.Equal(t, 200, resp.StatusCode)
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
resp.Body.Close()
}
10 changes: 5 additions & 5 deletions comp/core/log/impl-trace/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,26 @@ replace (
)

require (
github.com/DataDog/datadog-agent/comp/core/config v0.56.0-rc.3
github.com/DataDog/datadog-agent/comp/core/config v0.59.0
github.com/DataDog/datadog-agent/comp/core/log/def v0.0.0-00010101000000-000000000000
github.com/DataDog/datadog-agent/pkg/config/env v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/trace v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/util/fxutil v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/fxutil v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/log v0.59.0
github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 // indirect; v2.6
github.com/stretchr/testify v1.10.0
go.uber.org/fx v1.23.0 // indirect
)

require (
github.com/DataDog/datadog-agent/comp/def v0.56.0-rc.3
github.com/DataDog/datadog-agent/comp/def v0.59.0
github.com/DataDog/datadog-agent/pkg/config/mock v0.59.0
github.com/DataDog/datadog-agent/pkg/util/log/setup v0.0.0-00010101000000-000000000000
)

require (
github.com/DataDog/datadog-agent/comp/core/flare/builder v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/comp/core/flare/types v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/comp/core/flare/builder v0.59.0 // indirect
github.com/DataDog/datadog-agent/comp/core/flare/types v0.59.0 // indirect
github.com/DataDog/datadog-agent/comp/core/secrets v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/collector/check/defaults v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/config/model v0.59.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion comp/otelcol/ddflareextension/impl/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ require (
github.com/DataDog/datadog-agent/comp/otelcol/ddflareextension/def v0.58.0
github.com/DataDog/datadog-agent/comp/otelcol/otlp/components/exporter/datadogexporter v0.59.0
github.com/DataDog/datadog-agent/comp/otelcol/otlp/components/processor/infraattributesprocessor v0.59.0
github.com/DataDog/datadog-agent/pkg/api v0.57.1
github.com/DataDog/datadog-agent/pkg/api v0.59.0
github.com/DataDog/datadog-agent/pkg/config/mock v0.59.0
github.com/DataDog/datadog-agent/pkg/version v0.59.0
github.com/google/go-cmp v0.6.0
Expand Down
8 changes: 4 additions & 4 deletions comp/otelcol/otlp/components/exporter/datadogexporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,14 @@ require (
github.com/DataDog/datadog-agent/comp/core/secrets v0.59.0 // indirect
github.com/DataDog/datadog-agent/comp/core/status v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/comp/core/telemetry v0.57.1 // indirect
github.com/DataDog/datadog-agent/comp/def v0.57.1 // indirect
github.com/DataDog/datadog-agent/comp/def v0.59.0 // indirect
github.com/DataDog/datadog-agent/comp/forwarder/defaultforwarder v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/comp/forwarder/orchestrator/orchestratorinterface v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/comp/logs/agent/config v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/comp/serializer/compression v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/comp/trace/compression/def v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/aggregator/ckey v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/api v0.57.1 // indirect
github.com/DataDog/datadog-agent/pkg/api v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/collector/check/defaults v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/config/env v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/config/mock v0.59.0 // indirect
Expand All @@ -155,7 +155,7 @@ require (
github.com/DataDog/datadog-agent/pkg/config/setup v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/config/structure v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/config/teeconfig v0.60.0-devel // indirect
github.com/DataDog/datadog-agent/pkg/config/utils v0.57.1 // indirect
github.com/DataDog/datadog-agent/pkg/config/utils v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/logs/auditor v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/logs/client v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/logs/diagnostic v0.56.0-rc.3 // indirect
Expand All @@ -182,7 +182,7 @@ require (
github.com/DataDog/datadog-agent/pkg/util/common v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/executable v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/filesystem v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/fxutil v0.57.1 // indirect
github.com/DataDog/datadog-agent/pkg/util/fxutil v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/hostname/validate v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/http v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/json v0.56.0-rc.3 // indirect
Expand Down
6 changes: 3 additions & 3 deletions comp/otelcol/otlp/components/statsprocessor/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ require (
github.com/DataDog/datadog-agent/pkg/obfuscate v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/cgroups v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/log v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/pointer v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/log v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/pointer v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.59.0 // indirect
github.com/DataDog/go-sqllexer v0.0.17 // indirect
github.com/DataDog/go-tuf v1.1.0-0.5.2 // indirect
github.com/DataDog/sketches-go v1.4.6 // indirect
Expand Down
13 changes: 6 additions & 7 deletions comp/trace/agent/impl/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ func runAgentSidekicks(ag component) error {
log.Errorf("could not set auth token: %s", err)
} else {
ag.Agent.DebugServer.AddRoute("/config", ag.config.GetConfigHandler())
api.AttachEndpoint(api.Endpoint{
Pattern: "/config/set",
Handler: func(_ *api.HTTPReceiver) http.Handler {
return ag.config.SetHandler()
},
})
}
if secrets, ok := ag.secrets.Get(); ok {
// Adding a route to trigger a secrets refresh from the CLI.
Expand All @@ -92,13 +98,6 @@ func runAgentSidekicks(ag component) error {
}))
}

api.AttachEndpoint(api.Endpoint{
Pattern: "/config/set",
Handler: func(_ *api.HTTPReceiver) http.Handler {
return ag.config.SetHandler()
},
})

log.Infof("Trace agent running on host %s", tracecfg.Hostname)
if pcfg := profilingConfig(tracecfg); pcfg != nil {
if err := profiling.Start(*pcfg); err != nil {
Expand Down
3 changes: 3 additions & 0 deletions comp/trace/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ func (c *cfg) SetHandler() http.Handler {
httpError(w, http.StatusMethodNotAllowed, fmt.Errorf("%s method not allowed, only %s", req.Method, http.MethodPost))
return
}
if apiutil.Validate(w, req) != nil {
return
}
for key, values := range req.URL.Query() {
if len(values) == 0 {
continue
Expand Down
32 changes: 32 additions & 0 deletions comp/trace/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2234,6 +2234,38 @@ func TestGetCoreConfigHandler(t *testing.T) {
assert.Contains(t, conf, "apm_config")
}

func TestSetConfigHandler(t *testing.T) {
config := buildConfigComponent(t, true, fx.Supply(corecomp.Params{}))

handler := config.SetHandler().ServeHTTP

// Refuse non POST query
resp := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/config", nil)
handler(resp, req)
assert.Equal(t, http.StatusMethodNotAllowed, resp.Code)

// Refuse missing auth token
resp = httptest.NewRecorder()
req = httptest.NewRequest("POST", "/config", nil)
handler(resp, req)
assert.Equal(t, http.StatusUnauthorized, resp.Code)

// Refuse invalid auth token
resp = httptest.NewRecorder()
req = httptest.NewRequest("POST", "/config", nil)
req.Header.Set("Authorization", "Bearer ABCDE")
handler(resp, req)
assert.Equal(t, http.StatusForbidden, resp.Code)

// Accept valid auth token return OK
resp = httptest.NewRecorder()
req = httptest.NewRequest("POST", "/config", nil)
req.Header.Set("Authorization", "Bearer "+apiutil.GetAuthToken())
handler(resp, req)
assert.Equal(t, http.StatusOK, resp.Code)
}

func TestDisableReceiverConfig(t *testing.T) {
config := buildConfigComponent(t, true, fx.Replace(corecomp.MockParams{
Params: corecomp.Params{ConfFilePath: "./testdata/disable_receiver.yaml"},
Expand Down
2 changes: 2 additions & 0 deletions comp/trace/config/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"go.opentelemetry.io/collector/component/componenttest"

apiutil "github.com/DataDog/datadog-agent/pkg/api/util"
"github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes"

corecompcfg "github.com/DataDog/datadog-agent/comp/core/config"
Expand Down Expand Up @@ -121,6 +122,7 @@ func prepareConfig(c corecompcfg.Component, tagger tagger.Component) (*config.Ag
return tagger.Tag(types.NewEntityID(types.ContainerID, cid), types.HighCardinality)
}
cfg.ContainerProcRoot = coreConfigObject.GetString("container_proc_root")
cfg.GetAgentAuthToken = apiutil.GetAuthToken
return cfg, nil
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ require (
github.com/DataDog/datadog-agent/comp/trace/compression/impl-gzip v0.59.0
github.com/DataDog/datadog-agent/comp/trace/compression/impl-zstd v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/aggregator/ckey v0.59.0-rc.6
github.com/DataDog/datadog-agent/pkg/api v0.57.1
github.com/DataDog/datadog-agent/pkg/api v0.59.0
github.com/DataDog/datadog-agent/pkg/collector/check/defaults v0.59.0
github.com/DataDog/datadog-agent/pkg/config/env v0.59.0
github.com/DataDog/datadog-agent/pkg/config/mock v0.59.0
Expand Down
4 changes: 4 additions & 0 deletions pkg/trace/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,10 @@ type AgentConfig struct {
// Azure container apps tags, in the form of a comma-separated list of
// key-value pairs, starting with a comma
AzureContainerAppTags string

// GetAgentAuthToken retrieves an auth token to communicate with other agent processes
// Function will be nil if in an environment without an auth token
GetAgentAuthToken func() string `json:"-"`
}

// RemoteClient client is used to APM Sampling Updates from a remote source.
Expand Down
6 changes: 3 additions & 3 deletions pkg/trace/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ require (
github.com/DataDog/datadog-agent/pkg/proto v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/util/cgroups v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/util/log v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/util/pointer v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/util/log v0.59.0
github.com/DataDog/datadog-agent/pkg/util/pointer v0.59.0
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.59.0
github.com/DataDog/datadog-go/v5 v5.5.0
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes v0.21.0
github.com/DataDog/sketches-go v1.4.6
Expand Down
28 changes: 26 additions & 2 deletions pkg/trace/remoteconfighandler/remote_config_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,21 @@ func (h *RemoteConfigHandler) onAgentConfigUpdate(updates map[string]state.RawCo
return
}

// todo refactor shared code

if len(mergedConfig.LogLevel) > 0 {
// Get the current log level
var newFallback seelog.LogLevel
newFallback, err = pkglog.GetLogLevel()
if err == nil {
h.configState.FallbackLogLevel = newFallback.String()
var resp *http.Response
resp, err = http.Post(fmt.Sprintf(h.configSetEndpointFormatString, mergedConfig.LogLevel), "", nil)
var req *http.Request
req, err = h.buildLogLevelRequest(mergedConfig.LogLevel)
if err != nil {
return
}
resp, err = http.DefaultClient.Do(req)
if err == nil {
resp.Body.Close()
h.configState.LatestLogLevel = mergedConfig.LogLevel
Expand All @@ -111,7 +118,12 @@ func (h *RemoteConfigHandler) onAgentConfigUpdate(updates map[string]state.RawCo
if err == nil && currentLogLevel.String() == h.configState.LatestLogLevel {
pkglog.Infof("Removing remote-config log level override of the trace-agent, falling back to %s", h.configState.FallbackLogLevel)
var resp *http.Response
resp, err = http.Post(fmt.Sprintf(h.configSetEndpointFormatString, h.configState.FallbackLogLevel), "", nil)
var req *http.Request
req, err = h.buildLogLevelRequest(h.configState.FallbackLogLevel)
if err != nil {
return
}
resp, err = http.DefaultClient.Do(req)
if err == nil {
resp.Body.Close()
}
Expand All @@ -135,6 +147,18 @@ func (h *RemoteConfigHandler) onAgentConfigUpdate(updates map[string]state.RawCo
}
}

func (h *RemoteConfigHandler) buildLogLevelRequest(newLevel string) (*http.Request, error) {
req, err := http.NewRequest(http.MethodPost, fmt.Sprintf(h.configSetEndpointFormatString, newLevel), nil)
if err != nil {
pkglog.Infof("Failed to build request to change log level of the trace-agent to %s through remote config", newLevel)
return nil, err
}
if h.agentConfig.GetAgentAuthToken != nil {
req.Header.Set("Authorization", "Bearer "+h.agentConfig.GetAgentAuthToken())
}
return req, nil
}

func (h *RemoteConfigHandler) onUpdate(update map[string]state.RawConfig, _ func(string, state.ApplyStatus)) {
if len(update) == 0 {
log.Debugf("no samplers configuration in remote config update payload")
Expand Down
13 changes: 9 additions & 4 deletions pkg/trace/remoteconfighandler/remote_config_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import (
"strings"
"testing"

"github.com/cihub/seelog"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"

"github.com/DataDog/datadog-agent/pkg/remoteconfig/state"
"github.com/DataDog/datadog-agent/pkg/remoteconfig/state/products/apmsampling"
"github.com/DataDog/datadog-agent/pkg/trace/config"
pkglog "github.com/DataDog/datadog-agent/pkg/util/log"
"github.com/DataDog/datadog-agent/pkg/util/pointer"
"github.com/cihub/seelog"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
)

// nolint: revive
Expand Down Expand Up @@ -193,7 +194,8 @@ func TestLogLevel(t *testing.T) {
rareSampler := NewMockrareSampler(ctrl)

pkglog.SetupLogger(seelog.Default, "debug")
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "Bearer fakeToken", r.Header.Get("Authorization"))
w.WriteHeader(200)
}))
defer srv.Close()
Expand All @@ -204,6 +206,9 @@ func TestLogLevel(t *testing.T) {
DefaultEnv: "agent-env",
ReceiverHost: "127.0.0.1",
ReceiverPort: port,
GetAgentAuthToken: func() string {
return "fakeToken"
},
}
h := New(&agentConfig, prioritySampler, rareSampler, errorsSampler)

Expand Down
6 changes: 3 additions & 3 deletions pkg/trace/stats/oteltest/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ require (
github.com/DataDog/datadog-agent/pkg/obfuscate v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/cgroups v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/log v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/pointer v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.56.0-rc.3 // indirect
github.com/DataDog/datadog-agent/pkg/util/log v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/pointer v0.59.0 // indirect
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.59.0 // indirect
github.com/DataDog/go-sqllexer v0.0.17 // indirect
github.com/DataDog/go-tuf v1.1.0-0.5.2 // indirect
github.com/DataDog/sketches-go v1.4.6 // indirect
Expand Down
11 changes: 11 additions & 0 deletions releasenotes/notes/traceagent-loglevel-027cc2c0e581b195.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
enhancements:
- |
APM: The trace agent endpoint for changing the configured log level now requires authentication so it is only accessible to other Agent processes.
6 changes: 3 additions & 3 deletions test/new-e2e/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ require (
github.com/DataDog/agent-payload/v5 v5.0.137
github.com/DataDog/datadog-agent/comp/otelcol/ddflareextension/def v0.56.2
github.com/DataDog/datadog-agent/pkg/util/optional v0.59.0
github.com/DataDog/datadog-agent/pkg/util/pointer v0.56.2
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.56.2
github.com/DataDog/datadog-agent/pkg/util/pointer v0.59.0
github.com/DataDog/datadog-agent/pkg/util/scrubber v0.59.0
github.com/DataDog/datadog-agent/pkg/util/testutil v0.56.2
github.com/DataDog/datadog-agent/pkg/version v0.56.0-rc.3
github.com/DataDog/datadog-agent/pkg/version v0.59.0
github.com/DataDog/datadog-agent/test/fakeintake v0.56.0-rc.3
github.com/DataDog/datadog-api-client-go v1.16.0
github.com/DataDog/datadog-api-client-go/v2 v2.31.0
Expand Down
Loading

0 comments on commit e6017fd

Please sign in to comment.