Skip to content

Commit 9c63594

Browse files
committed
kvserver/spanset: introduce forbidden spans matchers
This commit introduces the concept of forbidden spans, which will allow us to assert our batches don't modify Raft's engine keys. References: #156537 Release note: None
1 parent 2b759bf commit 9c63594

File tree

2 files changed

+78
-16
lines changed

2 files changed

+78
-16
lines changed

pkg/kv/kvserver/spanset/batch.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,35 @@ func DisableReadWriterAssertions(rw storage.ReadWriter) storage.ReadWriter {
888888
}
889889
}
890890

891+
// DisableForbiddenSpanAssertionsOnBatch returns a new batch wrapper with
892+
// forbidden span assertions disabled. It does not modify the original batch.
893+
// The returned batch shares the same underlying storage.Batch but has its own
894+
// SpanSet copies with assertions disabled.
895+
func DisableForbiddenSpanAssertionsOnBatch(rw storage.ReadWriter) storage.ReadWriter {
896+
switch v := rw.(type) {
897+
case *spanSetBatch:
898+
// Clone the span sets and disable assertions on the copies
899+
readSpans := v.ReadWriter.spanSetReader.spans.Copy()
900+
writeSpans := v.ReadWriter.spanSetWriter.spans.Copy()
901+
readSpans.DisableForbiddenSpansAssertions()
902+
writeSpans.DisableForbiddenSpansAssertions()
903+
904+
// Create a new spanSetBatch with the modified span sets.
905+
return &spanSetBatch{
906+
ReadWriter: ReadWriter{
907+
spanSetReader: spanSetReader{r: v.b, spans: readSpans, spansOnly: v.spansOnly, ts: v.ts},
908+
spanSetWriter: spanSetWriter{w: v.b, spans: writeSpans, spansOnly: v.spansOnly, ts: v.ts},
909+
},
910+
b: v.b,
911+
spans: readSpans, // Use the read spans as the main spans
912+
spansOnly: v.spansOnly,
913+
ts: v.ts,
914+
}
915+
default:
916+
return rw
917+
}
918+
}
919+
891920
// addLockTableSpans adds corresponding lock table spans for the declared
892921
// spans. This is to implicitly allow raw access to separated intents in the
893922
// lock table for any declared keys. Explicitly declaring lock table spans is

pkg/kv/kvserver/spanset/spanset.go

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,14 @@ type Span struct {
8383
// The Span slice for a particular access and scope contains non-overlapping
8484
// spans in increasing key order after calls to SortAndDedup.
8585
type SpanSet struct {
86-
spans [NumSpanAccess][NumSpanScope][]Span
87-
allowUndeclared bool
86+
spans [NumSpanAccess][NumSpanScope][]Span
87+
// forbiddenSpansMatchers are functions that return an error if the given span
88+
// shouldn't be accessed (forbidden). This allows for complex pattern matching
89+
// like forbidding specific keys across all range IDs without enumerating them
90+
// explicitly.
91+
forbiddenSpansMatchers []func(roachpb.Span) error
92+
allowUndeclared bool
93+
allowForbidden bool
8894
}
8995

9096
var spanSetPool = sync.Pool{
@@ -113,6 +119,8 @@ func (s *SpanSet) Release() {
113119
s.spans[sa][ss] = recycle
114120
}
115121
}
122+
s.forbiddenSpansMatchers = nil
123+
s.allowForbidden = false
116124
s.allowUndeclared = false
117125
spanSetPool.Put(s)
118126
}
@@ -155,7 +163,9 @@ func (s *SpanSet) Copy() *SpanSet {
155163
n.spans[sa][ss] = append(n.spans[sa][ss], s.spans[sa][ss]...)
156164
}
157165
}
166+
n.forbiddenSpansMatchers = append(n.forbiddenSpansMatchers, s.forbiddenSpansMatchers...)
158167
n.allowUndeclared = s.allowUndeclared
168+
n.allowForbidden = s.allowForbidden
159169
return n
160170
}
161171

