Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## [Unreleased]
### Added
- Schema: generic `@decorator` syntax in the schema DSL. This change is machinery only — no decorator is defined yet, and the shipped registry is empty.

### Changed
- Schema compilation: reduce memory usage when caveats are involved (https://github.com/authzed/spicedb/pull/3266)
- Schema: `use` flags are now validated against the deployment's allowed set at compile time. A bare `use import` (or `use expiration` where expiration is disabled) submitted to `WriteSchema` now errors where it previously compiled, because `WriteSchema` applies `DisallowImportFlag()` unconditionally.
- `generator.GenerateCaveatSource` changed signature from `(string, bool, error)` to `(string, []string, bool, error)`; it previously discarded the `use` flags a caveat requires. This is a breaking change for downstream Go consumers.

### Fixed
- Postgres: read replicas no longer intermittently return `object definition not found` under load. The strict read-replica guard now verifies that the replica's snapshot has caught up to the revision being read (snapshot domination) instead of checking a single transaction id, and raises from within the read itself rather than from a trailing assertion, so a replica that catches up mid-query can no longer let an incomplete read through. In both cases the read correctly falls back to the primary. (https://github.com/authzed/spicedb/pull/3243)
Expand Down
8 changes: 7 additions & 1 deletion internal/datastore/proxy/schemacaching/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@ import "github.com/authzed/spicedb/pkg/schemadsl/compiler"
// on-wire size, as returned by SizeVT. This was determined by testing
// all existing definitions found in consistency tests and is
// enforced via the estimatedsize_test.
//
// NOTE: the namespace multiplier was raised from 10 to 12 when decorators were
// added. A nil `Decorators` slice costs 24 bytes in memory on NamespaceDefinition,
// Relation and AllowedRelation, but contributes nothing to SizeVT, so the in-memory
// cost of a definition grew while its on-wire size did not. Any future field added
// to those messages has the same effect and may require raising this again.
const (
namespaceDefinitionSizeVTMultiplier = 10
namespaceDefinitionSizeVTMultiplier = 12
namespaceDefinitionMinimumSize = 150

caveatDefinitionSizeVTMultiplier = 10
Expand Down
10 changes: 10 additions & 0 deletions internal/services/v1/expreflection.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,11 @@ func expConvertDiff(
case nsdiff.NamespaceRemoved:
return nil, spiceerrors.MustBugf("should be handled above")

case nsdiff.NamespaceDecoratorsChanged, nsdiff.RelationDecoratorsChanged:
// TODO: surface a decorator diff message once a decorator actually
// ships; there is no reflection API representation for one yet.
continue

default:
return nil, spiceerrors.MustBugf("unexpected delta type %v", delta.Type)
}
Expand Down Expand Up @@ -513,6 +518,11 @@ func expConvertDiff(
case caveatdiff.CaveatRemoved:
return nil, spiceerrors.MustBugf("should be handled above")

case caveatdiff.CaveatDecoratorsChanged:
// TODO: surface a decorator diff message once a decorator actually
// ships; there is no reflection API representation for one yet.
continue

default:
return nil, spiceerrors.MustBugf("unexpected delta type %v", delta.Type)
}
Expand Down
70 changes: 70 additions & 0 deletions internal/services/v1/expreflection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/authzed/spicedb/pkg/diff"
"github.com/authzed/spicedb/pkg/genutil/mapz"
"github.com/authzed/spicedb/pkg/schemadsl/compiler"
"github.com/authzed/spicedb/pkg/schemadsl/decorators"
"github.com/authzed/spicedb/pkg/schemadsl/input"
"github.com/authzed/spicedb/pkg/testutil"
)
Expand Down Expand Up @@ -593,6 +594,75 @@ func TestExpConvertDiff(t *testing.T) {
}
}

