From b07e996d3b6659c2926bc60730970879d3cf5f91 Mon Sep 17 00:00:00 2001 From: Kent Quirk Date: Thu, 1 Aug 2024 19:26:58 -0400 Subject: [PATCH 1/5] Allow more complex key behavior --- README.md | 42 +++++++++++++++++++++++ config/config.go | 3 ++ config/file_config.go | 19 +++++----- config/metadata/configMeta.yaml | 35 +++++++++++++++++++ config/mock.go | 8 +++++ route/middleware.go | 61 +++++++++++++++++++++++++++++---- route/route.go | 2 +- 7 files changed, 154 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 8b1b273ac4..b517e927c4 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,48 @@ Refinery supports the following key environment variables; please see the comman Note: `REFINERY_HONEYCOMB_METRICS_API_KEY` takes precedence over `REFINERY_HONEYCOMB_API_KEY` for the `LegacyMetrics.APIKey` configuration. +## Managing Keys + +Sending data to Honeycomb requires attaching an API key to telemetry. In order to make managing telemetry easier, Refinery support the `ReceiveKeys` and `SendKey` config options, along with `AcceptOnlyListedKeys` and `SendKeyMode`. In various combinations, they have a lot of expressive power. Please see the configuration documentation for details on how to set these parameters. + +A quick start for specific scenarios is below: + +### A small number of services +* Set keys in your applications the way you normally would, and leave Refinery set to the defaults. + +### Large number of services, central key preferred +* Do not set keys in your applications +* Set `SendKey` to a valid Honeycomb Key +* Set `SendKeyMode` to `all` + +### Applications must set a key, but control the actual key at Refinery +* Set `SendKey` to a valid Honeycomb Key +* Set `SendKeyMode` to `nonblank` + +### Replace most keys but permit exceptions +* Set `ReceiveKeys` to the list of exceptions +* Set `SendKey` to a valid Honeycomb Key +* Set `SendKeyMode` to `unlisted` + +### Some applications have custom keys, but others should use central key +* Set custom keys in your applications as needed, leave others blank +* Set `SendKey` to a valid Honeycomb Key +* Set `SendKeyMode` to `missingonly` + +### Only applications knowing a specific secret should be able to send telemetry, but a central key is preferred +* Choose an internal secret key (any arbitrary string) +* Add that secret to `ReceiveKeys` +* Set `AcceptOnlyListedKeys` to `true` +* Set `SendKey` to a valid Honeycomb Key +* Set `SendKeyMode` to `listedonly` + +### Replace specific keys used by certain applications with the central key +* Set `AcceptOnlyListedKeys` to `false` +* Set `ReceiveKeys` to the keys that should be replaced +* Set `SendKey` to a valid Honeycomb Key +* Set `SendKeyMode` to `listedonly` + + ## Dry Run Mode When getting started with Refinery or when updating sampling rules, it may be helpful to verify that the rules are working as expected before you start dropping traffic. To do so, use Dry Run Mode in Refinery. diff --git a/config/config.go b/config/config.go index 0d503fc36b..291c6d1fe5 100644 --- a/config/config.go +++ b/config/config.go @@ -54,6 +54,9 @@ type Config interface { // Returns the entire GRPC config block GetGRPCConfig() GRPCServerParameters + // GetAccessKeyConfig returns the access key configuration + GetAccessKeyConfig() AccessKeyConfig + // IsAPIKeyValid checks if the given API key is valid according to the rules IsAPIKeyValid(key string) bool diff --git a/config/file_config.go b/config/file_config.go index a1e398409a..82cecda9ca 100644 --- a/config/file_config.go +++ b/config/file_config.go @@ -9,7 +9,7 @@ import ( "sync" "time" - "github.com/honeycombio/refinery/generics" + "golang.org/x/exp/slices" "gopkg.in/yaml.v3" ) @@ -86,8 +86,9 @@ type NetworkConfig struct { type AccessKeyConfig struct { ReceiveKeys []string `yaml:"ReceiveKeys" default:"[]"` + SendKey string `yaml:"SendKey"` + SendKeyMode string `yaml:"SendKeyMode" default:"none"` AcceptOnlyListedKeys bool `yaml:"AcceptOnlyListedKeys"` - keymap generics.Set[string] } type DefaultTrue bool @@ -532,6 +533,13 @@ func (f *fileConfig) GetGRPCConfig() GRPCServerParameters { return f.mainConfig.GRPCServerParameters } +func (f *fileConfig) GetAccessKeyConfig() AccessKeyConfig { + f.mux.RLock() + defer f.mux.RUnlock() + + return f.mainConfig.AccessKeys +} + func (f *fileConfig) IsAPIKeyValid(key string) bool { f.mux.RLock() defer f.mux.RUnlock() @@ -540,12 +548,7 @@ func (f *fileConfig) IsAPIKeyValid(key string) bool { return true } - // if we haven't built the keymap yet, do it now - if f.mainConfig.AccessKeys.keymap == nil { - f.mainConfig.AccessKeys.keymap = generics.NewSet(f.mainConfig.AccessKeys.ReceiveKeys...) - } - - return f.mainConfig.AccessKeys.keymap.Contains(key) + return slices.Contains(f.mainConfig.AccessKeys.ReceiveKeys, key) } func (f *fileConfig) GetPeerManagementType() string { diff --git a/config/metadata/configMeta.yaml b/config/metadata/configMeta.yaml index 531454b617..562becc9c5 100644 --- a/config/metadata/configMeta.yaml +++ b/config/metadata/configMeta.yaml @@ -181,6 +181,41 @@ groups: If `false`, then all traffic is accepted and `ReceiveKeys` is ignored. + This setting is applied *before* the `SendKey` and `SendKeyMode` settings. + + - name: SendKey + type: string + reload: true + validations: + - type: format + arg: apikey + - type: validapikey + summary: is a Honeycomb API key that Refinery can use to send data to Honeycomb, depending on configuration. + description: > + If `SendKey` is set to a valid Honeycomb key, then Refinery can use + the listed key to send data. + How this works depends on the value of `SendKeyMode`. + + - name: SendKeyMode + type: string + valuetype: choice + choices: ["none", "all", "nonblank", "listedonly", "unlisted", "missingonly"] + default: "none" + reload: true + summary: controls how SendKey is used to replace or augment API keys used in incoming telemetry. + description: > + Controls how SendKey is used to replace or supply API keys used in + incoming telemetry. If `AcceptOnlyListedKeys` is `true`, then + `SendKeys` will only be used for events with keys listed in + `ReceiveKeys`. + + `none` uses the incoming key for all telemetry (default). + `all` overwrites all keys, even missing ones, with `SendKey`. + `nonblank` overwrites all supplied keys but will not inject `SendKey` if the incoming key is blank. + `listedonly` overwrites only the keys listed in `ReceiveKeys`. + `unlisted` uses the `SendKey` for all events *except* those with keys listed in `ReceiveKeys`, which use their original keys. + `missingonly` uses the SendKey only to inject keys into events with blank keys. All other events use their original keys. + - name: RefineryTelemetry title: "Refinery Telemetry" description: contains configuration information for the telemetry that Refinery uses to record its own operation. diff --git a/config/mock.go b/config/mock.go index 94dc4339d2..ea264839ae 100644 --- a/config/mock.go +++ b/config/mock.go @@ -9,6 +9,7 @@ import ( // initialization type MockConfig struct { Callbacks []ConfigReloadCallback + GetAccessKeyConfigVal AccessKeyConfig IsAPIKeyValidFunc func(string) bool GetCollectorTypeVal string GetCollectionConfigVal CollectionConfig @@ -102,6 +103,13 @@ func (m *MockConfig) GetHashes() (string, string) { return m.CfgHash, m.RulesHash } +func (m *MockConfig) GetAccessKeyConfig() AccessKeyConfig { + m.Mux.RLock() + defer m.Mux.RUnlock() + + return m.GetAccessKeyConfigVal +} + func (m *MockConfig) IsAPIKeyValid(key string) bool { m.Mux.RLock() defer m.Mux.RUnlock() diff --git a/route/middleware.go b/route/middleware.go index 3a9a67979b..111e51daf7 100644 --- a/route/middleware.go +++ b/route/middleware.go @@ -6,6 +6,7 @@ import ( "fmt" "math/rand" "net/http" + "slices" "time" "github.com/gorilla/mux" @@ -38,23 +39,69 @@ func (r *Router) queryTokenChecker(next http.Handler) http.Handler { }) } -func (r *Router) apiKeyChecker(next http.Handler) http.Handler { +// truncate the key to 8 characters for logging +func (r *Router) sanitize(key string) string { + return fmt.Sprintf("%.8s...", key) +} + +func (r *Router) apiKeyProcessor(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { apiKey := req.Header.Get(types.APIKeyHeader) if apiKey == "" { apiKey = req.Header.Get(types.APIKeyHeaderShort) } - if apiKey == "" { - err := errors.New("no " + types.APIKeyHeader + " header found from within authing middleware") + + keycfg := r.Config.GetAccessKeyConfig() + + // Apply AcceptOnlyListedKeys logic BEFORE we consider replacement + if keycfg.AcceptOnlyListedKeys && !slices.Contains(keycfg.ReceiveKeys, apiKey) { + err := fmt.Errorf("api key %s not found in list of authorized keys", r.sanitize(apiKey)) r.handlerReturnWithError(w, ErrAuthNeeded, err) return } - if r.Config.IsAPIKeyValid(apiKey) { - next.ServeHTTP(w, req) + + if keycfg.SendKey != "" { + overwriteWith := "" + switch keycfg.SendKeyMode { + case "none": + // don't replace keys at all + // (SendKey is disabled) + case "all": + // overwrite all keys, even missing ones, with the configured one + overwriteWith = keycfg.SendKey + case "nonblank": + // only replace nonblank keys with the configured one + if apiKey != "" { + overwriteWith = keycfg.SendKey + } + case "listedonly": + // only replace keys that are listed in the `ReceiveKeys` list + // reject everything else + if slices.Contains(keycfg.ReceiveKeys, apiKey) { + overwriteWith = keycfg.SendKey + } + case "missingonly": + // only inject keys into telemetry that doesn't have a key at all + if apiKey == "" { + overwriteWith = keycfg.SendKey + } + case "unlisted": + // only replace nonblank keys that are NOT listed in the `ReceiveKeys` list + if apiKey != "" && !slices.Contains(keycfg.ReceiveKeys, apiKey) { + overwriteWith = keycfg.SendKey + } + } + req.Header.Set(types.APIKeyHeader, overwriteWith) + } + + // we might still have a blank key, so check again + if apiKey == "" { + err := errors.New("no value for " + types.APIKeyHeader + " header found from within authing middleware") + r.handlerReturnWithError(w, ErrAuthNeeded, err) return } - err := fmt.Errorf("api key %s not found in list of authorized keys", apiKey) - r.handlerReturnWithError(w, ErrAuthNeeded, err) + + next.ServeHTTP(w, req) }) } diff --git a/route/route.go b/route/route.go index 4578ce767d..e5b357a63f 100644 --- a/route/route.go +++ b/route/route.go @@ -174,7 +174,7 @@ func (r *Router) LnS(incomingOrPeer string) { // require an auth header for events and batches authedMuxxer := muxxer.PathPrefix("/1/").Methods("POST").Subrouter() authedMuxxer.UseEncodedPath() - authedMuxxer.Use(r.apiKeyChecker) + authedMuxxer.Use(r.apiKeyProcessor) // handle events and batches authedMuxxer.HandleFunc("/events/{datasetName}", r.event).Name("event") From c2c617a92737044cfc8d6a65d6bb6ab2c7a6caa0 Mon Sep 17 00:00:00 2001 From: Kent Quirk Date: Tue, 6 Aug 2024 12:53:48 -0400 Subject: [PATCH 2/5] Make it work with OTLP, fix tests --- app/app_test.go | 5 +++- config/config.go | 3 -- config/file_config.go | 65 +++++++++++++++++++++++++++++++++------- config/mock.go | 13 -------- route/middleware.go | 53 ++------------------------------ route/otlp_logs.go | 19 +++++++----- route/otlp_logs_test.go | 12 +++++--- route/otlp_trace.go | 19 +++++++----- route/otlp_trace_test.go | 10 +++++-- 9 files changed, 101 insertions(+), 98 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index be21f805cc..6fdaa62654 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -106,13 +106,16 @@ func newStartedApp( GetPeerBufferSizeVal: 10000, GetListenAddrVal: "127.0.0.1:" + strconv.Itoa(basePort), GetPeerListenAddrVal: "127.0.0.1:" + strconv.Itoa(basePort+1), - IsAPIKeyValidFunc: func(k string) bool { return k == legacyAPIKey || k == nonLegacyAPIKey }, GetHoneycombAPIVal: "http://api.honeycomb.io", GetCollectionConfigVal: config.CollectionConfig{CacheCapacity: 10000}, AddHostMetadataToTrace: enableHostMetadata, TraceIdFieldNames: []string{"trace.trace_id"}, ParentIdFieldNames: []string{"trace.parent_id"}, SampleCache: config.SampleCacheConfig{KeptSize: 10000, DroppedSize: 100000, SizeCheckInterval: config.Duration(10 * time.Second)}, + GetAccessKeyConfigVal: config.AccessKeyConfig{ + ReceiveKeys: []string{legacyAPIKey, nonLegacyAPIKey}, + AcceptOnlyListedKeys: true, + }, } var err error diff --git a/config/config.go b/config/config.go index 291c6d1fe5..26384aeff9 100644 --- a/config/config.go +++ b/config/config.go @@ -57,9 +57,6 @@ type Config interface { // GetAccessKeyConfig returns the access key configuration GetAccessKeyConfig() AccessKeyConfig - // IsAPIKeyValid checks if the given API key is valid according to the rules - IsAPIKeyValid(key string) bool - // GetPeers returns a list of other servers participating in this proxy cluster GetPeers() []string diff --git a/config/file_config.go b/config/file_config.go index 82cecda9ca..ad235faa8a 100644 --- a/config/file_config.go +++ b/config/file_config.go @@ -91,6 +91,60 @@ type AccessKeyConfig struct { AcceptOnlyListedKeys bool `yaml:"AcceptOnlyListedKeys"` } +// truncate the key to 8 characters for logging +func (a *AccessKeyConfig) sanitize(key string) string { + return fmt.Sprintf("%.8s...", key) +} + +// CheckAndMaybeReplaceKey checks the given API key against the configuration +// and possibly replaces it with the configured SendKey, if the settings so indicate. +func (a *AccessKeyConfig) CheckAndMaybeReplaceKey(apiKey string) (string, error) { + // Apply AcceptOnlyListedKeys logic BEFORE we consider replacement + if a.AcceptOnlyListedKeys && !slices.Contains(a.ReceiveKeys, apiKey) { + err := fmt.Errorf("api key %s not found in list of authorized keys", a.sanitize(apiKey)) + return "", err + } + + if a.SendKey != "" { + overwriteWith := "" + switch a.SendKeyMode { + case "none": + // don't replace keys at all + // (SendKey is disabled) + case "all": + // overwrite all keys, even missing ones, with the configured one + overwriteWith = a.SendKey + case "nonblank": + // only replace nonblank keys with the configured one + if apiKey != "" { + overwriteWith = a.SendKey + } + case "listedonly": + // only replace keys that are listed in the `ReceiveKeys` list + // reject everything else + if slices.Contains(a.ReceiveKeys, apiKey) { + overwriteWith = a.SendKey + } + case "missingonly": + // only inject keys into telemetry that doesn't have a key at all + if apiKey == "" { + overwriteWith = a.SendKey + } + case "unlisted": + // only replace nonblank keys that are NOT listed in the `ReceiveKeys` list + if apiKey != "" && !slices.Contains(a.ReceiveKeys, apiKey) { + overwriteWith = a.SendKey + } + } + return overwriteWith, nil + } + + if apiKey == "" { + return "", fmt.Errorf("blank API key not permitted with this configuration") + } + return apiKey, nil +} + type DefaultTrue bool func (dt *DefaultTrue) Get() (enabled bool) { @@ -540,17 +594,6 @@ func (f *fileConfig) GetAccessKeyConfig() AccessKeyConfig { return f.mainConfig.AccessKeys } -func (f *fileConfig) IsAPIKeyValid(key string) bool { - f.mux.RLock() - defer f.mux.RUnlock() - - if !f.mainConfig.AccessKeys.AcceptOnlyListedKeys { - return true - } - - return slices.Contains(f.mainConfig.AccessKeys.ReceiveKeys, key) -} - func (f *fileConfig) GetPeerManagementType() string { f.mux.RLock() defer f.mux.RUnlock() diff --git a/config/mock.go b/config/mock.go index ea264839ae..b6c8570210 100644 --- a/config/mock.go +++ b/config/mock.go @@ -10,7 +10,6 @@ import ( type MockConfig struct { Callbacks []ConfigReloadCallback GetAccessKeyConfigVal AccessKeyConfig - IsAPIKeyValidFunc func(string) bool GetCollectorTypeVal string GetCollectionConfigVal CollectionConfig GetHoneycombAPIVal string @@ -110,18 +109,6 @@ func (m *MockConfig) GetAccessKeyConfig() AccessKeyConfig { return m.GetAccessKeyConfigVal } -func (m *MockConfig) IsAPIKeyValid(key string) bool { - m.Mux.RLock() - defer m.Mux.RUnlock() - - // if no function is set, assume the key is valid - if m.IsAPIKeyValidFunc == nil { - return true - } - - return m.IsAPIKeyValidFunc(key) -} - func (m *MockConfig) GetCollectorType() string { m.Mux.RLock() defer m.Mux.RUnlock() diff --git a/route/middleware.go b/route/middleware.go index 111e51daf7..0c8897e6c0 100644 --- a/route/middleware.go +++ b/route/middleware.go @@ -2,11 +2,9 @@ package route import ( "context" - "errors" "fmt" "math/rand" "net/http" - "slices" "time" "github.com/gorilla/mux" @@ -39,11 +37,6 @@ func (r *Router) queryTokenChecker(next http.Handler) http.Handler { }) } -// truncate the key to 8 characters for logging -func (r *Router) sanitize(key string) string { - return fmt.Sprintf("%.8s...", key) -} - func (r *Router) apiKeyProcessor(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { apiKey := req.Header.Get(types.APIKeyHeader) @@ -53,54 +46,14 @@ func (r *Router) apiKeyProcessor(next http.Handler) http.Handler { keycfg := r.Config.GetAccessKeyConfig() - // Apply AcceptOnlyListedKeys logic BEFORE we consider replacement - if keycfg.AcceptOnlyListedKeys && !slices.Contains(keycfg.ReceiveKeys, apiKey) { - err := fmt.Errorf("api key %s not found in list of authorized keys", r.sanitize(apiKey)) + overwriteWith, err := keycfg.CheckAndMaybeReplaceKey(apiKey) + if err != nil { r.handlerReturnWithError(w, ErrAuthNeeded, err) return } - - if keycfg.SendKey != "" { - overwriteWith := "" - switch keycfg.SendKeyMode { - case "none": - // don't replace keys at all - // (SendKey is disabled) - case "all": - // overwrite all keys, even missing ones, with the configured one - overwriteWith = keycfg.SendKey - case "nonblank": - // only replace nonblank keys with the configured one - if apiKey != "" { - overwriteWith = keycfg.SendKey - } - case "listedonly": - // only replace keys that are listed in the `ReceiveKeys` list - // reject everything else - if slices.Contains(keycfg.ReceiveKeys, apiKey) { - overwriteWith = keycfg.SendKey - } - case "missingonly": - // only inject keys into telemetry that doesn't have a key at all - if apiKey == "" { - overwriteWith = keycfg.SendKey - } - case "unlisted": - // only replace nonblank keys that are NOT listed in the `ReceiveKeys` list - if apiKey != "" && !slices.Contains(keycfg.ReceiveKeys, apiKey) { - overwriteWith = keycfg.SendKey - } - } + if overwriteWith != apiKey { req.Header.Set(types.APIKeyHeader, overwriteWith) } - - // we might still have a blank key, so check again - if apiKey == "" { - err := errors.New("no value for " + types.APIKeyHeader + " header found from within authing middleware") - r.handlerReturnWithError(w, ErrAuthNeeded, err) - return - } - next.ServeHTTP(w, req) }) } diff --git a/route/otlp_logs.go b/route/otlp_logs.go index 39535e8d04..371e1f1af9 100644 --- a/route/otlp_logs.go +++ b/route/otlp_logs.go @@ -3,7 +3,6 @@ package route import ( "context" "errors" - "fmt" "net/http" huskyotlp "github.com/honeycombio/husky/otlp" @@ -25,8 +24,11 @@ func (r *Router) postOTLPLogs(w http.ResponseWriter, req *http.Request) { return } - if !r.Config.IsAPIKeyValid(ri.ApiKey) { - r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: fmt.Sprintf("api key %s not found in list of authorized keys", ri.ApiKey), HTTPStatusCode: http.StatusUnauthorized}) + apicfg := r.Config.GetAccessKeyConfig() + keyToUse, err := apicfg.CheckAndMaybeReplaceKey(ri.ApiKey) + + if err != nil { + r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusUnauthorized}) return } @@ -36,7 +38,7 @@ func (r *Router) postOTLPLogs(w http.ResponseWriter, req *http.Request) { return } - if err := r.processOTLPRequest(req.Context(), result.Batches, ri.ApiKey); err != nil { + if err := r.processOTLPRequest(req.Context(), result.Batches, keyToUse); err != nil { r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusInternalServerError}) return } @@ -60,8 +62,11 @@ func (l *LogsServer) Export(ctx context.Context, req *collectorlogs.ExportLogsSe return nil, huskyotlp.AsGRPCError(err) } - if !l.router.Config.IsAPIKeyValid(ri.ApiKey) { - return nil, status.Error(codes.Unauthenticated, fmt.Sprintf("api key %s not found in list of authorized keys", ri.ApiKey)) + apicfg := l.router.Config.GetAccessKeyConfig() + keyToUse, err := apicfg.CheckAndMaybeReplaceKey(ri.ApiKey) + + if err != nil { + return nil, status.Error(codes.Unauthenticated, err.Error()) } result, err := huskyotlp.TranslateLogsRequest(ctx, req, ri) @@ -69,7 +74,7 @@ func (l *LogsServer) Export(ctx context.Context, req *collectorlogs.ExportLogsSe return nil, huskyotlp.AsGRPCError(err) } - if err := l.router.processOTLPRequest(ctx, result.Batches, ri.ApiKey); err != nil { + if err := l.router.processOTLPRequest(ctx, result.Batches, keyToUse); err != nil { return nil, huskyotlp.AsGRPCError(err) } diff --git a/route/otlp_logs_test.go b/route/otlp_logs_test.go index 85eeac61f7..593ef248a0 100644 --- a/route/otlp_logs_test.go +++ b/route/otlp_logs_test.go @@ -266,8 +266,10 @@ func TestLogsOTLPHandler(t *testing.T) { }) t.Run("rejects bad API keys - HTTP", func(t *testing.T) { - router.Config.(*config.MockConfig).IsAPIKeyValidFunc = func(k string) bool { return false } - defer func() { router.Config.(*config.MockConfig).IsAPIKeyValidFunc = nil }() + router.Config.(*config.MockConfig).GetAccessKeyConfigVal = config.AccessKeyConfig{ + ReceiveKeys: []string{}, + AcceptOnlyListedKeys: true, + } req := &collectorlogs.ExportLogsServiceRequest{ ResourceLogs: []*logs.ResourceLogs{{ ScopeLogs: []*logs.ScopeLogs{{ @@ -296,8 +298,10 @@ func TestLogsOTLPHandler(t *testing.T) { }) t.Run("rejects bad API keys - gRPC", func(t *testing.T) { - router.Config.(*config.MockConfig).IsAPIKeyValidFunc = func(k string) bool { return false } - defer func() { router.Config.(*config.MockConfig).IsAPIKeyValidFunc = nil }() + router.Config.(*config.MockConfig).GetAccessKeyConfigVal = config.AccessKeyConfig{ + ReceiveKeys: []string{}, + AcceptOnlyListedKeys: true, + } req := &collectorlogs.ExportLogsServiceRequest{ ResourceLogs: []*logs.ResourceLogs{{ ScopeLogs: []*logs.ScopeLogs{{ diff --git a/route/otlp_trace.go b/route/otlp_trace.go index eb2a0dcb90..c0137b0a76 100644 --- a/route/otlp_trace.go +++ b/route/otlp_trace.go @@ -3,7 +3,6 @@ package route import ( "context" "errors" - "fmt" "net/http" huskyotlp "github.com/honeycombio/husky/otlp" @@ -25,8 +24,11 @@ func (r *Router) postOTLPTrace(w http.ResponseWriter, req *http.Request) { return } - if !r.Config.IsAPIKeyValid(ri.ApiKey) { - r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: fmt.Sprintf("api key %s not found in list of authorized keys", ri.ApiKey), HTTPStatusCode: http.StatusUnauthorized}) + apicfg := r.Config.GetAccessKeyConfig() + keyToUse, err := apicfg.CheckAndMaybeReplaceKey(ri.ApiKey) + + if err != nil { + r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusUnauthorized}) return } @@ -36,7 +38,7 @@ func (r *Router) postOTLPTrace(w http.ResponseWriter, req *http.Request) { return } - if err := r.processOTLPRequest(req.Context(), result.Batches, ri.ApiKey); err != nil { + if err := r.processOTLPRequest(req.Context(), result.Batches, keyToUse); err != nil { r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusInternalServerError}) return } @@ -60,8 +62,11 @@ func (t *TraceServer) Export(ctx context.Context, req *collectortrace.ExportTrac return nil, huskyotlp.AsGRPCError(err) } - if !t.router.Config.IsAPIKeyValid(ri.ApiKey) { - return nil, status.Error(codes.Unauthenticated, fmt.Sprintf("api key %s not found in list of authorized keys", ri.ApiKey)) + apicfg := t.router.Config.GetAccessKeyConfig() + keyToUse, err := apicfg.CheckAndMaybeReplaceKey(ri.ApiKey) + + if err != nil { + return nil, status.Error(codes.Unauthenticated, err.Error()) } result, err := huskyotlp.TranslateTraceRequest(ctx, req, ri) @@ -69,7 +74,7 @@ func (t *TraceServer) Export(ctx context.Context, req *collectortrace.ExportTrac return nil, huskyotlp.AsGRPCError(err) } - if err := t.router.processOTLPRequest(ctx, result.Batches, ri.ApiKey); err != nil { + if err := t.router.processOTLPRequest(ctx, result.Batches, keyToUse); err != nil { return nil, huskyotlp.AsGRPCError(err) } diff --git a/route/otlp_trace_test.go b/route/otlp_trace_test.go index 3f27f37da3..e6609d0171 100644 --- a/route/otlp_trace_test.go +++ b/route/otlp_trace_test.go @@ -444,7 +444,10 @@ func TestOTLPHandler(t *testing.T) { }) t.Run("rejects bad API keys - HTTP", func(t *testing.T) { - router.Config.(*config.MockConfig).IsAPIKeyValidFunc = func(k string) bool { return false } + router.Config.(*config.MockConfig).GetAccessKeyConfigVal = config.AccessKeyConfig{ + ReceiveKeys: []string{}, + AcceptOnlyListedKeys: true, + } req := &collectortrace.ExportTraceServiceRequest{ ResourceSpans: []*trace.ResourceSpans{{ ScopeSpans: []*trace.ScopeSpans{{ @@ -473,7 +476,10 @@ func TestOTLPHandler(t *testing.T) { }) t.Run("rejects bad API keys - gRPC", func(t *testing.T) { - router.Config.(*config.MockConfig).IsAPIKeyValidFunc = func(k string) bool { return false } + router.Config.(*config.MockConfig).GetAccessKeyConfigVal = config.AccessKeyConfig{ + ReceiveKeys: []string{}, + AcceptOnlyListedKeys: true, + } req := &collectortrace.ExportTraceServiceRequest{ ResourceSpans: []*trace.ResourceSpans{{ ScopeSpans: []*trace.ScopeSpans{{ From 08b2ad962ba90d15683f663d2cc53fd64cecd16d Mon Sep 17 00:00:00 2001 From: Kent Quirk Date: Tue, 6 Aug 2024 13:07:32 -0400 Subject: [PATCH 3/5] And fix those tests too --- route/otlp_logs_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/route/otlp_logs_test.go b/route/otlp_logs_test.go index 593ef248a0..5ba74ab758 100644 --- a/route/otlp_logs_test.go +++ b/route/otlp_logs_test.go @@ -270,6 +270,12 @@ func TestLogsOTLPHandler(t *testing.T) { ReceiveKeys: []string{}, AcceptOnlyListedKeys: true, } + defer func() { + router.Config.(*config.MockConfig).GetAccessKeyConfigVal = config.AccessKeyConfig{ + ReceiveKeys: []string{}, + AcceptOnlyListedKeys: false, + } + }() req := &collectorlogs.ExportLogsServiceRequest{ ResourceLogs: []*logs.ResourceLogs{{ ScopeLogs: []*logs.ScopeLogs{{ @@ -302,6 +308,12 @@ func TestLogsOTLPHandler(t *testing.T) { ReceiveKeys: []string{}, AcceptOnlyListedKeys: true, } + defer func() { + router.Config.(*config.MockConfig).GetAccessKeyConfigVal = config.AccessKeyConfig{ + ReceiveKeys: []string{}, + AcceptOnlyListedKeys: false, + } + }() req := &collectorlogs.ExportLogsServiceRequest{ ResourceLogs: []*logs.ResourceLogs{{ ScopeLogs: []*logs.ScopeLogs{{ From d5c37cd0bd4c1dca468935036c105527987e2697 Mon Sep 17 00:00:00 2001 From: Kent Quirk Date: Tue, 6 Aug 2024 14:47:59 -0400 Subject: [PATCH 4/5] New config for optional api key --- config/metadata/configMeta.yaml | 11 +++++++---- config/validate.go | 7 +++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/config/metadata/configMeta.yaml b/config/metadata/configMeta.yaml index 562becc9c5..4a6fbb616a 100644 --- a/config/metadata/configMeta.yaml +++ b/config/metadata/configMeta.yaml @@ -185,16 +185,19 @@ groups: - name: SendKey type: string + pattern: apikey + valuetype: nondefault + default: "" + example: "SetThisToAHoneycombKey" reload: true validations: - type: format - arg: apikey - - type: validapikey - summary: is a Honeycomb API key that Refinery can use to send data to Honeycomb, depending on configuration. + arg: apikeyOrBlank + summary: is an optional Honeycomb API key that Refinery can use to send data to Honeycomb, depending on configuration. description: > If `SendKey` is set to a valid Honeycomb key, then Refinery can use the listed key to send data. - How this works depends on the value of `SendKeyMode`. + The exact behavior depends on the value of `SendKeyMode`. - name: SendKeyMode type: string diff --git a/config/validate.go b/config/validate.go index d207ed0b4b..2bd417dfd1 100644 --- a/config/validate.go +++ b/config/validate.go @@ -269,6 +269,7 @@ func (m *Metadata) Validate(data map[string]any) []string { } } for _, validation := range field.Validations { + nextValidation: switch validation.Type { case "choice": if !(isString(v) && slices.Contains(field.Choices, v.(string))) { @@ -279,6 +280,12 @@ func (m *Metadata) Validate(data map[string]any) []string { var format string mask := false switch validation.Arg.(string) { + case "apikeyOrBlank": + // allow an empty string as well as a valid API key + if v.(string) == "" { + break nextValidation + } + fallthrough // fallthrough to the apikey case case "apikey": // valid API key formats are: // 1. 32 hex characters ("classic" Honeycomb API key) From f6adf9dff9cca65db9cc57d0381de37e5676e3a0 Mon Sep 17 00:00:00 2001 From: Kent Quirk Date: Wed, 7 Aug 2024 15:16:38 -0400 Subject: [PATCH 5/5] Add tests and fix bugs --- config/file_config.go | 20 +++++++--- config/file_config_test.go | 81 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 config/file_config_test.go diff --git a/config/file_config.go b/config/file_config.go index ad235faa8a..7b148922cc 100644 --- a/config/file_config.go +++ b/config/file_config.go @@ -98,6 +98,7 @@ func (a *AccessKeyConfig) sanitize(key string) string { // CheckAndMaybeReplaceKey checks the given API key against the configuration // and possibly replaces it with the configured SendKey, if the settings so indicate. +// It returns the key to use, or an error if the key is invalid given the settings. func (a *AccessKeyConfig) CheckAndMaybeReplaceKey(apiKey string) (string, error) { // Apply AcceptOnlyListedKeys logic BEFORE we consider replacement if a.AcceptOnlyListedKeys && !slices.Contains(a.ReceiveKeys, apiKey) { @@ -120,27 +121,34 @@ func (a *AccessKeyConfig) CheckAndMaybeReplaceKey(apiKey string) (string, error) overwriteWith = a.SendKey } case "listedonly": - // only replace keys that are listed in the `ReceiveKeys` list - // reject everything else + // only replace keys that are listed in the `ReceiveKeys` list, + // otherwise use original key + overwriteWith = apiKey if slices.Contains(a.ReceiveKeys, apiKey) { overwriteWith = a.SendKey } case "missingonly": // only inject keys into telemetry that doesn't have a key at all + // otherwise use original key + overwriteWith = apiKey if apiKey == "" { overwriteWith = a.SendKey } case "unlisted": // only replace nonblank keys that are NOT listed in the `ReceiveKeys` list - if apiKey != "" && !slices.Contains(a.ReceiveKeys, apiKey) { - overwriteWith = a.SendKey + // otherwise use original key + if apiKey != "" { + overwriteWith = apiKey + if !slices.Contains(a.ReceiveKeys, apiKey) { + overwriteWith = a.SendKey + } } } - return overwriteWith, nil + apiKey = overwriteWith } if apiKey == "" { - return "", fmt.Errorf("blank API key not permitted with this configuration") + return "", fmt.Errorf("blank API key is not permitted with this configuration") } return apiKey, nil } diff --git a/config/file_config_test.go b/config/file_config_test.go new file mode 100644 index 0000000000..3c1b868fd5 --- /dev/null +++ b/config/file_config_test.go @@ -0,0 +1,81 @@ +package config + +import "testing" + +func TestAccessKeyConfig_CheckAndMaybeReplaceKey(t *testing.T) { + type fields struct { + ReceiveKeys []string + SendKey string + SendKeyMode string + AcceptOnlyListedKeys bool + } + + fNone := fields{} + fRcvAccept := fields{ + ReceiveKeys: []string{"key1", "key2"}, + AcceptOnlyListedKeys: true, + } + fSendAll := fields{ + ReceiveKeys: []string{"key1", "key2"}, + SendKey: "sendkey", + SendKeyMode: "all", + } + fListed := fields{ + ReceiveKeys: []string{"key1", "key2"}, + SendKey: "sendkey", + SendKeyMode: "listedonly", + } + fMissing := fields{ + ReceiveKeys: []string{"key1", "key2"}, + SendKey: "sendkey", + SendKeyMode: "missingonly", + } + fUnlisted := fields{ + ReceiveKeys: []string{"key1", "key2"}, + SendKey: "sendkey", + SendKeyMode: "unlisted", + } + + tests := []struct { + name string + fields fields + apiKey string + want string + wantErr bool + }{ + {"empty", fNone, "userkey", "userkey", false}, + {"acceptonly known key", fRcvAccept, "key1", "key1", false}, + {"acceptonly unknown key", fRcvAccept, "badkey", "", true}, + {"acceptonly missing key", fRcvAccept, "", "", true}, + {"send all known", fSendAll, "key1", "sendkey", false}, + {"send all unknown", fSendAll, "userkey", "sendkey", false}, + {"send all missing", fSendAll, "", "sendkey", false}, + {"listed known", fListed, "key1", "sendkey", false}, + {"listed unknown", fListed, "userkey", "userkey", false}, + {"listed missing", fListed, "", "", true}, + {"missing known", fMissing, "key1", "key1", false}, + {"missing unknown", fMissing, "userkey", "userkey", false}, + {"missing missing", fMissing, "", "sendkey", false}, + {"unlisted known", fUnlisted, "key1", "key1", false}, + {"unlisted unknown", fUnlisted, "userkey", "sendkey", false}, + {"unlisted missing", fUnlisted, "", "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &AccessKeyConfig{ + ReceiveKeys: tt.fields.ReceiveKeys, + SendKey: tt.fields.SendKey, + SendKeyMode: tt.fields.SendKeyMode, + AcceptOnlyListedKeys: tt.fields.AcceptOnlyListedKeys, + } + got, err := a.CheckAndMaybeReplaceKey(tt.apiKey) + if (err != nil) != tt.wantErr { + t.Errorf("AccessKeyConfig.CheckAndMaybeReplaceKey() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("AccessKeyConfig.CheckAndMaybeReplaceKey() = '%v', want '%v'", got, tt.want) + } + }) + } +}