@@ -201,14 +211,22 @@ func (s *SpanSet) AddMVCC(access SpanAccess, span roachpb.Span, timestamp hlc.Ti
201211
s.spans[access][scope] = append(s.spans[access][scope], Span{Span: span, Timestamp: timestamp})
202212
}
203213

214+
// AddForbiddenMatcher adds a forbidden span matcher. The matcher is a function
215+
// that is called for each span access to check if it should be forbidden.
216+
func (s *SpanSet) AddForbiddenMatcher(matcher func(roachpb.Span) error) {
217+
s.forbiddenSpansMatchers = append(s.forbiddenSpansMatchers, matcher)
218+
}
219+
204220
// Merge merges all spans in s2 into s. s2 is not modified.
205221
func (s *SpanSet) Merge(s2 *SpanSet) {
206222
for sa := SpanAccess(0); sa < NumSpanAccess; sa++ {
207223
for ss := SpanScope(0); ss < NumSpanScope; ss++ {
208224
s.spans[sa][ss] = append(s.spans[sa][ss], s2.spans[sa][ss]...)
209225
}
210226
}
227+
s.forbiddenSpansMatchers = append(s.forbiddenSpansMatchers, s2.forbiddenSpansMatchers...)
211228
s.allowUndeclared = s2.allowUndeclared
229+
s.allowForbidden = s2.allowForbidden
212230
s.SortAndDedup()
213231
}
214232

@@ -331,27 +349,37 @@ func (s *SpanSet) CheckAllowedAt(
331349
func (s *SpanSet) checkAllowed(
332350
access SpanAccess, span roachpb.Span, check func(SpanAccess, Span) bool,
333351
) error {
334-
if s.allowUndeclared {
335-
// If the request has specified that undeclared spans are allowed, do
336-
// nothing.
337-
return nil
352+
// Unless explicitly disabled, check if we access any forbidden spans.
353+
if !s.allowForbidden {
354+
// Check if the span is forbidden.
355+
for _, matcher := range s.forbiddenSpansMatchers {
356+
if err := matcher(span); err != nil {
357+
return errors.Errorf("cannot %s span %s: matches forbidden pattern",
358+
access, span)
359+
}
360+
}
338361
}
339362

340-
scope := SpanGlobal
341-
if (span.Key != nil && keys.IsLocal(span.Key)) ||
342-
(span.EndKey != nil && keys.IsLocal(span.EndKey)) {
343-
scope = SpanLocal
344-
}
363+
// Unless explicitly disabled, check if we access any undeclared spans.
364+
if !s.allowUndeclared {
365+
scope := SpanGlobal
366+
if (span.Key != nil && keys.IsLocal(span.Key)) ||
367+
(span.EndKey != nil && keys.IsLocal(span.EndKey)) {
368+
scope = SpanLocal
369+
}
345370

346-
for ac := access; ac < NumSpanAccess; ac++ {
347-
for _, cur := range s.spans[ac][scope] {
348-
if contains(cur.Span, span) && check(ac, cur) {
349-
return nil
371+
for ac := access; ac < NumSpanAccess; ac++ {
372+
for _, cur := range s.spans[ac][scope] {
373+
if contains(cur.Span, span) && check(ac, cur) {
374+
return nil
375+
}
350376
}
351377
}
378+
379+
return errors.Errorf("cannot %s undeclared span %s\ndeclared:\n%s\nstack:\n%s", access, span, s, debugutil.Stack())
352380
}
353381

354-
return errors.Errorf("cannot %s undeclared span %s\ndeclared:\n%s\nstack:\n%s", access, span, s, debugutil.Stack())
382+
return nil
355383
}
356384

357385
// contains returns whether s1 contains s2. Unlike Span.Contains, this function
@@ -396,3 +424,8 @@ func (s *SpanSet) Validate() error {
396424
func (s *SpanSet) DisableUndeclaredAccessAssertions() {
397425
s.allowUndeclared = true
398426
}
427+
428+
// DisableForbiddenAssertions disables forbidden spans assertions.
429+
func (s *SpanSet) DisableForbiddenSpansAssertions() {
430+
s.allowForbidden = true
431+
}

0 commit comments

Comments
 (0)