From 7c139d28b291deb38a3ef72ffae61e0651118a3d Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 13 Jun 2022 19:02:11 -0400 Subject: [PATCH] Have hclog payloads use `json` tags (#2171) * Have hclog payloads use `json` tags Previously a global setting for the `structs` lib was causing the `json` tag to be used, but this could have side effects from other packages, or could be affected by other packages' behavior. This behavior was removed earlier but it means this format call no longer uses `json` tags; this behavior reverts it. It also normalizes using mapstructure in a few other places. * Update some tests * Fix incorrect op const * Fix more tests * refactor (events): tell filter to use json tags (fix related tests) Explicitly tell the filter to use the "json" tags so we don't have to re-tag everything with a mapstructure tag via bexpr.WithTagName("json") Update docs Co-authored-by: Jim --- CHANGELOG.md | 2 + .../event/cloudevents_formatter_node.go | 4 +- .../event/cloudevents_formatter_node_test.go | 2 +- internal/observability/event/context_test.go | 8 ++-- .../event/hclog_formatter_node.go | 6 ++- .../event/hclog_formatter_node_test.go | 42 +++++++++---------- internal/servers/repository_workerauth.go | 7 ++-- .../servers/repository_workerauth_test.go | 10 +++-- internal/servers/worker.go | 7 ++-- .../docs/concepts/filtering/events.mdx | 8 ++-- 10 files changed, 52 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd660f6bc..f46ea7584 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. sessions after 1 hour. See Deprecations/Changes for some additional details. [PR](https://github.com/hashicorp/boundary/pull/2160). +* event filtering: Change event filters to use lowercase and snake case for data + elements like the rest of Boundary filters do. ### Deprecations/Changes diff --git a/internal/observability/event/cloudevents_formatter_node.go b/internal/observability/event/cloudevents_formatter_node.go index 3dee6dc8f..d457703f2 100644 --- a/internal/observability/event/cloudevents_formatter_node.go +++ b/internal/observability/event/cloudevents_formatter_node.go @@ -143,7 +143,9 @@ func newFilter(f string) (*filter, error) { if f == "" { return nil, fmt.Errorf("%s: missing filter: %w", op, ErrInvalidParameter) } - e, err := bexpr.CreateEvaluator(f, bexpr.WithHookFn(filterpkg.WellKnownTypeFilterHook)) + // explicitly tell the filter to use the "json" tags so we don't have to + // re-tag everything with a mapstructure tag via bexpr.WithTagName("json") + e, err := bexpr.CreateEvaluator(f, bexpr.WithHookFn(filterpkg.WellKnownTypeFilterHook), bexpr.WithTagName("json")) if err != nil { return nil, fmt.Errorf("%s: %w", op, err) } diff --git a/internal/observability/event/cloudevents_formatter_node_test.go b/internal/observability/event/cloudevents_formatter_node_test.go index 6ad44f316..2aee2598d 100644 --- a/internal/observability/event/cloudevents_formatter_node_test.go +++ b/internal/observability/event/cloudevents_formatter_node_test.go @@ -154,7 +154,7 @@ func TestNode_Process(t *testing.T) { testNode, err := newCloudEventsFormatterFilter(testUrl, cloudevents.FormatJSON, WithSchema(testUrl)) require.NoError(t, err) - f, err := newFilter(`Data == "match-filter"`) + f, err := newFilter(`data == "match-filter"`) require.NoError(t, err) tests := []struct { diff --git a/internal/observability/event/context_test.go b/internal/observability/event/context_test.go index c145c6863..69990ea29 100644 --- a/internal/observability/event/context_test.go +++ b/internal/observability/event/context_test.go @@ -517,25 +517,25 @@ func Test_Filtering(t *testing.T) { }{ { name: "allowed", - allow: []string{`"/Data/list" contains "1"`}, + allow: []string{`"/data/list" contains "1"`}, hdr: []interface{}{"list", []string{"1", "2"}}, found: true, }, { name: "not-allowed", - allow: []string{`"/Data/list" contains "22"`}, + allow: []string{`"/data/list" contains "22"`}, hdr: []interface{}{"list", []string{"1", "2"}}, found: false, }, { name: "deny", - deny: []string{`"/Data/list" contains "1"`}, + deny: []string{`"/data/list" contains "1"`}, hdr: []interface{}{"list", []string{"1", "2"}}, found: false, }, { name: "not-deny", - deny: []string{`"/Data/list" contains "22"`}, + deny: []string{`"/data/list" contains "22"`}, hdr: []interface{}{"list", []string{"1", "2"}}, found: true, }, diff --git a/internal/observability/event/hclog_formatter_node.go b/internal/observability/event/hclog_formatter_node.go index 7cb36562a..480ba0a6a 100644 --- a/internal/observability/event/hclog_formatter_node.go +++ b/internal/observability/event/hclog_formatter_node.go @@ -36,7 +36,7 @@ type hclogFormatterFilter struct { } func newHclogFormatterFilter(jsonFormat bool, opt ...Option) (*hclogFormatterFilter, error) { - const op = "event.NewHclogFormatter" + const op = "event.newHclogFormatterFilter" opts := getOpts(opt...) var s signer n := hclogFormatterFilter{ @@ -147,7 +147,9 @@ func (f *hclogFormatterFilter) Process(ctx context.Context, e *eventlogger.Event var m map[string]interface{} switch string(e.Type) { case string(ErrorType), string(AuditType), string(SystemType): - m = structs.Map(e.Payload) + s := structs.New(e.Payload) + s.TagName = "json" + m = s.Map() case string(ObservationType): m = e.Payload.(map[string]interface{}) default: diff --git a/internal/observability/event/hclog_formatter_node_test.go b/internal/observability/event/hclog_formatter_node_test.go index d8d717fbd..ab9ba3104 100644 --- a/internal/observability/event/hclog_formatter_node_test.go +++ b/internal/observability/event/hclog_formatter_node_test.go @@ -15,7 +15,7 @@ func TestHclogFormatter_Process(t *testing.T) { t.Parallel() ctx := context.Background() - f, e := newFilter(`Op == "match-filter"`) + f, e := newFilter(`op == "match-filter"`) require.NoError(t, e) testPredicate := newPredicate([]*filter{f}, nil) @@ -60,10 +60,9 @@ func TestHclogFormatter_Process(t *testing.T) { }, want: []string{ "[INFO] system event:", - "Data:msg=hello", - "Id=1", - "Version=v0.1", - "Op=text", + "data:msg=hello", + "version=v0.1", + "op=text", }, }, { @@ -122,10 +121,10 @@ func TestHclogFormatter_Process(t *testing.T) { }, want: []string{ "[ERROR] error event:", - "Error=\"invalid parameter\"", - "Id=1", - "Version=v0.1", - "Op=text", + "error=\"invalid parameter\"", + "id=1", + "version=v0.1", + "op=text", }, }, { @@ -144,10 +143,10 @@ func TestHclogFormatter_Process(t *testing.T) { }, want: []string{ "{\"@level\":\"error\",\"@message\":\"error event\"", - "\"Error\":\"invalid parameter\"", - "\"Id\":\"1\"", - "\"Version\":\"v0.1\"", - "\"Op\":\"text\"", + "\"error\":\"invalid parameter\"", + "\"id\":\"1\"", + "\"version\":\"v0.1\"", + "\"op\":\"text\"", }, }, { @@ -167,11 +166,11 @@ func TestHclogFormatter_Process(t *testing.T) { }, want: []string{ "[ERROR] error event:", - "Error=\"invalid parameter\"", - "Id=1", - "Version=v0.1", - "Info:name=alice", - "Op=text", + "error=\"invalid parameter\"", + "id=1", + "version=v0.1", + "info:name=alice", + "op=text", }, }, { @@ -193,10 +192,9 @@ func TestHclogFormatter_Process(t *testing.T) { }, want: []string{ "[INFO] system event:", - "Data:msg=hello", - "Id=1", - "Version=v0.1", - "Op=match-filter", + "data:msg=hello", + "version=v0.1", + "op=match-filter", }, }, { diff --git a/internal/servers/repository_workerauth.go b/internal/servers/repository_workerauth.go index cdcd34b80..9c94b4bb0 100644 --- a/internal/servers/repository_workerauth.go +++ b/internal/servers/repository_workerauth.go @@ -31,7 +31,7 @@ var ( ) type rootCertificatesVersion struct { - Version uint32 `mapstructure:"version" structs:"version"` + Version uint32 `mapstructure:"version"` } // WorkerAuthRepositoryStorage is the Worker Auth database repository @@ -398,8 +398,9 @@ func (r *WorkerAuthRepositoryStorage) loadNodeInformation(ctx context.Context, n } workerIdInfo := workerAuthWorkerId{WorkerId: authorizedWorker.GetWorkerId()} - workerIdMap := structs.Map(workerIdInfo) - state, err := structpb.NewStruct(workerIdMap) + s := structs.New(workerIdInfo) + s.TagName = "mapstructure" + state, err := structpb.NewStruct(s.Map()) if err != nil { return errors.Wrap(ctx, err, op) } diff --git a/internal/servers/repository_workerauth_test.go b/internal/servers/repository_workerauth_test.go index 94aa141bc..95b85853d 100644 --- a/internal/servers/repository_workerauth_test.go +++ b/internal/servers/repository_workerauth_test.go @@ -106,8 +106,9 @@ func TestStoreCertAuthorityVersioning(t *testing.T) { // Store CA with old version, expect error badVersion := &rootCertificatesVersion{Version: uint32(1)} - badStateOpt := structs.Map(badVersion) - badState, err := structpb.NewStruct(badStateOpt) + s := structs.New(badVersion) + s.TagName = "mapstructure" + badState, err := structpb.NewStruct(s.Map()) require.NoError(err) newRoots2 := &types.RootCertificates{ Id: ca_id, @@ -121,8 +122,9 @@ func TestStoreCertAuthorityVersioning(t *testing.T) { // Remove CA, expect updated version cAuthRemove := &types.RootCertificates{Id: ca_id} removeVersion := &rootCertificatesVersion{Version: uint32(2)} - removeOpt := structs.Map(removeVersion) - removeState, err := structpb.NewStruct(removeOpt) + s = structs.New(removeVersion) + s.TagName = "mapstructure" + removeState, err := structpb.NewStruct(s.Map()) require.NoError(err) cAuthRemove.State = removeState err = workerAuthRepo.Remove(ctx, cAuthRemove) diff --git a/internal/servers/worker.go b/internal/servers/worker.go index b13cf80e2..a80383508 100644 --- a/internal/servers/worker.go +++ b/internal/servers/worker.go @@ -13,7 +13,7 @@ import ( ) type workerAuthWorkerId struct { - WorkerId string `mapstructure:"worker_id" structs:"worker_id"` + WorkerId string `mapstructure:"worker_id"` } // AttachWorkerIdToState accepts a workerId and creates a struct for use with the Nodeenrollment lib @@ -26,8 +26,9 @@ func AttachWorkerIdToState(ctx context.Context, workerId string) (*structpb.Stru } workerMap := &workerAuthWorkerId{WorkerId: workerId} - stateOpt := structs.Map(workerMap) - return structpb.NewStruct(stateOpt) + s := structs.New(workerMap) + s.TagName = "mapstructure" + return structpb.NewStruct(s.Map()) } // A Worker is a server that provides an address which can be used to proxy diff --git a/website/content/docs/concepts/filtering/events.mdx b/website/content/docs/concepts/filtering/events.mdx index f2397d780..a2e1beb23 100644 --- a/website/content/docs/concepts/filtering/events.mdx +++ b/website/content/docs/concepts/filtering/events.mdx @@ -75,8 +75,8 @@ sink "stderr" = { event_types = ["*"] format = "cloudevents-text" allow_filters = [ - '"/Data/request_info/Path" contains ":authenticate"' - '"/Data/Op" contains ".createClientConn"' + '"/data/request_info/path" contains ":authenticate"' + '"/data/op" contains ".createClientConn"' ] # note: deny_filters are also supported. } @@ -85,8 +85,8 @@ sink "stderr" = { When running `boundary dev` the example allow filter can be given via: ```bash boundary dev \ - -event-allow-filter '"/Data/request_info/Path" contains ":authenticate"' \ - -event-allow-filter '"/Data/Op" contains ".createClientConn"' + -event-allow-filter '"/data/request_info/path" contains ":authenticate"' \ + -event-allow-filter '"/data/op" contains ".createClientConn"' ``` ~> Double quotes are part of the filter syntax; when using the CLI, it is likely easier to surround the filter with single quotes than to deal with escaping