maint: Refactor Config (#197)

## Which problem is this PR solving?
Config options were scattered through the codebase. This PR centralises
all configuration options into the Config package and updates usage to
be consistent.

## Short description of the changes
- Move config vars into NewConfig
- Remove flags.* usage
- Add descriptions to Config struct fields
- Move stats dataset name from assembler to config
- Add stats dataset config option and allow to read from env var
- Removes unused config options - max, statsevery, verbose, debug,
quiet, fname and tstype
- Add config validate func with Missing and Invalid error types
- Add GetMaskedAPIKey func to config so we can safely log it
- Add tests for key masking
- Add env util for retrieving string and bool env vars with a default
- Reorganise main.go, includes setup of logging, libhoney and k8s into
their own funcs
- Update README to include the env vars we currently support
- Add test and docker-test makefile targets
- Update main and pr github workflows to execute tests using new
makefile targets

## How to verify that this has the expected result
Config usage is much cleaner and easier to reason about.

Environment variables set in the k8s deployment are used when
configuring the agent. For example, you can set
`HONEYCOMB_DATASET=some-other-place` under env.

---------

Co-authored-by: Robb Kidd <robbkidd@honeycomb.io>
Co-authored-by: JamieDanielson <jamieedanielson@gmail.com>

.env.example

@@ -1,3 +1,6 @@
 HONEYCOMB_API_KEY=your-api-key
 GITHUB_TOKEN=githubusername:githubaccesstoken
 BASE64_TOKEN=$(echo -n $GITHUB_TOKEN | base64)
+HONEYCOMB_API_ENDPOINT=https://api.honeycomb.io
+HONEYCOMB_DATASET=k8s-network-agent
+HONEYCOMB_STATS_DATASET=k8s-network-agent-stats

.github/workflows/main.yaml

@@ -13,6 +13,9 @@ jobs:
     steps:
       - uses: actions/checkout@v3.5.3
 
+      - name: Run tests
+        run: make docker-test
+
       - name: Log in to the Container registry
         uses: docker/login-action@v2.2.0
         with:

.github/workflows/pr.yaml

@@ -10,6 +10,9 @@ jobs:
       - name: Checkout Repo
         uses: actions/checkout@v3.5.3
 
+      - name: Run tests
+        run: make docker-test
+
       - name: Set up QEMU
         uses: docker/setup-qemu-action@v2.2.0
 

Dockerfile.test

@@ -0,0 +1,7 @@
+FROM golang:1.20 as builder
+RUN apt update -yq && apt install -yq clang llvm make libpcap-dev
+WORKDIR /src
+COPY go.* .
+RUN go mod download
+COPY . .
+RUN make test

Makefile

@@ -10,6 +10,16 @@ build:
 docker-build:
 	docker build --tag $(IMG_NAME):$(IMG_TAG) .
 
+.PHONY: test
+#: run unit tests
+test:
+	go test ./...
+
+.PHONY: docker-test
+#: run unit tests in docker
+docker-test:
+	docker build -f Dockerfile.test .
+
 ### Testing targets
 
 .PHONY: apply-agent

README.md

@@ -45,6 +45,21 @@ kubectl create secret docker-registry ghcr-secret \
   --namespace=honeycomb
 ```
 
+
+### Configuration
+
+The network agent can be configured using the following environment variables.
+
+| Environment Variable | Description | Default | Required |
+| -------------------- | ----------- | ------- | -------- |
+| `HONEYCOMB_API_KEY` | The Honeycomb API key used when sending events | `` (empty) | `true` |
+| `HONEYCOMB_API_ENDPOINT` | The endpoint to send events to | `https://api.honeycomb.io` | `false` |
+| `HONEYCOMB_DATASET` | Dataset where network events are stored | `hny-network-agent` | `false` |
+| `HONEYCOMB_STATS_DATASET` | Dataset where operational statistics for the network agent are stored  | `hny-network-agent-stats` | `false` |
+| `LOG_LEVEL`  | The log level to use when printing logs to console | `INFO` | `false` |
+| `DEBUG` | Runs the agent in debug mode including enabling a profiling endpoint using Debug Address  | `false` | `false` |
+| `DEBUG_ADDRESS` | The endpoint to listen to when running the profile endpoint | `localhost:6060` | `false` |
+
 ### Run
 
 ```sh

assemblers/tcp_assembler.go

@@ -225,7 +225,7 @@ func (a *tcpAssembler) logAssemblerStats() {
 		"active_streams":        stats.active_streams,
 	}
 	statsEvent := libhoney.NewEvent()
-	statsEvent.Dataset = "hny-network-agent-stats"
+	statsEvent.Dataset = a.config.StatsDataset
 	statsEvent.AddField("name", "tcp_assembler_stats")
 	statsEvent.Add(statsFields)
 	statsEvent.Send()

config/config.go

@@ -1,98 +1,135 @@
 package config
 
 import (
-	"encoding/json"
-	"flag"
+	"errors"
 	"strings"
 	"time"
 
-	"github.com/rs/zerolog/log"
+	"github.com/honeycombio/honeycomb-network-agent/utils"
+	"github.com/honeycombio/libhoney-go"
 )
 
-// TODO hard-coded for now, make configurable
-const DebugAddr = "0.0.0.0:6060"
-
-var maxcount = flag.Int("c", -1, "Only grab this many packets, then exit")
-var statsevery = flag.Int("stats", 1000, "Output statistics every N packets")
-var lazy = flag.Bool("lazy", false, "If true, do lazy decoding")
-var nodefrag = flag.Bool("nodefrag", false, "If true, do not do IPv4 defrag")
-var checksum = flag.Bool("checksum", false, "Check TCP checksum")
-var nooptcheck = flag.Bool("nooptcheck", true, "Do not check TCP options (useful to ignore MSS on captures with TSO)")
-var ignorefsmerr = flag.Bool("ignorefsmerr", true, "Ignore TCP FSM errors")
-var allowmissinginit = flag.Bool("allowmissinginit", true, "Support streams without SYN/SYN+ACK/ACK sequence")
-var verbose = flag.Bool("verbose", false, "Be verbose")
-var debug = flag.Bool("debug", false, "Display debug information")
-var quiet = flag.Bool("quiet", false, "Be quiet regarding errors")
-
-// capture
-var iface = flag.String("i", "any", "Interface to read packets from")
-var fname = flag.String("r", "", "Filename to read from, overrides -i")
-var snaplen = flag.Int("s", 262144, "Snap length (number of bytes max to read per packet") // 262144 is the default snaplen for tcpdump
-var tstype = flag.String("timestamp_type", "", "Type of timestamps to use")
-var promisc = flag.Bool("promisc", true, "Set promiscuous mode")
-var packetSource = flag.String("source", "pcap", "Packet source (defaults to pcap)")
-var bpfFilter = flag.String("filter", "tcp", "BPF filter")
-var channelBufferSize = flag.Int("channel_buffer_size", 1000, "Channel buffer size (defaults to 1000)")
-var streamFlushTimeout = flag.Int("stream_flush_timeout", 10, "Stream flush timeout in seconds (defaults to 10)")
-var streamCloseTimeout = flag.Int("stream_close_timeout", 90, "Stream close timeout in seconds (defaults to 90)")
-var maxBufferedPagesTotal = flag.Int("gopacket_pages", 150_000, "Maximum number of TCP reassembly pages to allocate per interface")
-var maxBufferedPagesPerConnection = flag.Int("gopacket_per_conn", 4000, "Maximum number of TCP reassembly pages per connection")
-
+// Config holds the configuration for the agent
 type Config struct {
-	Maxcount                      int
-	Statsevery                    int
-	Lazy                          bool
-	Nodefrag                      bool
-	Checksum                      bool
-	Nooptcheck                    bool
-	Ignorefsmerr                  bool
-	Allowmissinginit              bool
-	Verbose                       bool
-	Debug                         bool
-	Quiet                         bool
-	Interface                     string
-	FileName                      string
-	Snaplen                       int
-	TsType                        string
-	Promiscuous                   bool
-	StreamFlushTimeout            time.Duration
-	StreamCloseTimeout            time.Duration
-	PacketSource                  string
-	BpfFilter                     string
-	ChannelBufferSize             int
-	MaxBufferedPagesTotal         int
+	// Honeycomb API key used to send events.
+	// Set via HONEYCOMB_API_KEY environment variable.
+	APIKey string
+
+	// Honeycomb API endpoint events are sent to.
+	// Set via HONEYCOMB_API_ENDPOINT environment variable.
+	Endpoint string
+
+	// Honeycomb destination dataset for events.
+	// Set via HONEYCOMB_DATASET environment variable.
+	Dataset string
+
+	// Honeycomb destination dataset for agent performance stats.
+	// Set via HONEYCOMB_STATS_DATASET environment variable.
+	StatsDataset string
+
+	// Log level used by the agent.
+	// Set via LOG_LEVEL environment variable
+	LogLevel string
+
+	// Run the agent in debug mode.
+	// Set via DEBUG environment variable.
+	Debug bool
+
+	// Debug service address.
+	// Set via DEBUG_ADDRESS environment variable.
+	DebugAddress string
+
+	// Do lazy decoding of layers when processing packets.
+	Lazy bool
+
+	// Do not do IPv4 defragmentation when processing packets.
+	Nodefrag bool
+
+	// Check TCP checksum when processing packets.
+	Checksum bool
+
+	// Do not check TCP options (useful to ignore MSS on captures with TSO).
+	Nooptcheck bool
+
+	// Ignore TCP FSM errors.
+	Ignorefsmerr bool
+
+	// Support streams without SYN/SYN+ACK/ACK sequence.
+	Allowmissinginit bool
+
+	// Interface to read packets from.
+	Interface string
+
+	// Snap length (number of bytes max to read per packet (defaults to 262144 which is the default snaplen tcpdump).
+	Snaplen int
+
+	// Set promiscuous mode on the interface.
+	Promiscuous bool
+
+	// Stream flush timeout in seconds (defaults to 10 seconds).
+	StreamFlushTimeout time.Duration
+
+	// Stream close timeout in seconds (defaults to 90 seconds).
+	StreamCloseTimeout time.Duration
+
+	// Packet source (defaults to pcap).
+	PacketSource string
+
+	// Channel buffer size (defaults to 1000).
+	BpfFilter string
+
+	// Maximum number of HTTP events waiting to be processed to buffer before dropping.
+	ChannelBufferSize int
+
+	// Maximum number of TCP reassembly pages to allocate per interface.
+	MaxBufferedPagesTotal int
+
+	// Maximum number of TCP reassembly pages per connection.
 	MaxBufferedPagesPerConnection int
-	DebugAddr                     string
 }
 
+// NewConfig returns a new Config struct.
+// Values are set from environment variables if they exist, otherwise they are set to default
 func NewConfig() Config {
-	c := Config{
-		Maxcount:                      *maxcount,
-		Statsevery:                    *statsevery,
-		Lazy:                          *lazy,
-		Nodefrag:                      *nodefrag,
-		Checksum:                      *checksum,
-		Nooptcheck:                    *nooptcheck,
-		Ignorefsmerr:                  *ignorefsmerr,
-		Allowmissinginit:              *allowmissinginit,
-		Verbose:                       *verbose,
-		Debug:                         *debug,
-		Quiet:                         *quiet,
-		Interface:                     *iface,
-		FileName:                      *fname,
-		Snaplen:                       *snaplen,
-		TsType:                        *tstype,
-		Promiscuous:                   *promisc,
-		StreamFlushTimeout:            time.Duration(*streamFlushTimeout) * time.Second,
-		StreamCloseTimeout:            time.Duration(*streamCloseTimeout) * time.Second,
-		PacketSource:                  *packetSource,
-		BpfFilter:                     *bpfFilter,
-		ChannelBufferSize:             *channelBufferSize,
-		MaxBufferedPagesTotal:         *maxBufferedPagesTotal,
-		MaxBufferedPagesPerConnection: *maxBufferedPagesPerConnection,
-		DebugAddr:                     DebugAddr,
+	return Config{
+		APIKey:                        utils.LookupEnvOrString("HONEYCOMB_API_KEY", ""),
+		Endpoint:                      utils.LookupEnvOrString("HONEYCOMB_API_ENDPOINT", "https://api.honeycomb.io"),
+		Dataset:                       utils.LookupEnvOrString("HONEYCOMB_DATASET", "hny-network-agent"),
+		StatsDataset:                  utils.LookupEnvOrString("HONEYCOMB_STATS_DATASET", "hny-network-agent-stats"),
+		LogLevel:                      utils.LookupEnvOrString("LOG_LEVEL", "INFO"),
+		Debug:                         utils.LookupEnvOrBool("DEBUG", false),
+		DebugAddress:                  utils.LookupEnvOrString("DEBUG_ADDRESS", "0.0.0.0:6060"),
+		Lazy:                          false,
+		Nodefrag:                      false,
+		Checksum:                      false,
+		Nooptcheck:                    true,
+		Ignorefsmerr:                  true,
+		Allowmissinginit:              true,
+		Interface:                     "any",
+		Snaplen:                       262144,
+		Promiscuous:                   true,
+		StreamFlushTimeout:            time.Duration(10 * time.Second),
+		StreamCloseTimeout:            time.Duration(90 * time.Second),
+		PacketSource:                  "pcap",
+		BpfFilter:                     buildBpfFilter(),
+		ChannelBufferSize:             1000,
+		MaxBufferedPagesTotal:         150_000,
+		MaxBufferedPagesPerConnection: 4000,
 	}
+}
+
+// GetMaskedAPIKey masks the API key for logging purposes
+// if the API key is less than 4 characters, it will be completely masked
+func (c *Config) GetMaskedAPIKey() string {
+	if len(c.APIKey) <= 4 {
+		return strings.Repeat("*", len(c.APIKey))
+	}
+	return strings.Repeat("*", len(c.APIKey)-4) + c.APIKey[len(c.APIKey)-4:]
+}
 
+// buildBpfFilter builds a BPF filter to only capture HTTP traffic
+// TODO: Move somewhere more appropriate
+func buildBpfFilter() string {
 	// Add filters to only capture common HTTP methods
 	// TODO "not host me", // how do we get our current IP?
 	// reference links:
@@ -109,15 +146,31 @@ func NewConfig() Config {
 		// HTTP 1.1 is the response start string
 		"tcp[((tcp[12:1] & 0xf0) >> 2):4] = 0x48545450", // 'HTTP' 1.1
 	}
-	c.BpfFilter = strings.Join(filters, " or ")
-
-	if c.Debug {
-		b, err := json.MarshalIndent(c, "", "  ")
-		if err != nil {
-			log.Debug().Err(err).Msg("Failed to marshal agent config")
-		} else {
-			log.Debug().RawJSON("Agent config", b)
-		}
+	return strings.Join(filters, " or ")
+}
+
+type MissingAPIKeyError struct{}
+
+func (e *MissingAPIKeyError) Error() string {
+	return "Missing API key"
+}
+
+type InvalidAPIKeyError struct{}
+
+func (e *InvalidAPIKeyError) Error() string {
+	return "Invalid API key"
+}
+
+// Validate checks that the config is valid
+func (c *Config) Validate() error {
+	e := []error{}
+	if c.APIKey == "" {
+		e = append(e, &MissingAPIKeyError{})
+	}
+	libhoneyConfig := libhoney.Config{APIKey: c.APIKey}
+	if _, err := libhoney.VerifyAPIKey(libhoneyConfig); err != nil {
+		e = append(e, &InvalidAPIKeyError{})
 	}
-	return c
+	// returns nil if no errors in slice
+	return errors.Join(e...)
 }