Skip to content

Conversation

neilalexander
Copy link
Member

This fixes a potential footgun with counter sourcing whereby you may have a subject transform from a single stream with a wildcard source but a non-wildcard destination, i.e. foo.> into bar. Without this, we would not correctly track foo.baz and foo.qux separately, causing a consistency error.

Signed-off-by: Neil Twigg [email protected]

…cing map

This fixes a potential footgun with counter sourcing whereby you may have
a subject transform from a single stream with a wildcard source but a non-wildcard
destination, i.e. `foo.>` into `bar`. Without this, we would not correctly track
`foo.baz` and `foo.qux` separately, causing a consistency error.

Signed-off-by: Neil Twigg <[email protected]>
@neilalexander neilalexander marked this pull request as ready for review July 18, 2025 15:57
@neilalexander neilalexander requested a review from a team as a code owner July 18, 2025 15:57
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this target 2.12, the PR mentions 2.11?

if len(fields) >= 3 {
origSubj = fields[2]
if len(fields) >= 5 {
origSubj = fields[4]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this remain compatible for non-2.12 source strings? If not longer or equal to 5, but longer than equal to 3, we would still use index 2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this but wasn't sure if it was needed, counters won't be originating from pre-2.12 servers.

if len(fields) >= 3 {
origSubj = fields[2]
if len(fields) >= 5 {
origSubj = fields[4]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here for compatibilty with previous/shorter version and during upgrades?

@neilalexander neilalexander changed the title (2.11) Include original subject in Nats-Stream-Source, use in counter sourcing map (2.12) Include original subject in Nats-Stream-Source, use in counter sourcing map Jul 18, 2025
@neilalexander
Copy link
Member Author

Typo, fixed.

Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@neilalexander neilalexander merged commit 1010b3e into main Jul 21, 2025
68 of 70 checks passed
@neilalexander neilalexander deleted the neil/countersourcing branch July 21, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants