Skip to content

Commit

Permalink
feat: Add opt-in config option for HTTP URL (#268)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
During event processing og HTTP request/response pairs, the HTTP URL is
added to the output event. This potentially could sensitive information
from the url path and or query string.

This PR adds an opt-in configuration option to enable HTTP URL capture. 
 
- Works toward #214 

## Short description of the change
- Adds new configuration option to determine whether HTTP URLs are
recorded in events, defaults to `false`
- Set the new config option by reading the new `INCLUDE_REQUEST_URL` env
var
- Use new config option during processing to decide if the HTTP URL is
added to libhoney event
- Add tests to check both the default config value is `false` and the
config can be updated by using the env var
- Add example of setting the env var in the smoke test deployment yaml

## How to verify that this has the expected result
The agent now requires user to opt-in to recording HTTP URLs.

---------

Co-authored-by: Jamie Danielson <jamieedanielson@gmail.com>
Co-authored-by: Robb Kidd <robbkidd@honeycomb.io>
  • Loading branch information
3 people committed Oct 4, 2023
1 parent d7b4d2a commit 8b41c0f
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 4 deletions.
4 changes: 4 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ type Config struct {

// Additional attributes to add to all events.
AdditionalAttributes map[string]string

// Include the request URL in the event.
IncludeRequestURL bool
}

// NewConfig returns a new Config struct.
Expand Down Expand Up @@ -139,6 +142,7 @@ func NewConfig() Config {
AgentPodIP: utils.LookupEnvOrString("AGENT_POD_IP", ""),
AgentPodName: utils.LookupEnvOrString("AGENT_POD_NAME", ""),
AdditionalAttributes: utils.LookupEnvAsStringMap("ADDITIONAL_ATTRIBUTES"),
IncludeRequestURL: utils.LookupEnvOrBool("INCLUDE_REQUEST_URL", false),
}
}

Expand Down
27 changes: 27 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config_test

