Skip to content

Commit

Permalink
Allow audit streams for $JS.EVENT.> even if they do not have the no…
Browse files Browse the repository at this point in the history
…-ack flag set.

This exception is for pre-existing audit streams that would not recover on a server upgrade.

Signed-off-by: Derek Collison <[email protected]>
  • Loading branch information
derekcollison committed Jun 17, 2024
1 parent 2595172 commit 413a59f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
29 changes: 26 additions & 3 deletions server/jetstream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23621,23 +23621,32 @@ func TestJetStreamAuditStreams(t *testing.T) {
nc, js := jsClientConnect(t, s)
defer nc.Close()

jsOverlap := errors.New("subjects that overlap with jetstream api require no-ack to be true")
sysOverlap := errors.New("subjects that overlap with system api require no-ack to be true")

_, err := js.AddStream(&nats.StreamConfig{
Name: "TEST",
Subjects: []string{"$JS.>"},
})
require_Error(t, err, NewJSStreamInvalidConfigError(fmt.Errorf("subjects overlap with jetstream api")))
require_Error(t, err, NewJSStreamInvalidConfigError(jsOverlap))

_, err = js.AddStream(&nats.StreamConfig{
Name: "TEST",
Subjects: []string{"$JS.API.>"},
})
require_Error(t, err, NewJSStreamInvalidConfigError(jsOverlap))

_, err = js.AddStream(&nats.StreamConfig{
Name: "TEST",
Subjects: []string{"$JSC.>"},
})
require_Error(t, err, NewJSStreamInvalidConfigError(fmt.Errorf("subjects overlap with jetstream api")))
require_Error(t, err, NewJSStreamInvalidConfigError(jsOverlap))

_, err = js.AddStream(&nats.StreamConfig{
Name: "TEST",
Subjects: []string{"$SYS.>"},
})
require_Error(t, err, NewJSStreamInvalidConfigError(fmt.Errorf("subjects overlap with system api")))
require_Error(t, err, NewJSStreamInvalidConfigError(sysOverlap))

// These should be ok if no pub ack.
_, err = js.AddStream(&nats.StreamConfig{
Expand All @@ -23660,4 +23669,18 @@ func TestJetStreamAuditStreams(t *testing.T) {
NoAck: true,
})
require_NoError(t, err)

// Since prior behavior did allow $JS.EVENT to be captured without no-ack, these might break
// on a server upgrade so make sure they still work ok without --no-ack.

// Do avoid overlap.
err = js.DeleteStream("TEST1")
require_NoError(t, err)

_, err = js.AddStream(&nats.StreamConfig{
Name: "TEST4",
Subjects: []string{"$JS.EVENT.>"},
NoAck: true,
})
require_NoError(t, err)
}
7 changes: 5 additions & 2 deletions server/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -1504,11 +1504,14 @@ func (s *Server) checkStreamCfg(config *StreamConfig, acc *Account) (StreamConfi
}
// Also check to make sure we do not overlap with our $JS API subjects.
if !cfg.NoAck && (subjectIsSubsetMatch(subj, "$JS.>") || subjectIsSubsetMatch(subj, "$JSC.>")) {
return StreamConfig{}, NewJSStreamInvalidConfigError(fmt.Errorf("subjects overlap with jetstream api"))
// We allow an exception for $JS.EVENT.> since these could have been created in the past.
if !subjectIsSubsetMatch(subj, "$JS.EVENT.>") {
return StreamConfig{}, NewJSStreamInvalidConfigError(fmt.Errorf("subjects that overlap with jetstream api require no-ack to be true"))
}
}
// And the $SYS subjects.
if !cfg.NoAck && subjectIsSubsetMatch(subj, "$SYS.>") {
return StreamConfig{}, NewJSStreamInvalidConfigError(fmt.Errorf("subjects overlap with system api"))
return StreamConfig{}, NewJSStreamInvalidConfigError(fmt.Errorf("subjects that overlap with system api require no-ack to be true"))
}
// Mark for duplicate check.
dset[subj] = struct{}{}
Expand Down

0 comments on commit 413a59f

Please sign in to comment.