diff --git a/lib/flattening/flatten.go b/lib/flattening/flatten.go index 3424014b7c..436ce65a4a 100644 --- a/lib/flattening/flatten.go +++ b/lib/flattening/flatten.go @@ -5,8 +5,14 @@ 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 for structures >= indexThreshold + index map[string][]interface{} } type Item struct { @@ -15,7 +21,15 @@ type Item struct { } func GetFromFlattened(flat Flattened, selector string) []interface{} { - itemsToReturn := []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: 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) @@ -24,18 +38,39 @@ func GetFromFlattened(flat Flattened, selector string) []interface{} { return itemsToReturn } +// Flatten returns a Flattened struct with an index for O(1) lookups via GetFromFlattened. +// For small structures (< indexThreshold items), the index is skipped and lookups use linear scan. func Flatten(m map[string]interface{}) (Flattened, error) { - flattened := Flattened{} items, err := flattenInterface(m) if err != nil { return Flattened{}, err } - flattened.Items = items - return flattened, nil + + // Build index in a separate pass, only for larger structures + idx := buildIndex(items) + + return Flattened{ + Items: items, + index: idx, + }, nil +} + +// 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) { - o := []Item{} + var o []Item switch child := i.(type) { case map[string]interface{}: for k, v := range child { @@ -44,20 +79,23 @@ func flattenInterface(i interface{}) ([]Item, error) { 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}) } } case []interface{}: - for idx, item := range child { - k := fmt.Sprintf("[%v]", idx) - k2 := "[]" + for index, item := range child { + kIdx := fmt.Sprintf("[%v]", index) + kAny := "[]" flattenedItem, err := flattenInterface(item) 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}) } } case bool, int, string, float64, float32: 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/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/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 { 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, }) 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..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", @@ -97,19 +101,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 +141,28 @@ 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. +// 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 + } + // 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 +170,52 @@ 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) { +// 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) { + 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() { @@ -200,70 +258,129 @@ ConditionEval: return false, errors.New("unsupported condition group boolean operator: " + conditionGroup.GetBooleanOperator().String()) } } + 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 evaluateConditionGroupNoCache(conditionGroup, entity) +} + 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") + return evaluateIN(condition.GetSubjectExternalValues(), mappedValues) + + case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_NOT_IN: + 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 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 + 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) { + return true, nil } - } - if result { - break } } - 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 - } + return false, nil + + case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_UNSPECIFIED: + return false, errors.New("unspecified subject mapping operator: " + condition.GetOperator().String()) + default: + 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 } - if !notInResult { - break + for _, pv := range possible { + if s == pv { + return true, nil + } } } - if notInResult { - result = true + 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 + } } - case policy.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN_CONTAINS: - // slog.Debug("the operator is CONTAINS") - 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) - if strings.Contains(mappedValueStr, possibleValue) { - result = true - break + } + 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 } } - if result { - break + } + 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 } } - 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 + return true, 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 }