From 98b5df79ab85cc1bb348800cc897ac229278cf81 Mon Sep 17 00:00:00 2001 From: strantalis Date: Thu, 4 Dec 2025 17:40:49 -0500 Subject: [PATCH 1/6] perf(authz): optimize subject mapping evaluation performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add performance optimizations to subject mapping evaluation hot paths: - Flattening: Add internal index map for O(1) selector lookups - GetFromFlattened_Large: 39x faster (217ns → 5.5ns), zero allocs - GetFromFlattened_MultipleLookups: 17x faster, zero allocs - EvaluateCondition: Set-based IN/NOT_IN with early returns - EvaluateCondition_IN_Large: 2.3x faster - EvaluateCondition_IN_Hit: 4.6x faster, zero allocs - Per-entity caches for SubjectSet/ConditionGroup evaluation - RepeatedSubjectSets: 21% faster, 37% fewer allocs - Action deduplication using per-valueFQN accumulator - WithActions: 6% faster, 51% fewer allocations - HierarchyRule: O(h) definition scan vs O(e) entitlements scan Trade-off: Flatten() is ~2x slower due to index building, but this cost is amortized over many GetFromFlattened calls per entity. --- lib/flattening/flatten.go | 47 ++-- lib/flattening/flatten_bench_large_test.go | 73 ++++++ lib/flattening/flatten_test.go | 3 +- service/internal/access/v2/evaluate.go | 30 ++- .../subject_mapping_bench_large_test.go | 218 ++++++++++++++++++ .../subject_mapping_builtin.go | 125 ++++++---- .../subject_mapping_builtin_actions.go | 39 ++-- 7 files changed, 447 insertions(+), 88 deletions(-) create mode 100644 lib/flattening/flatten_bench_large_test.go create mode 100644 service/internal/subjectmappingbuiltin/subject_mapping_bench_large_test.go diff --git a/lib/flattening/flatten.go b/lib/flattening/flatten.go index 3424014b7c..c8ef57a8bc 100644 --- a/lib/flattening/flatten.go +++ b/lib/flattening/flatten.go @@ -7,6 +7,8 @@ import ( type Flattened struct { Items []Item `json:"flattened"` + // index provides O(1) selector lookups; populated by Flatten + index map[string][]interface{} } type Item struct { @@ -15,6 +17,14 @@ type Item struct { } func GetFromFlattened(flat Flattened, selector string) []interface{} { + // Fast-path: use prebuilt index for O(1) lookup + if flat.index != nil { + if vals, ok := flat.index[selector]; ok { + return vals + } + return nil + } + // Fallback for backwards compatibility (e.g., if Flattened was created without Flatten) itemsToReturn := []interface{}{} for _, item := range flat.Items { if item.Key == selector { @@ -25,43 +35,52 @@ func GetFromFlattened(flat Flattened, selector string) []interface{} { } func Flatten(m map[string]interface{}) (Flattened, error) { - flattened := Flattened{} - items, err := flattenInterface(m) + idx := make(map[string][]interface{}) + items, err := flattenInterface(m, idx) if err != nil { return Flattened{}, err } - flattened.Items = items - return flattened, nil + return Flattened{ + Items: items, + index: idx, + }, nil } -func flattenInterface(i interface{}) ([]Item, error) { +func flattenInterface(i interface{}, idx map[string][]interface{}) ([]Item, error) { o := []Item{} switch child := i.(type) { case map[string]interface{}: for k, v := range child { - nm, err := flattenInterface(v) + nm, err := flattenInterface(v, idx) if err != nil { return nil, err } for _, item := range nm { - o = append(o, Item{Key: "." + k + item.Key, Value: item.Value}) + key := "." + k + item.Key + o = append(o, Item{Key: key, Value: item.Value}) + idx[key] = append(idx[key], item.Value) } } case []interface{}: - for idx, item := range child { - k := fmt.Sprintf("[%v]", idx) - k2 := "[]" - flattenedItem, err := flattenInterface(item) + for index, item := range child { + kIdx := fmt.Sprintf("[%v]", index) + kAny := "[]" + flattenedItem, err := flattenInterface(item, idx) if err != nil { return nil, err } - for _, item := range flattenedItem { - o = append(o, Item{Key: k + item.Key, Value: item.Value}) - o = append(o, Item{Key: k2 + item.Key, Value: item.Value}) + for _, it := range flattenedItem { + keyIdx := kIdx + it.Key + keyAny := kAny + it.Key + o = append(o, Item{Key: keyIdx, Value: it.Value}) + o = append(o, Item{Key: keyAny, Value: it.Value}) + idx[keyIdx] = append(idx[keyIdx], it.Value) + idx[keyAny] = append(idx[keyAny], it.Value) } } case bool, int, string, float64, float32: o = append(o, Item{Key: "", Value: child}) + idx[""] = append(idx[""], child) default: return nil, errors.New("unrecognized item in json") } diff --git a/lib/flattening/flatten_bench_large_test.go b/lib/flattening/flatten_bench_large_test.go new file mode 100644 index 0000000000..ccb78d024f --- /dev/null +++ b/lib/flattening/flatten_bench_large_test.go @@ -0,0 +1,73 @@ +package flattening + +import ( + "fmt" + "testing" +) + +// Benchmark with larger dataset to show lookup performance +func BenchmarkGetFromFlattened_Large(b *testing.B) { + // Create a larger flattened structure via Flatten + largeInput := make(map[string]interface{}) + for i := 0; i < 100; i++ { + largeInput[fmt.Sprintf("key%d", i)] = fmt.Sprintf("value%d", i) + } + flatInput, err := Flatten(largeInput) + if err != nil { + b.Fatal(err) + } + + // Query for a key in the middle + queryString := ".key50" + b.ResetTimer() + for n := 0; n < b.N; n++ { + _ = GetFromFlattened(flatInput, queryString) + } +} + +// Benchmark multiple lookups on same flattened entity +func BenchmarkGetFromFlattened_MultipleLookups(b *testing.B) { + largeInput := make(map[string]interface{}) + for i := 0; i < 50; i++ { + largeInput[fmt.Sprintf("attr%d", i)] = fmt.Sprintf("value%d", i) + } + flatInput, err := Flatten(largeInput) + if err != nil { + b.Fatal(err) + } + + queries := []string{".attr0", ".attr10", ".attr25", ".attr40", ".attr49"} + b.ResetTimer() + for n := 0; n < b.N; n++ { + for _, q := range queries { + _ = GetFromFlattened(flatInput, q) + } + } +} + +// Benchmark with nested structure (common in entity representations) +func BenchmarkFlatten_NestedEntity(b *testing.B) { + nestedInput := map[string]interface{}{ + "user": map[string]interface{}{ + "id": "user123", + "name": "Test User", + "email": "test@example.com", + "attributes": map[string]interface{}{ + "department": "Engineering", + "level": "Senior", + "groups": []interface{}{"group1", "group2", "group3"}, + }, + }, + "roles": []interface{}{ + map[string]interface{}{"name": "admin", "scope": "global"}, + map[string]interface{}{"name": "reader", "scope": "local"}, + }, + } + b.ResetTimer() + for n := 0; n < b.N; n++ { + _, err := Flatten(nestedInput) + if err != nil { + b.Fatal(err) + } + } +} diff --git a/lib/flattening/flatten_test.go b/lib/flattening/flatten_test.go index 90bac4b9bf..16c84c1451 100644 --- a/lib/flattening/flatten_test.go +++ b/lib/flattening/flatten_test.go @@ -344,7 +344,8 @@ func TestFlattenInterfaceNoPanic(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { require.NotPanics(t, func() { - _, _ = flattenInterface(tc.value) + idx := make(map[string][]interface{}) + _, _ = flattenInterface(tc.value, idx) }) }) } diff --git a/service/internal/access/v2/evaluate.go b/service/internal/access/v2/evaluate.go index e787fa62ec..52b2423e10 100644 --- a/service/internal/access/v2/evaluate.go +++ b/service/internal/access/v2/evaluate.go @@ -367,25 +367,37 @@ func hierarchyRule( } } - // Check if the entitlements contain any values with index <= lowestValueFQNIndex - // This checks the requested value and any hierarchically higher values in a single pass - O(e) where e is entitlements count - for entitlementFQN, entitledActions := range entitlements { - // Check if this entitlement FQN has a valid index in the hierarchy - if idx, exists := valueFQNToIndex[entitlementFQN]; exists && idx <= lowestValueFQNIndex { - // Check if the required action is entitled + // If we didn't find any matching value indices, fail with all as missing + if lowestValueFQNIndex == len(attrValues) { + failures := make([]EntitlementFailure, 0, len(resourceValueFQNs)) + for _, fqn := range resourceValueFQNs { + failures = append(failures, EntitlementFailure{ + AttributeValueFQN: fqn, + ActionName: actionName, + }) + } + return failures + } + + // Check entitlement at or above the hierarchy level: + // Scan only the FQNs in the definition up to the highest requested index (O(h)) + // This is more efficient than scanning all entitlements (O(e)) when e >> h + for i := 0; i <= lowestValueFQNIndex; i++ { + candidateFQN := attrValues[i].GetFqn() + if entitledActions, ok := entitlements[candidateFQN]; ok { for _, entitledAction := range entitledActions { if strings.EqualFold(entitledAction.GetName(), actionName) { l.DebugContext(ctx, "hierarchy rule satisfied", slog.Group("entitled_by_value", - slog.String("FQN", entitlementFQN), - slog.Int("index", idx), + slog.String("FQN", candidateFQN), + slog.Int("index", i), ), slog.Group("resource_highest_hierarchy_value", slog.String("FQN", attrValues[lowestValueFQNIndex].GetFqn()), slog.Int("index", lowestValueFQNIndex), ), ) - return nil // Found an entitled action at or above the hierarchy level, no failures + return nil } } } diff --git a/service/internal/subjectmappingbuiltin/subject_mapping_bench_large_test.go b/service/internal/subjectmappingbuiltin/subject_mapping_bench_large_test.go new file mode 100644 index 0000000000..e910fcf7a5 --- /dev/null +++ b/service/internal/subjectmappingbuiltin/subject_mapping_bench_large_test.go @@ -0,0 +1,218 @@ +package subjectmappingbuiltin_test + +import ( + "fmt" + "testing" + + "github.com/opentdf/platform/lib/flattening" + "github.com/opentdf/platform/protocol/go/entityresolution" + entityresolutionV2 "github.com/opentdf/platform/protocol/go/entityresolution/v2" + "github.com/opentdf/platform/protocol/go/policy" + "github.com/opentdf/platform/protocol/go/policy/attributes" + "github.com/opentdf/platform/service/internal/subjectmappingbuiltin" + "google.golang.org/protobuf/types/known/structpb" +) + +// Helper to generate attribute mappings with configurable size +func generateAttributeMappings(attrCount, smPerAttr, condsPerSM int) map[string]*attributes.GetAttributeValuesByFqnsResponse_AttributeAndValue { + mappings := make(map[string]*attributes.GetAttributeValuesByFqnsResponse_AttributeAndValue, attrCount) + + for i := 0; i < attrCount; i++ { + valueFQN := fmt.Sprintf("https://demo.org/attr/attr%d/value/val%d", i, i) + + subjectMappings := make([]*policy.SubjectMapping, smPerAttr) + for j := 0; j < smPerAttr; j++ { + conditions := make([]*policy.Condition, condsPerSM) + for k := 0; k < condsPerSM; k++ { + conditions[k] = &policy.Condition{ + SubjectExternalSelectorValue: ".attributes.groups[]", + Operator: policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN, + SubjectExternalValues: []string{fmt.Sprintf("group%d", k)}, + } + } + + subjectMappings[j] = &policy.SubjectMapping{ + SubjectConditionSet: &policy.SubjectConditionSet{ + SubjectSets: []*policy.SubjectSet{ + { + ConditionGroups: []*policy.ConditionGroup{ + { + BooleanOperator: policy.ConditionBooleanTypeEnum_CONDITION_BOOLEAN_TYPE_ENUM_OR, + Conditions: conditions, + }, + }, + }, + }, + }, + Actions: []*policy.Action{ + {Name: "read"}, + {Name: "write"}, + }, + } + } + + mappings[valueFQN] = &attributes.GetAttributeValuesByFqnsResponse_AttributeAndValue{ + Value: &policy.Value{ + SubjectMappings: subjectMappings, + }, + } + } + + return mappings +} + +// Helper to generate entity with groups +func generateEntity(groupCount int) map[string]interface{} { + groups := make([]interface{}, groupCount) + for i := 0; i < groupCount; i++ { + groups[i] = fmt.Sprintf("group%d", i) + } + return map[string]interface{}{ + "attributes": map[string]interface{}{ + "groups": groups, + "department": "Engineering", + "level": "Senior", + }, + } +} + +// Benchmark EvaluateCondition with larger value sets +func BenchmarkEvaluateCondition_IN_Large(b *testing.B) { + // 50 possible values, entity has 20 groups + entity := generateEntity(20) + flatEntity, _ := flattening.Flatten(entity) + + possibleValues := make([]string, 50) + for i := 0; i < 50; i++ { + possibleValues[i] = fmt.Sprintf("possibleGroup%d", i) + } + + condition := &policy.Condition{ + SubjectExternalSelectorValue: ".attributes.groups[]", + Operator: policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN, + SubjectExternalValues: possibleValues, + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = subjectmappingbuiltin.EvaluateCondition(condition, flatEntity) + } +} + +// Benchmark EvaluateCondition with matching values (hit case) +func BenchmarkEvaluateCondition_IN_Hit(b *testing.B) { + entity := generateEntity(20) + flatEntity, _ := flattening.Flatten(entity) + + // Include "group5" which exists in the entity + possibleValues := []string{"unknownGroup1", "unknownGroup2", "unknownGroup3", "unknownGroup4", "group5"} + + condition := &policy.Condition{ + SubjectExternalSelectorValue: ".attributes.groups[]", + Operator: policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN, + SubjectExternalValues: possibleValues, + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = subjectmappingbuiltin.EvaluateCondition(condition, flatEntity) + } +} + +// Benchmark EvaluateSubjectMappings with many attribute mappings +func BenchmarkEvaluateSubjectMappings_LargeAttrCount(b *testing.B) { + mappings := generateAttributeMappings(50, 2, 3) // 50 attrs, 2 SMs each, 3 conditions each + entity := generateEntity(10) + additionalProps, _ := structpb.NewStruct(entity) + entityRep := &entityresolution.EntityRepresentation{ + AdditionalProps: []*structpb.Struct{additionalProps}, + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = subjectmappingbuiltin.EvaluateSubjectMappings(mappings, entityRep) + } +} + +// Benchmark EvaluateSubjectMappings with repeated subject sets (cache benefit) +func BenchmarkEvaluateSubjectMappings_RepeatedSubjectSets(b *testing.B) { + // Create shared subject sets that will be reused + sharedConditionGroup := &policy.ConditionGroup{ + BooleanOperator: policy.ConditionBooleanTypeEnum_CONDITION_BOOLEAN_TYPE_ENUM_OR, + Conditions: []*policy.Condition{ + { + SubjectExternalSelectorValue: ".attributes.groups[]", + Operator: policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN, + SubjectExternalValues: []string{"group0", "group1"}, + }, + }, + } + sharedSubjectSet := &policy.SubjectSet{ + ConditionGroups: []*policy.ConditionGroup{sharedConditionGroup}, + } + + // Create 20 attribute mappings that all reference the same subject set + mappings := make(map[string]*attributes.GetAttributeValuesByFqnsResponse_AttributeAndValue, 20) + for i := 0; i < 20; i++ { + valueFQN := fmt.Sprintf("https://demo.org/attr/attr%d/value/val%d", i, i) + mappings[valueFQN] = &attributes.GetAttributeValuesByFqnsResponse_AttributeAndValue{ + Value: &policy.Value{ + SubjectMappings: []*policy.SubjectMapping{ + { + SubjectConditionSet: &policy.SubjectConditionSet{ + SubjectSets: []*policy.SubjectSet{sharedSubjectSet, sharedSubjectSet}, // Duplicate references + }, + Actions: []*policy.Action{{Name: "read"}}, + }, + }, + }, + } + } + + entity := generateEntity(10) + additionalProps, _ := structpb.NewStruct(entity) + entityRep := &entityresolution.EntityRepresentation{ + AdditionalProps: []*structpb.Struct{additionalProps}, + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = subjectmappingbuiltin.EvaluateSubjectMappings(mappings, entityRep) + } +} + +// Benchmark EvaluateSubjectMappingsWithActions +func BenchmarkEvaluateSubjectMappingsWithActions_LargeAttrCount(b *testing.B) { + mappings := generateAttributeMappings(50, 2, 3) + entity := generateEntity(10) + additionalProps, _ := structpb.NewStruct(entity) + entityRep := &entityresolutionV2.EntityRepresentation{ + OriginalId: "test-entity", + AdditionalProps: []*structpb.Struct{additionalProps}, + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = subjectmappingbuiltin.EvaluateSubjectMappingsWithActions(mappings, entityRep) + } +} + +// Benchmark multiple entities +func BenchmarkEvaluateSubjectMappingMultipleEntitiesWithActions(b *testing.B) { + mappings := generateAttributeMappings(20, 2, 2) + + entities := make([]*entityresolutionV2.EntityRepresentation, 10) + for i := 0; i < 10; i++ { + entity := generateEntity(5 + i) // Different group counts + additionalProps, _ := structpb.NewStruct(entity) + entities[i] = &entityresolutionV2.EntityRepresentation{ + OriginalId: fmt.Sprintf("entity-%d", i), + AdditionalProps: []*structpb.Struct{additionalProps}, + } + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = subjectmappingbuiltin.EvaluateSubjectMappingMultipleEntitiesWithActions(mappings, entities) + } +} diff --git a/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go b/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go index c66f62ac77..db1a777e62 100644 --- a/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go +++ b/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go @@ -97,19 +97,24 @@ func EvaluateSubjectMappings(attributeMappings map[string]*attributes.GetAttribu // We only provide one input to ERS to resolve jsonEntities := entityRepresentation.GetAdditionalProps() entitlementsSet := make(map[string]bool) - entitlements := []string{} + for _, entity := range jsonEntities { flattenedEntity, err := flattening.Flatten(entity.AsMap()) if err != nil { return nil, fmt.Errorf("failure to flatten entity in subject mapping builtin: %w", err) } + + // Per-entity caches to avoid re-evaluating the same subject sets/groups across mappings + subjectSetCache := make(map[*policy.SubjectSet]bool) + condGroupCache := make(map[*policy.ConditionGroup]bool) + for attr, mapping := range attributeMappings { - // subject mapping results or-ed togethor + // subject mapping results or-ed together mappingResult := false for _, subjectMapping := range mapping.GetValue().GetSubjectMappings() { subjectMappingResult := true for _, subjectSet := range subjectMapping.GetSubjectConditionSet().GetSubjectSets() { - subjectSetConditionResult, err := EvaluateSubjectSet(subjectSet, flattenedEntity) + subjectSetConditionResult, err := evaluateSubjectSetCached(subjectSet, flattenedEntity, subjectSetCache, condGroupCache) if err != nil { return nil, err } @@ -132,21 +137,27 @@ func EvaluateSubjectMappings(attributeMappings map[string]*attributes.GetAttribu } } } + + entitlements := make([]string, 0, len(entitlementsSet)) for k := range entitlementsSet { entitlements = append(entitlements, k) } return entitlements, nil } -func EvaluateSubjectSet(subjectSet *policy.SubjectSet, entity flattening.Flattened) (bool, error) { +// evaluateSubjectSetCached evaluates a subject set with caching keyed by pointer identity. +func evaluateSubjectSetCached(subjectSet *policy.SubjectSet, entity flattening.Flattened, subjectSetCache map[*policy.SubjectSet]bool, condGroupCache map[*policy.ConditionGroup]bool) (bool, error) { + if res, ok := subjectSetCache[subjectSet]; ok { + return res, nil + } + // condition groups anded together subjectSetConditionResult := true for _, conditionGroup := range subjectSet.GetConditionGroups() { - conditionGroupResult, err := EvaluateConditionGroup(conditionGroup, entity) + conditionGroupResult, err := evaluateConditionGroupCached(conditionGroup, entity, condGroupCache) if err != nil { return false, err } - // update the subject condition set result // and together with previous condition group results subjectSetConditionResult = subjectSetConditionResult && conditionGroupResult // if one condition group fails, subject condition set fails @@ -154,10 +165,21 @@ func EvaluateSubjectSet(subjectSet *policy.SubjectSet, entity flattening.Flatten break } } + + subjectSetCache[subjectSet] = subjectSetConditionResult return subjectSetConditionResult, nil } -func EvaluateConditionGroup(conditionGroup *policy.ConditionGroup, entity flattening.Flattened) (bool, error) { +func EvaluateSubjectSet(subjectSet *policy.SubjectSet, entity flattening.Flattened) (bool, error) { + // Create ephemeral caches for a single-call path + return evaluateSubjectSetCached(subjectSet, entity, make(map[*policy.SubjectSet]bool), make(map[*policy.ConditionGroup]bool)) +} + +func evaluateConditionGroupCached(conditionGroup *policy.ConditionGroup, entity flattening.Flattened, condGroupCache map[*policy.ConditionGroup]bool) (bool, error) { + if res, ok := condGroupCache[conditionGroup]; ok { + return res, nil + } + // get boolean operator for condition group var conditionGroupResult bool switch conditionGroup.GetBooleanOperator() { @@ -200,70 +222,79 @@ ConditionEval: return false, errors.New("unsupported condition group boolean operator: " + conditionGroup.GetBooleanOperator().String()) } } + + condGroupCache[conditionGroup] = conditionGroupResult return conditionGroupResult, nil } +func EvaluateConditionGroup(conditionGroup *policy.ConditionGroup, entity flattening.Flattened) (bool, error) { + return evaluateConditionGroupCached(conditionGroup, entity, make(map[*policy.ConditionGroup]bool)) +} + func EvaluateCondition(condition *policy.Condition, entity flattening.Flattened) (bool, error) { mappedValues := flattening.GetFromFlattened(entity, condition.GetSubjectExternalSelectorValue()) - // slog.Debug("mapped values", "", mappedValues) - result := false + switch condition.GetOperator() { case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN: - // slog.Debug("the operator is IN") - for _, possibleValue := range condition.GetSubjectExternalValues() { - // slog.Debug("possible value", "", possibleValue) - for _, mappedValue := range mappedValues { - // slog.Debug("comparing values: ", "possible=", possibleValue, "mapped=", mappedValue) - if possibleValue == mappedValue { - // slog.Debug("comparison true") - result = true - break + // Build set of possible values for O(1) lookup + possible := condition.GetSubjectExternalValues() + if len(possible) == 0 || len(mappedValues) == 0 { + return false, nil + } + possibleSet := make(map[string]struct{}, len(possible)) + for _, pv := range possible { + possibleSet[pv] = struct{}{} + } + for _, mv := range mappedValues { + if s, ok := mv.(string); ok { + if _, found := possibleSet[s]; found { + return true, nil } } - if result { - break - } } + return false, nil + case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_NOT_IN: - notInResult := true - for _, possibleValue := range condition.GetSubjectExternalValues() { - for _, mappedValue := range mappedValues { - // slog.Debug("comparing values: ", "possible=", possibleValue, "mapped=", mappedValue) - if possibleValue == mappedValue { - // slog.Debug("comparison true") - notInResult = false - break + possible := condition.GetSubjectExternalValues() + if len(possible) == 0 { + return true, nil + } + possibleSet := make(map[string]struct{}, len(possible)) + for _, pv := range possible { + possibleSet[pv] = struct{}{} + } + for _, mv := range mappedValues { + if s, ok := mv.(string); ok { + if _, found := possibleSet[s]; found { + return false, nil } } - if !notInResult { - break - } - } - if notInResult { - result = true } + return true, nil + case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN_CONTAINS: - // slog.Debug("the operator is CONTAINS") + // Fast-path for string mapped values; fallback to fmt.Sprint only when necessary for _, possibleValue := range condition.GetSubjectExternalValues() { - // slog.Debug("possible value", "", possibleValue) for _, mappedValue := range mappedValues { - mappedValueStr := fmt.Sprintf("%v", mappedValue) - // slog.Debug("comparing values: ", "possible=", possibleValue, "mapped=", mappedValueStr) + var mappedValueStr string + switch v := mappedValue.(type) { + case string: + mappedValueStr = v + case []byte: + mappedValueStr = string(v) + default: + mappedValueStr = fmt.Sprint(mappedValue) + } if strings.Contains(mappedValueStr, possibleValue) { - result = true - break + return true, nil } } - if result { - break - } } + return false, nil + case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_UNSPECIFIED: - // unspecified subject mapping operator return false, errors.New("unspecified subject mapping operator: " + condition.GetOperator().String()) default: - // unsupported subject mapping operator return false, errors.New("unsupported subject mapping operator: " + condition.GetOperator().String()) } - return result, nil } diff --git a/service/internal/subjectmappingbuiltin/subject_mapping_builtin_actions.go b/service/internal/subjectmappingbuiltin/subject_mapping_builtin_actions.go index 491d1bb8f7..fcb26e7529 100644 --- a/service/internal/subjectmappingbuiltin/subject_mapping_builtin_actions.go +++ b/service/internal/subjectmappingbuiltin/subject_mapping_builtin_actions.go @@ -2,8 +2,6 @@ package subjectmappingbuiltin import ( "fmt" - "maps" - "slices" "strings" "github.com/opentdf/platform/lib/flattening" @@ -38,7 +36,10 @@ func EvaluateSubjectMappingsWithActions( entityRepresentation *entityresolutionV2.EntityRepresentation, ) (AttributeValueFQNsToActions, error) { jsonEntities := entityRepresentation.GetAdditionalProps() - entitlementsSet := make(map[string][]*policy.Action) + + // Accumulator that deduplicates actions across all subject mappings per value FQN + type actionSet map[string]*policy.Action + entitlementsAcc := make(map[string]actionSet) for _, entity := range jsonEntities { flattenedEntity, err := flattening.Flatten(entity.AsMap()) @@ -46,18 +47,19 @@ func EvaluateSubjectMappingsWithActions( return nil, fmt.Errorf("failure to flatten entity in subject mapping builtin: %w", err) } + // Per-entity caches for subject set and condition group evaluations + subjectSetCache := make(map[*policy.SubjectSet]bool) + condGroupCache := make(map[*policy.ConditionGroup]bool) + for valueFQN, attributeAndValue := range resolveableAttributes { - // subject mapping results or-ed together for _, subjectMapping := range attributeAndValue.GetValue().GetSubjectMappings() { subjectMappingResult := true for _, subjectSet := range subjectMapping.GetSubjectConditionSet().GetSubjectSets() { - subjectSetConditionResult, err := EvaluateSubjectSet(subjectSet, flattenedEntity) + subjectSetConditionResult, err := evaluateSubjectSetCached(subjectSet, flattenedEntity, subjectSetCache, condGroupCache) if err != nil { return nil, err } - // update the result for the subject mapping subjectMappingResult = subjectMappingResult && subjectSetConditionResult - // if one subject condition set fails, subject mapping fails if !subjectSetConditionResult { break } @@ -65,22 +67,25 @@ func EvaluateSubjectMappingsWithActions( // each subject mapping that is true should permit the actions on the mapped value if subjectMappingResult { - // add value FQN to the entitlements set - if _, ok := entitlementsSet[valueFQN]; !ok { - entitlementsSet[valueFQN] = make([]*policy.Action, 0) + if _, ok := entitlementsAcc[valueFQN]; !ok { + entitlementsAcc[valueFQN] = make(actionSet) } - actions := subjectMapping.GetActions() - - // Cache each action by name to deduplicate - m := make(map[string]*policy.Action, len(actions)) - for _, action := range actions { - m[strings.ToLower(action.GetName())] = action + for _, action := range subjectMapping.GetActions() { + entitlementsAcc[valueFQN][strings.ToLower(action.GetName())] = action } - entitlementsSet[valueFQN] = append(entitlementsSet[valueFQN], slices.Collect(maps.Values(m))...) } } } } + // Convert actionSet accumulator to final result shape + entitlementsSet := make(AttributeValueFQNsToActions, len(entitlementsAcc)) + for fqn, aSet := range entitlementsAcc { + entitlementsSet[fqn] = make([]*policy.Action, 0, len(aSet)) + for _, a := range aSet { + entitlementsSet[fqn] = append(entitlementsSet[fqn], a) + } + } + return entitlementsSet, nil } From b7c396c82d1487bd6e0e53d06ab534c4328edefe Mon Sep 17 00:00:00 2001 From: strantalis Date: Thu, 4 Dec 2025 21:00:09 -0500 Subject: [PATCH 2/6] perf(authz): fix selector dedupe in JIT PDP Fixed bug where subjectPropertySet was checked but never updated, causing duplicate selectors to be added. --- lib/flattening/flatten.go | 16 ++++++++++++---- lib/flattening/flatten_bench_large_test.go | 2 ++ service/internal/access/v2/just_in_time_pdp.go | 1 + 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/flattening/flatten.go b/lib/flattening/flatten.go index c8ef57a8bc..701d49316b 100644 --- a/lib/flattening/flatten.go +++ b/lib/flattening/flatten.go @@ -34,6 +34,8 @@ func GetFromFlattened(flat Flattened, selector string) []interface{} { return itemsToReturn } +// Flatten returns a Flattened struct with an index for O(1) lookups via GetFromFlattened. +// Use this when you will perform lookups by selector. func Flatten(m map[string]interface{}) (Flattened, error) { idx := make(map[string][]interface{}) items, err := flattenInterface(m, idx) @@ -58,7 +60,9 @@ func flattenInterface(i interface{}, idx map[string][]interface{}) ([]Item, erro for _, item := range nm { key := "." + k + item.Key o = append(o, Item{Key: key, Value: item.Value}) - idx[key] = append(idx[key], item.Value) + if idx != nil { + idx[key] = append(idx[key], item.Value) + } } } case []interface{}: @@ -74,13 +78,17 @@ func flattenInterface(i interface{}, idx map[string][]interface{}) ([]Item, erro keyAny := kAny + it.Key o = append(o, Item{Key: keyIdx, Value: it.Value}) o = append(o, Item{Key: keyAny, Value: it.Value}) - idx[keyIdx] = append(idx[keyIdx], it.Value) - idx[keyAny] = append(idx[keyAny], it.Value) + if idx != nil { + idx[keyIdx] = append(idx[keyIdx], it.Value) + idx[keyAny] = append(idx[keyAny], it.Value) + } } } case bool, int, string, float64, float32: o = append(o, Item{Key: "", Value: child}) - idx[""] = append(idx[""], child) + if idx != nil { + idx[""] = append(idx[""], child) + } default: return nil, errors.New("unrecognized item in json") } diff --git a/lib/flattening/flatten_bench_large_test.go b/lib/flattening/flatten_bench_large_test.go index ccb78d024f..df55647624 100644 --- a/lib/flattening/flatten_bench_large_test.go +++ b/lib/flattening/flatten_bench_large_test.go @@ -71,3 +71,5 @@ func BenchmarkFlatten_NestedEntity(b *testing.B) { } } } + + diff --git a/service/internal/access/v2/just_in_time_pdp.go b/service/internal/access/v2/just_in_time_pdp.go index 3f52427320..9222753299 100644 --- a/service/internal/access/v2/just_in_time_pdp.go +++ b/service/internal/access/v2/just_in_time_pdp.go @@ -331,6 +331,7 @@ func (p *JustInTimePDP) getMatchedSubjectMappings( } for _, item := range flattened.Items { if _, ok := subjectPropertySet[item.Key]; !ok { + subjectPropertySet[item.Key] = struct{}{} // Track key to avoid duplicates subjectProperties = append(subjectProperties, &policy.SubjectProperty{ ExternalSelectorValue: item.Key, }) From c5ec0906f103a01cad552fbed87abeb4b937bb1b Mon Sep 17 00:00:00 2001 From: strantalis Date: Thu, 4 Dec 2025 21:36:08 -0500 Subject: [PATCH 3/6] feat(examples): add complex authorization benchmarks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add new benchmark commands that exercise subject mapping evaluation with multiple attributes per resource and multiple resources per request: - benchmark-decision-complex: v2 API with configurable resources/attrs - benchmark-decision-v1-complex: v1 API with same capabilities These benchmarks show where optimization improvements shine by testing with 500 resources × 5 attributes = 2,500 attribute checks per request. Also updates CI workflow to run and report these benchmarks. --- .github/workflows/checks.yaml | 24 +++ examples/cmd/benchmark_decision_complex.go | 144 ++++++++++++++++++ examples/cmd/benchmark_decision_v1_complex.go | 127 +++++++++++++++ lib/flattening/flatten_bench_large_test.go | 2 - 4 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 examples/cmd/benchmark_decision_complex.go create mode 100644 examples/cmd/benchmark_decision_v1_complex.go diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 62df5c151e..2ace79153b 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -273,6 +273,28 @@ jobs: echo "$OUTPUT"; echo "EODECISIONV2" } >> "$GITHUB_ENV" + - name: run complex decision benchmark tests + run: | + OUTPUT=$(./examples/examples benchmark-decision-complex --resources 500 --attrs 5) + echo "--- Complex Decision Benchmark Output ---" + echo "$OUTPUT" + echo "$OUTPUT" >> "$GITHUB_STEP_SUMMARY" + { + echo "BENCHMARK_DECISION_COMPLEX_OUTPUT<> "$GITHUB_ENV" + - name: run complex decision v1 benchmark tests + run: | + OUTPUT=$(./examples/examples benchmark-decision-v1-complex --resources 500 --attrs 5) + echo "--- Complex Decision v1 Benchmark Output ---" + echo "$OUTPUT" + echo "$OUTPUT" >> "$GITHUB_STEP_SUMMARY" + { + echo "BENCHMARK_DECISION_V1_COMPLEX_OUTPUT<> "$GITHUB_ENV" - name: run tdf3 benchmark tests run: | OUTPUT=$(./examples/examples benchmark --count=5000 --concurrent=50) @@ -322,6 +344,8 @@ jobs: h2 "${BENCHMARK_DECISION_OUTPUT}" "Decision Benchmark" h2 "${BENCHMARK_DECISION_V2_OUTPUT}" "Decision Benchmark v2" + h2 "${BENCHMARK_DECISION_COMPLEX_OUTPUT}" "Complex Decision Benchmark" + h2 "${BENCHMARK_DECISION_V1_COMPLEX_OUTPUT}" "Complex Decision v1 Benchmark" h2 "${BENCHMARK_METRICS_OUTPUT}" "Standard Benchmark Metrics" h2 "${BENCHMARK_BULK_OUTPUT}" "Bulk Benchmark" h2 "${BENCHMARK_TDF3_OUTPUT}" "TDF3 Benchmark" diff --git a/examples/cmd/benchmark_decision_complex.go b/examples/cmd/benchmark_decision_complex.go new file mode 100644 index 0000000000..9e52e3ffc8 --- /dev/null +++ b/examples/cmd/benchmark_decision_complex.go @@ -0,0 +1,144 @@ +//nolint:forbidigo // We use Println here extensively because we are printing markdown. +package cmd + +import ( + "context" + "fmt" + "strconv" + "time" + + authzV2 "github.com/opentdf/platform/protocol/go/authorization/v2" + "github.com/opentdf/platform/protocol/go/entity" + "github.com/opentdf/platform/protocol/go/policy" + "github.com/spf13/cobra" +) + +var ( + complexAttrCount int + complexResourceCount int +) + +func init() { + benchmarkCmd := &cobra.Command{ + Use: "benchmark-decision-complex", + Short: "benchmark authorization with complex policy scenarios", + Long: `A benchmark tool to measure authorization performance with complex policies. +This benchmark exercises subject mapping evaluation with multiple attributes per resource +and multiple resources per request, showing where optimization improvements shine.`, + RunE: runDecisionBenchmarkComplex, + } + benchmarkCmd.Flags().IntVar(&complexAttrCount, "attrs", 5, "Number of attributes per resource") //nolint:mnd // default shown in help + benchmarkCmd.Flags().IntVar(&complexResourceCount, "resources", 100, "Number of resources per request") //nolint:mnd // default shown in help + ExamplesCmd.AddCommand(benchmarkCmd) +} + +func runDecisionBenchmarkComplex(_ *cobra.Command, _ []string) error { + client, err := newSDK() + if err != nil { + return err + } + + // Available attribute values from fixtures that have subject mappings + // These exercise different subject mapping conditions + availableAttrs := []string{ + "https://example.com/attr/attr1/value/value1", // 4 subject mappings with complex AND/OR conditions + "https://example.com/attr/attr1/value/value2", // 1 subject mapping + "https://example.net/attr/attr1/value/value1", // 1 subject mapping with NOT_IN condition + "https://example.com/attr/attr2/value/value1", // ALL_OF rule + "https://example.com/attr/attr2/value/value2", // ALL_OF rule + } + + // Build resources with multiple attributes each + var resources []*authzV2.Resource + for i := 0; i < complexResourceCount; i++ { + // Select attributes for this resource (cycle through available ones) + var fqns []string + for j := 0; j < complexAttrCount; j++ { + fqns = append(fqns, availableAttrs[(i+j)%len(availableAttrs)]) + } + + r := &authzV2.Resource{ + EphemeralId: "resource-" + strconv.Itoa(i), + Resource: &authzV2.Resource_AttributeValues_{ + AttributeValues: &authzV2.Resource_AttributeValues{ + Fqns: fqns, + }, + }, + } + resources = append(resources, r) + } + + // Run the benchmark multiple times to get stable measurements + const iterations = 5 + var totalTime time.Duration + var lastApproved, lastDenied int + + for iter := 0; iter < iterations; iter++ { + start := time.Now() + res, err := client.AuthorizationV2.GetDecisionMultiResource(context.Background(), &authzV2.GetDecisionMultiResourceRequest{ + Action: &policy.Action{ + Name: "read", + }, + EntityIdentifier: &authzV2.EntityIdentifier{ + Identifier: &authzV2.EntityIdentifier_EntityChain{ + EntityChain: &entity.EntityChain{ + EphemeralId: "complex-benchmark-entity-chain", + Entities: []*entity.Entity{ + { + EphemeralId: "jwtentity-0-clientid-opentdf-sdk", + EntityType: &entity.Entity_ClientId{ClientId: "opentdf-sdk"}, + Category: entity.Entity_CATEGORY_ENVIRONMENT, + }, + { + EphemeralId: "jwtentity-1-username-sample-user", + EntityType: &entity.Entity_UserName{UserName: "sample-user"}, + Category: entity.Entity_CATEGORY_SUBJECT, + }, + }, + }, + }, + }, + Resources: resources, + }) + elapsed := time.Since(start) + totalTime += elapsed + + if err != nil { + return fmt.Errorf("iteration %d failed: %w", iter, err) + } + + lastApproved = 0 + lastDenied = 0 + for _, decision := range res.GetResourceDecisions() { + if decision.GetDecision() == authzV2.Decision_DECISION_PERMIT { + lastApproved++ + } else { + lastDenied++ + } + } + } + + avgTime := totalTime / iterations + decisionsPerSecond := float64(complexResourceCount) / avgTime.Seconds() + + // Print results + fmt.Printf("# Complex Authorization Benchmark Results\n\n") + fmt.Printf("## Configuration\n") + fmt.Printf("| Parameter | Value |\n") + fmt.Printf("|----------------------|------------------------|\n") + fmt.Printf("| Resources per request | %d |\n", complexResourceCount) + fmt.Printf("| Attributes per resource | %d |\n", complexAttrCount) + fmt.Printf("| Total attribute checks | %d |\n", complexResourceCount*complexAttrCount) + fmt.Printf("| Iterations | %d |\n", iterations) + fmt.Printf("\n") + fmt.Printf("## Results\n") + fmt.Printf("| Metric | Value |\n") + fmt.Printf("|-------------------------|------------------------|\n") + fmt.Printf("| Approved Decisions | %d |\n", lastApproved) + fmt.Printf("| Denied Decisions | %d |\n", lastDenied) + fmt.Printf("| Average Request Time | %s |\n", avgTime) + fmt.Printf("| Decisions/second | %.2f |\n", decisionsPerSecond) + fmt.Printf("| Time per decision | %s |\n", time.Duration(int64(avgTime)/int64(complexResourceCount))) + + return nil +} diff --git a/examples/cmd/benchmark_decision_v1_complex.go b/examples/cmd/benchmark_decision_v1_complex.go new file mode 100644 index 0000000000..bce59b6fcd --- /dev/null +++ b/examples/cmd/benchmark_decision_v1_complex.go @@ -0,0 +1,127 @@ +//nolint:forbidigo // We use Println here extensively because we are printing markdown. +package cmd + +import ( + "context" + "fmt" + "time" + + "github.com/opentdf/platform/protocol/go/authorization" + "github.com/opentdf/platform/protocol/go/policy" + "github.com/spf13/cobra" +) + +func init() { + benchmarkCmd := &cobra.Command{ + Use: "benchmark-decision-v1-complex", + Short: "benchmark authorization.GetDecisions with complex policy scenarios", + Long: `A benchmark tool to measure authorization v1 performance with complex policies. +This benchmark exercises subject mapping evaluation with multiple attributes per resource.`, + RunE: runDecisionBenchmarkV1Complex, + } + benchmarkCmd.Flags().IntVar(&complexAttrCount, "attrs", 5, "Number of attributes per resource") //nolint:mnd // default shown in help + benchmarkCmd.Flags().IntVar(&complexResourceCount, "resources", 100, "Number of resources per request") //nolint:mnd // default shown in help + ExamplesCmd.AddCommand(benchmarkCmd) +} + +func runDecisionBenchmarkV1Complex(_ *cobra.Command, _ []string) error { + client, err := newSDK() + if err != nil { + return err + } + + // Available attribute values from fixtures that have subject mappings + availableAttrs := []string{ + "https://example.com/attr/attr1/value/value1", + "https://example.com/attr/attr1/value/value2", + "https://example.net/attr/attr1/value/value1", + "https://example.com/attr/attr2/value/value1", + "https://example.com/attr/attr2/value/value2", + } + + // Build resource attributes with multiple attribute values each + var ras []*authorization.ResourceAttribute + for i := 0; i < complexResourceCount; i++ { + var fqns []string + for j := 0; j < complexAttrCount; j++ { + fqns = append(fqns, availableAttrs[(i+j)%len(availableAttrs)]) + } + ras = append(ras, &authorization.ResourceAttribute{AttributeValueFqns: fqns}) + } + + // Run benchmark iterations + const iterations = 5 + var totalTime time.Duration + var lastApproved, lastDenied int + + for iter := 0; iter < iterations; iter++ { + start := time.Now() + res, err := client.Authorization.GetDecisions(context.Background(), &authorization.GetDecisionsRequest{ + DecisionRequests: []*authorization.DecisionRequest{ + { + Actions: []*policy.Action{{Value: &policy.Action_Standard{ + Standard: policy.Action_STANDARD_ACTION_DECRYPT, + }}}, + EntityChains: []*authorization.EntityChain{ + { + Id: "complex-v1-entity-chain", + Entities: []*authorization.Entity{ + { + Id: "jwtentity-0-clientid-opentdf-sdk", + EntityType: &authorization.Entity_ClientId{ClientId: "opentdf-sdk"}, + Category: authorization.Entity_CATEGORY_ENVIRONMENT, + }, + { + Id: "jwtentity-1-username-sample-user", + EntityType: &authorization.Entity_UserName{UserName: "sample-user"}, + Category: authorization.Entity_CATEGORY_SUBJECT, + }, + }, + }, + }, + ResourceAttributes: ras, + }, + }, + }) + elapsed := time.Since(start) + totalTime += elapsed + + if err != nil { + return fmt.Errorf("iteration %d failed: %w", iter, err) + } + + lastApproved = 0 + lastDenied = 0 + for _, dr := range res.GetDecisionResponses() { + if dr.GetDecision() == authorization.DecisionResponse_DECISION_PERMIT { + lastApproved++ + } else { + lastDenied++ + } + } + } + + avgTime := totalTime / iterations + decisionsPerSecond := float64(complexResourceCount) / avgTime.Seconds() + + // Print results + fmt.Printf("# Complex Authorization v1 Benchmark Results\n\n") + fmt.Printf("## Configuration\n") + fmt.Printf("| Parameter | Value |\n") + fmt.Printf("|------------------------|------------------------|\n") + fmt.Printf("| Resources per request | %d |\n", complexResourceCount) + fmt.Printf("| Attributes per resource | %d |\n", complexAttrCount) + fmt.Printf("| Total attribute checks | %d |\n", complexResourceCount*complexAttrCount) + fmt.Printf("| Iterations | %d |\n", iterations) + fmt.Printf("\n") + fmt.Printf("## Results\n") + fmt.Printf("| Metric | Value |\n") + fmt.Printf("|-------------------------|------------------------|\n") + fmt.Printf("| Approved Decisions | %d |\n", lastApproved) + fmt.Printf("| Denied Decisions | %d |\n", lastDenied) + fmt.Printf("| Average Request Time | %s |\n", avgTime) + fmt.Printf("| Decisions/second | %.2f |\n", decisionsPerSecond) + fmt.Printf("| Time per decision | %s |\n", time.Duration(int64(avgTime)/int64(complexResourceCount))) + + return nil +} diff --git a/lib/flattening/flatten_bench_large_test.go b/lib/flattening/flatten_bench_large_test.go index df55647624..ccb78d024f 100644 --- a/lib/flattening/flatten_bench_large_test.go +++ b/lib/flattening/flatten_bench_large_test.go @@ -71,5 +71,3 @@ func BenchmarkFlatten_NestedEntity(b *testing.B) { } } } - - From 9bbc0241f5039845507e31bf147753a5f2994204 Mon Sep 17 00:00:00 2001 From: strantalis Date: Fri, 5 Dec 2025 07:43:12 -0500 Subject: [PATCH 4/6] revert(examples): remove complex authorization benchmarks The complex benchmarks were not providing useful metrics as all decisions were being denied. Removing them to simplify the CI. --- .github/workflows/checks.yaml | 24 --- examples/cmd/benchmark_decision_complex.go | 144 ------------------ examples/cmd/benchmark_decision_v1_complex.go | 127 --------------- 3 files changed, 295 deletions(-) delete mode 100644 examples/cmd/benchmark_decision_complex.go delete mode 100644 examples/cmd/benchmark_decision_v1_complex.go diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 2ace79153b..62df5c151e 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -273,28 +273,6 @@ jobs: echo "$OUTPUT"; echo "EODECISIONV2" } >> "$GITHUB_ENV" - - name: run complex decision benchmark tests - run: | - OUTPUT=$(./examples/examples benchmark-decision-complex --resources 500 --attrs 5) - echo "--- Complex Decision Benchmark Output ---" - echo "$OUTPUT" - echo "$OUTPUT" >> "$GITHUB_STEP_SUMMARY" - { - echo "BENCHMARK_DECISION_COMPLEX_OUTPUT<> "$GITHUB_ENV" - - name: run complex decision v1 benchmark tests - run: | - OUTPUT=$(./examples/examples benchmark-decision-v1-complex --resources 500 --attrs 5) - echo "--- Complex Decision v1 Benchmark Output ---" - echo "$OUTPUT" - echo "$OUTPUT" >> "$GITHUB_STEP_SUMMARY" - { - echo "BENCHMARK_DECISION_V1_COMPLEX_OUTPUT<> "$GITHUB_ENV" - name: run tdf3 benchmark tests run: | OUTPUT=$(./examples/examples benchmark --count=5000 --concurrent=50) @@ -344,8 +322,6 @@ jobs: h2 "${BENCHMARK_DECISION_OUTPUT}" "Decision Benchmark" h2 "${BENCHMARK_DECISION_V2_OUTPUT}" "Decision Benchmark v2" - h2 "${BENCHMARK_DECISION_COMPLEX_OUTPUT}" "Complex Decision Benchmark" - h2 "${BENCHMARK_DECISION_V1_COMPLEX_OUTPUT}" "Complex Decision v1 Benchmark" h2 "${BENCHMARK_METRICS_OUTPUT}" "Standard Benchmark Metrics" h2 "${BENCHMARK_BULK_OUTPUT}" "Bulk Benchmark" h2 "${BENCHMARK_TDF3_OUTPUT}" "TDF3 Benchmark" diff --git a/examples/cmd/benchmark_decision_complex.go b/examples/cmd/benchmark_decision_complex.go deleted file mode 100644 index 9e52e3ffc8..0000000000 --- a/examples/cmd/benchmark_decision_complex.go +++ /dev/null @@ -1,144 +0,0 @@ -//nolint:forbidigo // We use Println here extensively because we are printing markdown. -package cmd - -import ( - "context" - "fmt" - "strconv" - "time" - - authzV2 "github.com/opentdf/platform/protocol/go/authorization/v2" - "github.com/opentdf/platform/protocol/go/entity" - "github.com/opentdf/platform/protocol/go/policy" - "github.com/spf13/cobra" -) - -var ( - complexAttrCount int - complexResourceCount int -) - -func init() { - benchmarkCmd := &cobra.Command{ - Use: "benchmark-decision-complex", - Short: "benchmark authorization with complex policy scenarios", - Long: `A benchmark tool to measure authorization performance with complex policies. -This benchmark exercises subject mapping evaluation with multiple attributes per resource -and multiple resources per request, showing where optimization improvements shine.`, - RunE: runDecisionBenchmarkComplex, - } - benchmarkCmd.Flags().IntVar(&complexAttrCount, "attrs", 5, "Number of attributes per resource") //nolint:mnd // default shown in help - benchmarkCmd.Flags().IntVar(&complexResourceCount, "resources", 100, "Number of resources per request") //nolint:mnd // default shown in help - ExamplesCmd.AddCommand(benchmarkCmd) -} - -func runDecisionBenchmarkComplex(_ *cobra.Command, _ []string) error { - client, err := newSDK() - if err != nil { - return err - } - - // Available attribute values from fixtures that have subject mappings - // These exercise different subject mapping conditions - availableAttrs := []string{ - "https://example.com/attr/attr1/value/value1", // 4 subject mappings with complex AND/OR conditions - "https://example.com/attr/attr1/value/value2", // 1 subject mapping - "https://example.net/attr/attr1/value/value1", // 1 subject mapping with NOT_IN condition - "https://example.com/attr/attr2/value/value1", // ALL_OF rule - "https://example.com/attr/attr2/value/value2", // ALL_OF rule - } - - // Build resources with multiple attributes each - var resources []*authzV2.Resource - for i := 0; i < complexResourceCount; i++ { - // Select attributes for this resource (cycle through available ones) - var fqns []string - for j := 0; j < complexAttrCount; j++ { - fqns = append(fqns, availableAttrs[(i+j)%len(availableAttrs)]) - } - - r := &authzV2.Resource{ - EphemeralId: "resource-" + strconv.Itoa(i), - Resource: &authzV2.Resource_AttributeValues_{ - AttributeValues: &authzV2.Resource_AttributeValues{ - Fqns: fqns, - }, - }, - } - resources = append(resources, r) - } - - // Run the benchmark multiple times to get stable measurements - const iterations = 5 - var totalTime time.Duration - var lastApproved, lastDenied int - - for iter := 0; iter < iterations; iter++ { - start := time.Now() - res, err := client.AuthorizationV2.GetDecisionMultiResource(context.Background(), &authzV2.GetDecisionMultiResourceRequest{ - Action: &policy.Action{ - Name: "read", - }, - EntityIdentifier: &authzV2.EntityIdentifier{ - Identifier: &authzV2.EntityIdentifier_EntityChain{ - EntityChain: &entity.EntityChain{ - EphemeralId: "complex-benchmark-entity-chain", - Entities: []*entity.Entity{ - { - EphemeralId: "jwtentity-0-clientid-opentdf-sdk", - EntityType: &entity.Entity_ClientId{ClientId: "opentdf-sdk"}, - Category: entity.Entity_CATEGORY_ENVIRONMENT, - }, - { - EphemeralId: "jwtentity-1-username-sample-user", - EntityType: &entity.Entity_UserName{UserName: "sample-user"}, - Category: entity.Entity_CATEGORY_SUBJECT, - }, - }, - }, - }, - }, - Resources: resources, - }) - elapsed := time.Since(start) - totalTime += elapsed - - if err != nil { - return fmt.Errorf("iteration %d failed: %w", iter, err) - } - - lastApproved = 0 - lastDenied = 0 - for _, decision := range res.GetResourceDecisions() { - if decision.GetDecision() == authzV2.Decision_DECISION_PERMIT { - lastApproved++ - } else { - lastDenied++ - } - } - } - - avgTime := totalTime / iterations - decisionsPerSecond := float64(complexResourceCount) / avgTime.Seconds() - - // Print results - fmt.Printf("# Complex Authorization Benchmark Results\n\n") - fmt.Printf("## Configuration\n") - fmt.Printf("| Parameter | Value |\n") - fmt.Printf("|----------------------|------------------------|\n") - fmt.Printf("| Resources per request | %d |\n", complexResourceCount) - fmt.Printf("| Attributes per resource | %d |\n", complexAttrCount) - fmt.Printf("| Total attribute checks | %d |\n", complexResourceCount*complexAttrCount) - fmt.Printf("| Iterations | %d |\n", iterations) - fmt.Printf("\n") - fmt.Printf("## Results\n") - fmt.Printf("| Metric | Value |\n") - fmt.Printf("|-------------------------|------------------------|\n") - fmt.Printf("| Approved Decisions | %d |\n", lastApproved) - fmt.Printf("| Denied Decisions | %d |\n", lastDenied) - fmt.Printf("| Average Request Time | %s |\n", avgTime) - fmt.Printf("| Decisions/second | %.2f |\n", decisionsPerSecond) - fmt.Printf("| Time per decision | %s |\n", time.Duration(int64(avgTime)/int64(complexResourceCount))) - - return nil -} diff --git a/examples/cmd/benchmark_decision_v1_complex.go b/examples/cmd/benchmark_decision_v1_complex.go deleted file mode 100644 index bce59b6fcd..0000000000 --- a/examples/cmd/benchmark_decision_v1_complex.go +++ /dev/null @@ -1,127 +0,0 @@ -//nolint:forbidigo // We use Println here extensively because we are printing markdown. -package cmd - -import ( - "context" - "fmt" - "time" - - "github.com/opentdf/platform/protocol/go/authorization" - "github.com/opentdf/platform/protocol/go/policy" - "github.com/spf13/cobra" -) - -func init() { - benchmarkCmd := &cobra.Command{ - Use: "benchmark-decision-v1-complex", - Short: "benchmark authorization.GetDecisions with complex policy scenarios", - Long: `A benchmark tool to measure authorization v1 performance with complex policies. -This benchmark exercises subject mapping evaluation with multiple attributes per resource.`, - RunE: runDecisionBenchmarkV1Complex, - } - benchmarkCmd.Flags().IntVar(&complexAttrCount, "attrs", 5, "Number of attributes per resource") //nolint:mnd // default shown in help - benchmarkCmd.Flags().IntVar(&complexResourceCount, "resources", 100, "Number of resources per request") //nolint:mnd // default shown in help - ExamplesCmd.AddCommand(benchmarkCmd) -} - -func runDecisionBenchmarkV1Complex(_ *cobra.Command, _ []string) error { - client, err := newSDK() - if err != nil { - return err - } - - // Available attribute values from fixtures that have subject mappings - availableAttrs := []string{ - "https://example.com/attr/attr1/value/value1", - "https://example.com/attr/attr1/value/value2", - "https://example.net/attr/attr1/value/value1", - "https://example.com/attr/attr2/value/value1", - "https://example.com/attr/attr2/value/value2", - } - - // Build resource attributes with multiple attribute values each - var ras []*authorization.ResourceAttribute - for i := 0; i < complexResourceCount; i++ { - var fqns []string - for j := 0; j < complexAttrCount; j++ { - fqns = append(fqns, availableAttrs[(i+j)%len(availableAttrs)]) - } - ras = append(ras, &authorization.ResourceAttribute{AttributeValueFqns: fqns}) - } - - // Run benchmark iterations - const iterations = 5 - var totalTime time.Duration - var lastApproved, lastDenied int - - for iter := 0; iter < iterations; iter++ { - start := time.Now() - res, err := client.Authorization.GetDecisions(context.Background(), &authorization.GetDecisionsRequest{ - DecisionRequests: []*authorization.DecisionRequest{ - { - Actions: []*policy.Action{{Value: &policy.Action_Standard{ - Standard: policy.Action_STANDARD_ACTION_DECRYPT, - }}}, - EntityChains: []*authorization.EntityChain{ - { - Id: "complex-v1-entity-chain", - Entities: []*authorization.Entity{ - { - Id: "jwtentity-0-clientid-opentdf-sdk", - EntityType: &authorization.Entity_ClientId{ClientId: "opentdf-sdk"}, - Category: authorization.Entity_CATEGORY_ENVIRONMENT, - }, - { - Id: "jwtentity-1-username-sample-user", - EntityType: &authorization.Entity_UserName{UserName: "sample-user"}, - Category: authorization.Entity_CATEGORY_SUBJECT, - }, - }, - }, - }, - ResourceAttributes: ras, - }, - }, - }) - elapsed := time.Since(start) - totalTime += elapsed - - if err != nil { - return fmt.Errorf("iteration %d failed: %w", iter, err) - } - - lastApproved = 0 - lastDenied = 0 - for _, dr := range res.GetDecisionResponses() { - if dr.GetDecision() == authorization.DecisionResponse_DECISION_PERMIT { - lastApproved++ - } else { - lastDenied++ - } - } - } - - avgTime := totalTime / iterations - decisionsPerSecond := float64(complexResourceCount) / avgTime.Seconds() - - // Print results - fmt.Printf("# Complex Authorization v1 Benchmark Results\n\n") - fmt.Printf("## Configuration\n") - fmt.Printf("| Parameter | Value |\n") - fmt.Printf("|------------------------|------------------------|\n") - fmt.Printf("| Resources per request | %d |\n", complexResourceCount) - fmt.Printf("| Attributes per resource | %d |\n", complexAttrCount) - fmt.Printf("| Total attribute checks | %d |\n", complexResourceCount*complexAttrCount) - fmt.Printf("| Iterations | %d |\n", iterations) - fmt.Printf("\n") - fmt.Printf("## Results\n") - fmt.Printf("| Metric | Value |\n") - fmt.Printf("|-------------------------|------------------------|\n") - fmt.Printf("| Approved Decisions | %d |\n", lastApproved) - fmt.Printf("| Denied Decisions | %d |\n", lastDenied) - fmt.Printf("| Average Request Time | %s |\n", avgTime) - fmt.Printf("| Decisions/second | %.2f |\n", decisionsPerSecond) - fmt.Printf("| Time per decision | %s |\n", time.Duration(int64(avgTime)/int64(complexResourceCount))) - - return nil -} From 22987e455be2ee32bc7cfddeca91812065d19a8d Mon Sep 17 00:00:00 2001 From: strantalis Date: Fri, 5 Dec 2025 09:38:06 -0500 Subject: [PATCH 5/6] perf(authz): optimize flatten and subject mapping for small inputs - Flatten: separate index building into a linear pass, skip index for structures with fewer than 8 items (linear scan is faster) - EvaluateSubjectSet/ConditionGroup: split into cached and non-cached versions, use non-cached for single evaluations to avoid map overhead - IN/NOT_IN conditions: use linear search for small value sets (<=4) to avoid map allocation Benchmarks show -38% geomean improvement in subject mapping evaluation and -66% geomean improvement in flattening lookups. --- lib/flattening/flatten.go | 51 +++--- lib/flattening/flatten_test.go | 3 +- .../subject_mapping_builtin.go | 160 ++++++++++++++---- 3 files changed, 155 insertions(+), 59 deletions(-) diff --git a/lib/flattening/flatten.go b/lib/flattening/flatten.go index 701d49316b..436ce65a4a 100644 --- a/lib/flattening/flatten.go +++ b/lib/flattening/flatten.go @@ -5,9 +5,13 @@ import ( "fmt" ) +// indexThreshold is the minimum number of items before we build an index. +// For smaller structures, linear scan is faster than map overhead. +const indexThreshold = 8 + type Flattened struct { Items []Item `json:"flattened"` - // index provides O(1) selector lookups; populated by Flatten + // index provides O(1) selector lookups; populated by Flatten for structures >= indexThreshold index map[string][]interface{} } @@ -24,8 +28,8 @@ func GetFromFlattened(flat Flattened, selector string) []interface{} { } return nil } - // Fallback for backwards compatibility (e.g., if Flattened was created without Flatten) - itemsToReturn := []interface{}{} + // Fallback: linear scan for small structures or backwards compatibility + var itemsToReturn []interface{} for _, item := range flat.Items { if item.Key == selector { itemsToReturn = append(itemsToReturn, item.Value) @@ -35,41 +39,55 @@ func GetFromFlattened(flat Flattened, selector string) []interface{} { } // Flatten returns a Flattened struct with an index for O(1) lookups via GetFromFlattened. -// Use this when you will perform lookups by selector. +// For small structures (< indexThreshold items), the index is skipped and lookups use linear scan. func Flatten(m map[string]interface{}) (Flattened, error) { - idx := make(map[string][]interface{}) - items, err := flattenInterface(m, idx) + items, err := flattenInterface(m) if err != nil { return Flattened{}, err } + + // Build index in a separate pass, only for larger structures + idx := buildIndex(items) + return Flattened{ Items: items, index: idx, }, nil } -func flattenInterface(i interface{}, idx map[string][]interface{}) ([]Item, error) { - o := []Item{} +// buildIndex constructs the selector index from flattened items. +// Returns nil for small structures where linear scan is faster. +func buildIndex(items []Item) map[string][]interface{} { + if len(items) < indexThreshold { + return nil + } + + idx := make(map[string][]interface{}, len(items)) + for _, it := range items { + idx[it.Key] = append(idx[it.Key], it.Value) + } + return idx +} + +func flattenInterface(i interface{}) ([]Item, error) { + var o []Item switch child := i.(type) { case map[string]interface{}: for k, v := range child { - nm, err := flattenInterface(v, idx) + nm, err := flattenInterface(v) if err != nil { return nil, err } for _, item := range nm { key := "." + k + item.Key o = append(o, Item{Key: key, Value: item.Value}) - if idx != nil { - idx[key] = append(idx[key], item.Value) - } } } case []interface{}: for index, item := range child { kIdx := fmt.Sprintf("[%v]", index) kAny := "[]" - flattenedItem, err := flattenInterface(item, idx) + flattenedItem, err := flattenInterface(item) if err != nil { return nil, err } @@ -78,17 +96,10 @@ func flattenInterface(i interface{}, idx map[string][]interface{}) ([]Item, erro keyAny := kAny + it.Key o = append(o, Item{Key: keyIdx, Value: it.Value}) o = append(o, Item{Key: keyAny, Value: it.Value}) - if idx != nil { - idx[keyIdx] = append(idx[keyIdx], it.Value) - idx[keyAny] = append(idx[keyAny], it.Value) - } } } case bool, int, string, float64, float32: o = append(o, Item{Key: "", Value: child}) - if idx != nil { - idx[""] = append(idx[""], child) - } default: return nil, errors.New("unrecognized item in json") } diff --git a/lib/flattening/flatten_test.go b/lib/flattening/flatten_test.go index 16c84c1451..90bac4b9bf 100644 --- a/lib/flattening/flatten_test.go +++ b/lib/flattening/flatten_test.go @@ -344,8 +344,7 @@ func TestFlattenInterfaceNoPanic(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { require.NotPanics(t, func() { - idx := make(map[string][]interface{}) - _, _ = flattenInterface(tc.value, idx) + _, _ = flattenInterface(tc.value) }) }) } diff --git a/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go b/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go index db1a777e62..4c382aa658 100644 --- a/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go +++ b/service/internal/subjectmappingbuiltin/subject_mapping_builtin.go @@ -18,6 +18,10 @@ import ( "google.golang.org/protobuf/encoding/protojson" ) +// smallSetThreshold is the maximum size for which we use linear search instead of a map. +// For small sets, nested loops are faster than map allocation + lookup. +const smallSetThreshold = 4 + func SubjectMappingBuiltin() { rego.RegisterBuiltin2(®o.Function{ Name: "subjectmapping.resolve", @@ -146,6 +150,7 @@ func EvaluateSubjectMappings(attributeMappings map[string]*attributes.GetAttribu } // evaluateSubjectSetCached evaluates a subject set with caching keyed by pointer identity. +// Used internally where subject sets may repeat across mappings. func evaluateSubjectSetCached(subjectSet *policy.SubjectSet, entity flattening.Flattened, subjectSetCache map[*policy.SubjectSet]bool, condGroupCache map[*policy.ConditionGroup]bool) (bool, error) { if res, ok := subjectSetCache[subjectSet]; ok { return res, nil @@ -170,16 +175,47 @@ func evaluateSubjectSetCached(subjectSet *policy.SubjectSet, entity flattening.F return subjectSetConditionResult, nil } +// evaluateSubjectSetNoCache evaluates a subject set without any memoization. +// Used for single-call paths where caching overhead isn't worth it. +func evaluateSubjectSetNoCache(subjectSet *policy.SubjectSet, entity flattening.Flattened) (bool, error) { + // condition groups anded together + subjectSetConditionResult := true + for _, conditionGroup := range subjectSet.GetConditionGroups() { + conditionGroupResult, err := evaluateConditionGroupNoCache(conditionGroup, entity) + if err != nil { + return false, err + } + subjectSetConditionResult = subjectSetConditionResult && conditionGroupResult + if !subjectSetConditionResult { + break + } + } + return subjectSetConditionResult, nil +} + +// EvaluateSubjectSet evaluates a subject set against a flattened entity. +// For single evaluations; does not use caching. func EvaluateSubjectSet(subjectSet *policy.SubjectSet, entity flattening.Flattened) (bool, error) { - // Create ephemeral caches for a single-call path - return evaluateSubjectSetCached(subjectSet, entity, make(map[*policy.SubjectSet]bool), make(map[*policy.ConditionGroup]bool)) + return evaluateSubjectSetNoCache(subjectSet, entity) } +// evaluateConditionGroupCached evaluates a condition group with caching. func evaluateConditionGroupCached(conditionGroup *policy.ConditionGroup, entity flattening.Flattened, condGroupCache map[*policy.ConditionGroup]bool) (bool, error) { if res, ok := condGroupCache[conditionGroup]; ok { return res, nil } + res, err := evaluateConditionGroupNoCache(conditionGroup, entity) + if err != nil { + return false, err + } + + condGroupCache[conditionGroup] = res + return res, nil +} + +// evaluateConditionGroupNoCache evaluates a condition group without memoization. +func evaluateConditionGroupNoCache(conditionGroup *policy.ConditionGroup, entity flattening.Flattened) (bool, error) { // get boolean operator for condition group var conditionGroupResult bool switch conditionGroup.GetBooleanOperator() { @@ -223,12 +259,13 @@ ConditionEval: } } - condGroupCache[conditionGroup] = conditionGroupResult return conditionGroupResult, nil } +// EvaluateConditionGroup evaluates a condition group against a flattened entity. +// For single evaluations; does not use caching. func EvaluateConditionGroup(conditionGroup *policy.ConditionGroup, entity flattening.Flattened) (bool, error) { - return evaluateConditionGroupCached(conditionGroup, entity, make(map[*policy.ConditionGroup]bool)) + return evaluateConditionGroupNoCache(conditionGroup, entity) } func EvaluateCondition(condition *policy.Condition, entity flattening.Flattened) (bool, error) { @@ -236,41 +273,10 @@ func EvaluateCondition(condition *policy.Condition, entity flattening.Flattened) switch condition.GetOperator() { case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN: - // Build set of possible values for O(1) lookup - possible := condition.GetSubjectExternalValues() - if len(possible) == 0 || len(mappedValues) == 0 { - return false, nil - } - possibleSet := make(map[string]struct{}, len(possible)) - for _, pv := range possible { - possibleSet[pv] = struct{}{} - } - for _, mv := range mappedValues { - if s, ok := mv.(string); ok { - if _, found := possibleSet[s]; found { - return true, nil - } - } - } - return false, nil + return evaluateIN(condition.GetSubjectExternalValues(), mappedValues) case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_NOT_IN: - possible := condition.GetSubjectExternalValues() - if len(possible) == 0 { - return true, nil - } - possibleSet := make(map[string]struct{}, len(possible)) - for _, pv := range possible { - possibleSet[pv] = struct{}{} - } - for _, mv := range mappedValues { - if s, ok := mv.(string); ok { - if _, found := possibleSet[s]; found { - return false, nil - } - } - } - return true, nil + return evaluateNotIN(condition.GetSubjectExternalValues(), mappedValues) case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN_CONTAINS: // Fast-path for string mapped values; fallback to fmt.Sprint only when necessary @@ -298,3 +304,83 @@ func EvaluateCondition(condition *policy.Condition, entity flattening.Flattened) return false, errors.New("unsupported subject mapping operator: " + condition.GetOperator().String()) } } + +// evaluateIN checks if any mapped value is in the possible values set. +// Uses linear search for small sets to avoid map allocation overhead. +func evaluateIN(possible []string, mappedValues []interface{}) (bool, error) { + if len(possible) == 0 || len(mappedValues) == 0 { + return false, nil + } + + // Small set: use nested loops to avoid map allocation + if len(possible) <= smallSetThreshold { + for _, mv := range mappedValues { + s, ok := mv.(string) + if !ok { + continue + } + for _, pv := range possible { + if s == pv { + return true, nil + } + } + } + return false, nil + } + + // Larger set: use map for O(1) lookups + possibleSet := make(map[string]struct{}, len(possible)) + for _, pv := range possible { + possibleSet[pv] = struct{}{} + } + for _, mv := range mappedValues { + if s, ok := mv.(string); ok { + if _, found := possibleSet[s]; found { + return true, nil + } + } + } + return false, nil +} + +// evaluateNotIN checks that no mapped value is in the possible values set. +// Uses linear search for small sets to avoid map allocation overhead. +func evaluateNotIN(possible []string, mappedValues []interface{}) (bool, error) { + if len(possible) == 0 { + return true, nil + } + if len(mappedValues) == 0 { + // No mapped values means none are in the forbidden set + return true, nil + } + + // Small set: use nested loops to avoid map allocation + if len(possible) <= smallSetThreshold { + for _, mv := range mappedValues { + s, ok := mv.(string) + if !ok { + continue + } + for _, pv := range possible { + if s == pv { + return false, nil + } + } + } + return true, nil + } + + // Larger set: use map for O(1) lookups + possibleSet := make(map[string]struct{}, len(possible)) + for _, pv := range possible { + possibleSet[pv] = struct{}{} + } + for _, mv := range mappedValues { + if s, ok := mv.(string); ok { + if _, found := possibleSet[s]; found { + return false, nil + } + } + } + return true, nil +} From 090167915c9dfcefdcbc355de9b747ca59791ef5 Mon Sep 17 00:00:00 2001 From: strantalis Date: Mon, 8 Dec 2025 13:27:48 -0500 Subject: [PATCH 6/6] test(authz): add test for hierarchyRule defensive code path Add TestHierarchyRule_DefinitionMissingValues to exercise the defensive code path that handles data inconsistency where resourceValueFQNs contains values not present in attrDefinition.GetValues(). This guards against panics if the policy service returns incomplete attribute definitions. --- service/internal/access/v2/evaluate_test.go | 66 +++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/service/internal/access/v2/evaluate_test.go b/service/internal/access/v2/evaluate_test.go index 9e096afe1a..44fcd6620f 100644 --- a/service/internal/access/v2/evaluate_test.go +++ b/service/internal/access/v2/evaluate_test.go @@ -630,6 +630,72 @@ func (s *EvaluateTestSuite) TestHierarchyRule() { } } +// TestHierarchyRule_DefinitionMissingValues tests the defensive code path where +// resourceValueFQNs contains values not present in attrDefinition.GetValues(). +// This guards against data inconsistency between the policy service and attribute definitions. +func (s *EvaluateTestSuite) TestHierarchyRule_DefinitionMissingValues() { + tests := []struct { + name string + attrDefinition *policy.Attribute + resourceValueFQNs []string + entitlements subjectmappingbuiltin.AttributeValueFQNsToActions + expectedFailures int + }{ + { + name: "empty definition values - all resource FQNs should fail", + attrDefinition: &policy.Attribute{ + Fqn: levelFQN, + Rule: policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_HIERARCHY, + Values: []*policy.Value{}, // Empty - simulates data inconsistency + }, + resourceValueFQNs: []string{levelMidFQN, levelLowerMidFQN}, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{}, + expectedFailures: 2, + }, + { + name: "definition missing requested values", + attrDefinition: &policy.Attribute{ + Fqn: levelFQN, + Rule: policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_HIERARCHY, + Values: []*policy.Value{ + // Only has highest - missing the values we're requesting + {Fqn: levelHighestFQN, Value: "highest"}, + }, + }, + resourceValueFQNs: []string{levelMidFQN, levelLowerMidFQN}, // Not in definition + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ + levelHighestFQN: []*policy.Action{actionRead}, + }, + expectedFailures: 2, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + failures := hierarchyRule( + s.T().Context(), + s.logger, + tc.entitlements, + s.action, + tc.resourceValueFQNs, + tc.attrDefinition, + ) + + s.Len(failures, tc.expectedFailures, "Expected %d failures but got %d", tc.expectedFailures, len(failures)) + + // Verify each resource FQN is reported as a failure + failedFQNs := make(map[string]bool) + for _, f := range failures { + failedFQNs[f.AttributeValueFQN] = true + s.Equal(s.action.GetName(), f.ActionName) + } + for _, fqn := range tc.resourceValueFQNs { + s.True(failedFQNs[fqn], "Expected FQN %s to be in failures", fqn) + } + }) + } +} + // Test cases for evaluateDefinition func (s *EvaluateTestSuite) TestEvaluateDefinition() { tests := []struct {