-
Notifications
You must be signed in to change notification settings - Fork 103
fix(nats): derive CloudEvents subject from metadata instead of hardcoded TODO #6269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ed656cf
a83bf92
8064d40
de4b3e3
bc5344d
e875b00
2b9b45b
f1c2b69
a1912a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this has been fixed at head; try |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -206,7 +206,15 @@ func sendEvent( | |||||||||||||||||||||
| event.SetID(msg.UUID) | ||||||||||||||||||||||
| event.SetType(eventType) | ||||||||||||||||||||||
| event.SetSource("minder") // The system which generated the event. The Minder URL would be nice here. | ||||||||||||||||||||||
| event.SetSubject("TODO") // This *should* represent the entity, but we don't have a standard field for it yet. | ||||||||||||||||||||||
| subject := "" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if val, ok := msg.Metadata["entity_id"]; ok && val != "" { | ||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need to check for presence if you're already checking that
Suggested change
|
||||||||||||||||||||||
| subject = val | ||||||||||||||||||||||
| } else { | ||||||||||||||||||||||
| subject = "minder" | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| event.SetSubject(subject) | ||||||||||||||||||||||
|
Comment on lines
+209
to
+217
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a stupid little Go trick, but you can use the default-zero return value from map lookup combined with
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // All our current payloads are encoded JSON; we need to unmarshal | ||||||||||||||||||||||
| payload := map[string]any{} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // SPDX-FileCopyrightText: Copyright 2026 The Minder Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // Package labels contains logic for parsing label filters. | ||
| package labels | ||
|
|
||
| import ( | ||
| "strings" | ||
| ) | ||
|
|
||
| // ParseLabelFilter parses a comma-separated label filter string into lists of | ||
| // labels to include and exclude. It resolves wildcards so that if any inclusion | ||
| // rule is `*`, the included labels list evaluates simply to `["*"]`. | ||
| func ParseLabelFilter(filter string) (include []string, exclude []string) { | ||
| if filter == "" { | ||
| return nil, nil | ||
| } | ||
|
|
||
| var starMatched bool | ||
| for _, label := range strings.Split(filter, ",") { | ||
| label = strings.TrimSpace(label) | ||
| if label == "" { | ||
| continue | ||
| } | ||
|
|
||
| inc, exc := ParseLabel(label) | ||
|
|
||
| if inc != "" { | ||
| if inc == "*" { | ||
| starMatched = true | ||
| } else { | ||
| include = append(include, inc) | ||
| } | ||
| } | ||
| if exc != "" { | ||
| exclude = append(exclude, exc) | ||
| } | ||
| } | ||
|
|
||
| if starMatched { | ||
| include = []string{"*"} | ||
| } | ||
|
|
||
| return include, exclude | ||
| } | ||
|
|
||
| // ParseLabel parses a single label (without commas) into an include or exclude string. | ||
| // Returns the include label (if any) and the exclude label (if any). | ||
| func ParseLabel(label string) (include string, exclude string) { | ||
| if strings.HasPrefix(label, "!") { | ||
| return "", strings.TrimPrefix(label, "!") | ||
| } | ||
| return label, "" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| // SPDX-FileCopyrightText: Copyright 2026 The Minder Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package labels | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestParseLabelFilter(t *testing.T) { | ||
| t.Parallel() | ||
| tests := []struct { | ||
| name string | ||
| filter string | ||
| expectedInc []string | ||
| expectedExc []string | ||
| }{ | ||
| { | ||
| name: "empty", | ||
| filter: "", | ||
| }, | ||
| { | ||
| name: "single include", | ||
| filter: "foo", | ||
| expectedInc: []string{"foo"}, | ||
| }, | ||
| { | ||
| name: "single exclude", | ||
| filter: "!foo", | ||
| expectedExc: []string{"foo"}, | ||
| }, | ||
| { | ||
| name: "star include", | ||
| filter: "*", | ||
| expectedInc: []string{"*"}, | ||
| }, | ||
| { | ||
| name: "star exclude", | ||
| filter: "!*", | ||
| expectedExc: []string{"*"}, | ||
| }, | ||
| { | ||
| name: "multiple includes", | ||
| filter: "foo,bar", | ||
| expectedInc: []string{"foo", "bar"}, | ||
| }, | ||
| { | ||
| name: "includes and excludes", | ||
| filter: "foo,!bar,baz,!qux", | ||
| expectedInc: []string{"foo", "baz"}, | ||
| expectedExc: []string{"bar", "qux"}, | ||
| }, | ||
| { | ||
| name: "star mixed with includes", | ||
| filter: "foo,*", | ||
| expectedInc: []string{"*"}, | ||
| }, | ||
| { | ||
| name: "includes mixed with star", | ||
| filter: "*,foo", | ||
| expectedInc: []string{"*"}, | ||
| }, | ||
| { | ||
| name: "star and excludes", | ||
| filter: "*,!foo", | ||
| expectedInc: []string{"*"}, | ||
| expectedExc: []string{"foo"}, | ||
| }, | ||
| { | ||
| name: "whitespace handling", | ||
| filter: " foo , !bar ", | ||
| expectedInc: []string{"foo"}, | ||
| expectedExc: []string{"bar"}, | ||
| }, | ||
| { | ||
| name: "trailing commas", | ||
| filter: "foo,,!bar,", | ||
| expectedInc: []string{"foo"}, | ||
| expectedExc: []string{"bar"}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| tt := tt | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| inc, exc := ParseLabelFilter(tt.filter) | ||
| require.Equal(t, tt.expectedInc, inc) | ||
| require.Equal(t, tt.expectedExc, exc) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file looks like it might be a bad merge -- try
git checkout upstream/main internal/db/domain.go