// TestExpConvertDiffDecoratorOnlyChanges ensures that a schema change consisting solely of
// decorator changes (on a namespace, a relation and a caveat) is handled by expConvertDiff
// without hitting the `default` arm's spiceerrors.MustBugf, which panics in test binaries.
// The reflection API has no representation for a decorator diff yet, so the expected
// result is simply an empty diff list, not a panic.
func TestExpConvertDiffDecoratorOnlyChanges(t *testing.T) {
t.Cleanup(func() {
goleak.VerifyNone(t, testutil.GoLeakIgnores()...)
})

existingSchema, err := compiler.Compile(compiler.InputSchema{
Source: input.Source("schema"),
SchemaString: `use testdecorators

caveat somecaveat(somevalue int) {
somevalue == 42
}

definition user {}

definition resource {
relation viewer: user
}`,
}, compiler.AllowUnprefixedObjectType(), compiler.WithDecoratorRegistry(decorators.TestRegistry))
require.NoError(t, err)

comparisonSchema, err := compiler.Compile(compiler.InputSchema{
Source: input.Source("schema"),
SchemaString: `use testdecorators

@testcaveat
caveat somecaveat(somevalue int) {
somevalue == 42
}

@testdef
definition user {}

definition resource {
@testrel
relation viewer: user
}`,
}, compiler.AllowUnprefixedObjectType(), compiler.WithDecoratorRegistry(decorators.TestRegistry))
require.NoError(t, err)

es := diff.NewDiffableSchemaFromCompiledSchema(existingSchema)
cs := diff.NewDiffableSchemaFromCompiledSchema(comparisonSchema)

schemaDiff, err := diff.DiffSchemas(es, cs, caveattypes.Default.TypeSet)
require.NoError(t, err)

dl, err := dsfortesting.DataLayerForTesting(t, 100, 1*time.Second, 100*time.Minute)
require.NoError(t, err)

ctx := t.Context()
ctx = datalayer.ContextWithDataLayer(ctx, dl)

resp, err := expConvertDiff(
ctx,
schemaDiff,
&es,
&cs,
revisionparsing.MustParseRevisionForTest("1"),
caveattypes.Default.TypeSet,
)
require.NoError(t, err)
require.Empty(t, resp.Diffs)
}

type expFilterCheck func(sf *expSchemaFilters) bool

func TestExpSchemaFiltering(t *testing.T) {
Expand Down
10 changes: 10 additions & 0 deletions internal/services/v1/reflectionapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,11 @@ func convertDiff(
case nsdiff.NamespaceRemoved:
return nil, spiceerrors.MustBugf("should be handled above")

case nsdiff.NamespaceDecoratorsChanged, nsdiff.RelationDecoratorsChanged:
// TODO: surface a decorator diff message once a decorator actually
// ships; there is no reflection API representation for one yet.
continue

default:
return nil, spiceerrors.MustBugf("unexpected delta type %v", delta.Type)
}
Expand Down Expand Up @@ -513,6 +518,11 @@ func convertDiff(
case caveatdiff.CaveatRemoved:
return nil, spiceerrors.MustBugf("should be handled above")

case caveatdiff.CaveatDecoratorsChanged:
// TODO: surface a decorator diff message once a decorator actually
// ships; there is no reflection API representation for one yet.
continue

default:
return nil, spiceerrors.MustBugf("unexpected delta type %v", delta.Type)
}
Expand Down
66 changes: 66 additions & 0 deletions internal/services/v1/reflectionapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/authzed/spicedb/pkg/diff"
"github.com/authzed/spicedb/pkg/genutil/mapz"
"github.com/authzed/spicedb/pkg/schemadsl/compiler"
"github.com/authzed/spicedb/pkg/schemadsl/decorators"
"github.com/authzed/spicedb/pkg/schemadsl/input"
"github.com/authzed/spicedb/pkg/testutil"
)
Expand Down Expand Up @@ -591,6 +592,71 @@ func TestConvertDiff(t *testing.T) {
}
}

// TestConvertDiffDecoratorOnlyChanges ensures that a schema change consisting solely of
// decorator changes (on a namespace, a relation and a caveat) is handled by convertDiff
// without hitting the `default` arm's spiceerrors.MustBugf, which panics in test binaries.
// The reflection API has no representation for a decorator diff yet, so the expected
// result is simply an empty diff list, not a panic.
func TestConvertDiffDecoratorOnlyChanges(t *testing.T) {
existingSchema, err := compiler.Compile(compiler.InputSchema{
Source: input.Source("schema"),
SchemaString: `use testdecorators

caveat somecaveat(somevalue int) {
somevalue == 42
}

definition user {}

definition resource {
relation viewer: user
}`,
}, compiler.AllowUnprefixedObjectType(), compiler.WithDecoratorRegistry(decorators.TestRegistry))
require.NoError(t, err)

comparisonSchema, err := compiler.Compile(compiler.InputSchema{
Source: input.Source("schema"),
SchemaString: `use testdecorators

@testcaveat
caveat somecaveat(somevalue int) {
somevalue == 42
}

@testdef
definition user {}

definition resource {
@testrel
relation viewer: user
}`,
}, compiler.AllowUnprefixedObjectType(), compiler.WithDecoratorRegistry(decorators.TestRegistry))
require.NoError(t, err)

es := diff.NewDiffableSchemaFromCompiledSchema(existingSchema)
cs := diff.NewDiffableSchemaFromCompiledSchema(comparisonSchema)

schemaDiff, err := diff.DiffSchemas(es, cs, caveattypes.Default.TypeSet)
require.NoError(t, err)

dl, err := dsfortesting.DataLayerForTesting(t, 100, 1*time.Second, 100*time.Minute)
require.NoError(t, err)

ctx := t.Context()
ctx = datalayer.ContextWithDataLayer(ctx, dl)

resp, err := convertDiff(
ctx,
schemaDiff,
&es,
&cs,
revisionparsing.MustParseRevisionForTest("1"),
caveattypes.Default.TypeSet,
)
require.NoError(t, err)
require.Empty(t, resp.Diffs)
}

