diff --git a/pkg/caveats/compile.go b/pkg/caveats/compile.go index bde2275cf3..821b311d4f 100644 --- a/pkg/caveats/compile.go +++ b/pkg/caveats/compile.go @@ -6,6 +6,7 @@ import ( "github.com/authzed/cel-go/cel" "github.com/authzed/cel-go/common" + "google.golang.org/protobuf/proto" "github.com/authzed/spicedb/pkg/caveats/types" "github.com/authzed/spicedb/pkg/genutil/mapz" @@ -52,7 +53,9 @@ func (cc CompiledCaveat) Serialize() ([]byte, error) { Name: cc.name, } - return caveat.MarshalVT() + // TODO(jschorr): change back to MarshalVT once stable is supported. + // See: https://github.com/planetscale/vtprotobuf/pull/133 + return proto.MarshalOptions{Deterministic: true}.Marshal(caveat) } // ReferencedParameters returns the names of the parameters referenced in the expression. diff --git a/pkg/diff/caveats/diff.go b/pkg/diff/caveats/diff.go index d45e6ae911..e2a25ce8c0 100644 --- a/pkg/diff/caveats/diff.go +++ b/pkg/diff/caveats/diff.go @@ -34,10 +34,8 @@ const ( // ParameterTypeChanged indicates that the type of the parameter was changed. ParameterTypeChanged DeltaType = "parameter-type-changed" - // CaveatExpressionMayHaveChanged indicates that the expression of the caveat *may* have changed. - // This uses a direct byte comparison which can return that a change occurred, even when it has - // not. - CaveatExpressionMayHaveChanged DeltaType = "expression-may-have-changed" + // CaveatExpressionChanged indicates that the expression of the caveat has changed. + CaveatExpressionChanged DeltaType = "expression-has-changed" ) // Diff holds the diff between two caveats. @@ -154,7 +152,7 @@ func DiffCaveats(existing *core.CaveatDefinition, updated *core.CaveatDefinition if !bytes.Equal(existing.SerializedExpression, updated.SerializedExpression) { deltas = append(deltas, Delta{ - Type: CaveatExpressionMayHaveChanged, + Type: CaveatExpressionChanged, }) } diff --git a/pkg/diff/caveats/diff_test.go b/pkg/diff/caveats/diff_test.go index 5469a00912..5359cfa326 100644 --- a/pkg/diff/caveats/diff_test.go +++ b/pkg/diff/caveats/diff_test.go @@ -187,7 +187,7 @@ func TestCaveatDiff(t *testing.T) { "false", ), []Delta{ - {Type: CaveatExpressionMayHaveChanged}, + {Type: CaveatExpressionChanged}, }, }, { @@ -207,9 +207,65 @@ func TestCaveatDiff(t *testing.T) { "someparam <= 1", ), []Delta{ - {Type: CaveatExpressionMayHaveChanged}, + {Type: CaveatExpressionChanged}, }, }, + { + "third changed expression", + ns.MustCaveatDefinition( + caveats.MustEnvForVariables(map[string]types.VariableType{ + "someparam": types.IntType, + }), + "somecaveat", + "someparam == 1", + ), + ns.MustCaveatDefinition( + caveats.MustEnvForVariables(map[string]types.VariableType{ + "someparam": types.IntType, + }), + "somecaveat", + "someparam == 2", + ), + []Delta{ + {Type: CaveatExpressionChanged}, + }, + }, + { + "unchanged expression", + ns.MustCaveatDefinition( + caveats.MustEnvForVariables(map[string]types.VariableType{ + "someparam": types.IntType, + }), + "somecaveat", + "someparam == 1", + ), + ns.MustCaveatDefinition( + caveats.MustEnvForVariables(map[string]types.VariableType{ + "someparam": types.IntType, + }), + "somecaveat", + "someparam == 1", + ), + []Delta{}, + }, + { + "unchanged slightly more complex expression", + ns.MustCaveatDefinition( + caveats.MustEnvForVariables(map[string]types.VariableType{ + "someparam": types.IntType, + }), + "somecaveat", + "someparam == {\"a\": 42}.a", + ), + ns.MustCaveatDefinition( + caveats.MustEnvForVariables(map[string]types.VariableType{ + "someparam": types.IntType, + }), + "somecaveat", + "someparam == {\"a\": 42}.a", + ), + []Delta{}, + }, } for _, tc := range testCases {