import (
"os"
"testing"

"github.com/honeycombio/honeycomb-network-agent/config"
Expand Down Expand Up @@ -61,6 +62,7 @@ func TestEnvVars(t *testing.T) {
t.Setenv("AGENT_POD_IP", "pod_ip")
t.Setenv("AGENT_POD_NAME", "pod_name")
t.Setenv("ADDITIONAL_ATTRIBUTES", "key1=value1,key2=value2")
t.Setenv("INCLUDE_REQUEST_URL", "true")

config := config.NewConfig()
assert.Equal(t, "1234567890123456789012", config.APIKey)
Expand All @@ -76,4 +78,29 @@ func TestEnvVars(t *testing.T) {
assert.Equal(t, "pod_ip", config.AgentPodIP)
assert.Equal(t, "pod_name", config.AgentPodName)
assert.Equal(t, map[string]string{"key1": "value1", "key2": "value2"}, config.AdditionalAttributes)
assert.Equal(t, true, config.IncludeRequestURL)
}

func TestEnvVarsDefault(t *testing.T) {
// clear all env vars
// this doesn't reset the env vars for the test suite
// we could change to use os.Unsetenv() but that would require us to know and maontain
// all the env vars in an array
os.Clearenv()

config := config.NewConfig()
assert.Equal(t, "", config.APIKey)
assert.Equal(t, "https://api.honeycomb.io", config.Endpoint)
assert.Equal(t, "hny-network-agent", config.Dataset)
assert.Equal(t, "hny-network-agent-stats", config.StatsDataset)
assert.Equal(t, "INFO", config.LogLevel)
assert.Equal(t, false, config.Debug)
assert.Equal(t, "0.0.0.0:6060", config.DebugAddress)
assert.Equal(t, "", config.AgentNodeIP)
assert.Equal(t, "", config.AgentNodeName)
assert.Equal(t, "", config.AgentServiceAccount)
assert.Equal(t, "", config.AgentPodIP)
assert.Equal(t, "", config.AgentPodName)
assert.Equal(t, map[string]string{}, config.AdditionalAttributes)
assert.Equal(t, false, config.IncludeRequestURL)
}
6 changes: 4 additions & 2 deletions handlers/libhoney_event_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,14 @@ func (handler *libhoneyEventHandler) handleEvent(event assemblers.HttpEvent) {

// request attributes
if event.Request != nil {
requestURI = event.Request.RequestURI
ev.AddField("name", fmt.Sprintf("HTTP %s", event.Request.Method))
ev.AddField(string(semconv.HTTPRequestMethodKey), event.Request.Method)
ev.AddField(string(semconv.URLPathKey), requestURI)
ev.AddField(string(semconv.UserAgentOriginalKey), event.Request.Header.Get("User-Agent"))
ev.AddField(string(semconv.HTTPRequestBodySizeKey), event.Request.ContentLength)
if handler.config.IncludeRequestURL {
requestURI = event.Request.RequestURI
ev.AddField(string(semconv.URLPathKey), requestURI)
}
} else {
ev.AddField("name", "HTTP")
ev.AddField("http.request.missing", "no request on this event")
Expand Down
68 changes: 66 additions & 2 deletions handlers/libhoney_event_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,12 @@ func Test_libhoneyEventHandler_handleEvent(t *testing.T) {

wgTest := sync.WaitGroup{} // used to wait for the event handler to finish

testConfig := config.Config{
IncludeRequestURL: true,
}

// create the event handler with default config, fake k8s client & event channel then start it
handler := NewLibhoneyEventHandler(config.Config{}, fakeCachedK8sClient, eventsChannel, "test")
handler := NewLibhoneyEventHandler(testConfig, fakeCachedK8sClient, eventsChannel, "test")
wgTest.Add(1)
go handler.Start(cancelableCtx, &wgTest)

Expand Down Expand Up @@ -138,6 +142,63 @@ func Test_libhoneyEventHandler_handleEvent(t *testing.T) {
assert.Equal(t, expectedAttrs, attrs)
}

func Test_libhoneyEventHandler_handleEvent_doesNotSetUrlPath(t *testing.T) {
// Test Data - an assembled HTTP Event
httpEvent := assemblers.HttpEvent{
StreamIdent: "c->s:1->2",
Request: &http.Request{
Method: "GET",
RequestURI: "/check?teapot=true",
ContentLength: 42,
Header: http.Header{"User-Agent": []string{"teapot-checker/1.0"}},
},
Response: &http.Response{
StatusCode: 418,
ContentLength: 84,
},
SrcIp: "1.2.3.4",
DstIp: "5.6.7.8",
}

// create a fake k8s clientset with the test pod metadata and start the cached client with it
fakeCachedK8sClient := utils.NewCachedK8sClient(fake.NewSimpleClientset())
cancelableCtx, done := context.WithCancel(context.Background())
fakeCachedK8sClient.Start(cancelableCtx)

// create event channel used to pass in events to the handler
eventsChannel := make(chan assemblers.HttpEvent, 1)

wgTest := sync.WaitGroup{} // used to wait for the event handler to finish

defaultConfig := config.Config{
IncludeRequestURL: false,
}
// create the event handler with default config, fake k8s client & event channel then start it
handler := NewLibhoneyEventHandler(defaultConfig, fakeCachedK8sClient, eventsChannel, "test")
wgTest.Add(1)
go handler.Start(cancelableCtx, &wgTest)

// Setup libhoney for testing, use mock transmission to retrieve events "sent"
// must be done after the event handler is created
mockTransmission := setupTestLibhoney(t)

// TEST ACTION: pass in httpEvent to handler
eventsChannel <- httpEvent
time.Sleep(10 * time.Millisecond) // give the handler time to process the event

done()
wgTest.Wait()
handler.Close()

// VALIDATE
events := mockTransmission.Events()
assert.Equal(t, 1, len(events), "Expected 1 and only 1 event to be sent")

attrs := events[0].Data

assert.NotContains(t, attrs, "url.path")
}

func Test_libhoneyEventHandler_handleEvent_routed_to_service(t *testing.T) {
// TEST SETUP

Expand Down Expand Up @@ -198,8 +259,11 @@ func Test_libhoneyEventHandler_handleEvent_routed_to_service(t *testing.T) {

wgTest := sync.WaitGroup{} // used to wait for the event handler to finish

testConfig := config.Config{
IncludeRequestURL: true,
}
// create the event handler with default config, fake k8s client & event channel then start it
handler := NewLibhoneyEventHandler(config.Config{}, fakeCachedK8sClient, eventsChannel, "test")
handler := NewLibhoneyEventHandler(testConfig, fakeCachedK8sClient, eventsChannel, "test")
wgTest.Add(1)
go handler.Start(cancelableCtx, &wgTest)

Expand Down
3 changes: 3 additions & 0 deletions smoke-tests/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ spec:
## uncomment this to add extra attributes to all events
# - name: ADDITIONAL_ATTRIBUTES
# value: "key1=value1,key2=value2"
## uncomment this to enable including the request URL in events
# - name: INCLUDE_REQUEST_URL
# value: "true"
securityContext:
capabilities:
add:
Expand Down

0 comments on commit 8b41c0f

Please sign in to comment.