type filterCheck func(sf *schemaFilters) bool

func TestSchemaFiltering(t *testing.T) {
Expand Down
17 changes: 17 additions & 0 deletions pkg/diff/caveats/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"maps"
"slices"

"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/testing/protocmp"

caveattypes "github.com/authzed/spicedb/pkg/caveats/types"
"github.com/authzed/spicedb/pkg/genutil/mapz"
nspkg "github.com/authzed/spicedb/pkg/namespace"
Expand Down Expand Up @@ -35,6 +38,9 @@ const (

// CaveatExpressionChanged indicates that the expression of the caveat has changed.
CaveatExpressionChanged DeltaType = "expression-has-changed"

// CaveatDecoratorsChanged indicates that the decorators on the caveat changed.
CaveatDecoratorsChanged DeltaType = "caveat-decorators-changed"
)

// Diff holds the diff between two caveats.
Expand Down Expand Up @@ -107,6 +113,10 @@ func DiffCaveats(existing *core.CaveatDefinition, updated *core.CaveatDefinition
})
}

if areDifferentDecorators(existing.GetDecorators(), updated.GetDecorators()) {
deltas = append(deltas, Delta{Type: CaveatDecoratorsChanged})
}

existingParameterNames := mapz.NewSet(slices.Collect(maps.Keys(existing.ParameterTypes))...)
updatedParameterNames := mapz.NewSet(slices.Collect(maps.Keys(updated.ParameterTypes))...)

Expand Down Expand Up @@ -161,3 +171,10 @@ func DiffCaveats(existing *core.CaveatDefinition, updated *core.CaveatDefinition
deltas: deltas,
}, nil
}

// areDifferentDecorators returns whether the two sets of decorators differ. As with
// namespace and relation decorators, comparison is order-sensitive: decorators are stored
// in source order, and a reordering is a meaningful source-level change.
func areDifferentDecorators(existing []*core.Decorator, updated []*core.Decorator) bool {
return cmp.Diff(existing, updated, protocmp.Transform()) != ""
}
38 changes: 38 additions & 0 deletions pkg/diff/caveats/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,41 @@ func TestCaveatDiff(t *testing.T) {
})
}
}

func TestDiffCaveatDecorators(t *testing.T) {
t.Parallel()

withDecorator := func(name string) *core.Decorator {
return &core.Decorator{Name: name, RequiredFlag: "testdecorators"}
}

newCaveat := func(decorators ...*core.Decorator) *core.CaveatDefinition {
cd := ns.MustCaveatDefinition(
caveats.MustEnvForVariablesWithDefaultTypeSet(map[string]caveattypes.VariableType{
"someparam": caveattypes.Default.IntType,
}),
"somecaveat",
"true",
)
cd.Decorators = decorators
return cd
}

t.Run("caveat decorator added", func(t *testing.T) {
t.Parallel()
diff, err := DiffCaveats(newCaveat(), newCaveat(withDecorator("testcaveat")), caveattypes.Default.TypeSet)
require.NoError(t, err)
require.Equal(t, []Delta{{Type: CaveatDecoratorsChanged}}, diff.Deltas())
})

t.Run("no decorator change produces no delta", func(t *testing.T) {
t.Parallel()
diff, err := DiffCaveats(
newCaveat(withDecorator("testcaveat")),
newCaveat(withDecorator("testcaveat")),
caveattypes.Default.TypeSet,
)
require.NoError(t, err)
require.Empty(t, diff.Deltas())
})
}
Loading
Loading