diff --git a/service/internal/access/v2/entitleable.go b/service/internal/access/v2/entitleable.go index e8dbc79ee7..d1e2e298c5 100644 --- a/service/internal/access/v2/entitleable.go +++ b/service/internal/access/v2/entitleable.go @@ -149,37 +149,49 @@ func fetchEntitleableAttributes( return resp, false, nil } - // processBatch resolves a batch, falling back to per-FQN resolution on a batch NotFound. The - // server rejects the whole batch with NotFound if any requested FQN does not exist, so the retry - // keeps the values that DO exist and skips the missing ones (denied per-resource downstream). This - // preserves valid decisions in a multi-resource request that also references an unknown FQN. - processBatch := func(batch []string) error { - resp, err := getBatch(batch) - if err == nil { - return process(resp, batch) - } - if connect.CodeOf(err) != connect.CodeNotFound { - return fmt.Errorf("failed to get entitleable attributes by fqns: %w", err) - } - for _, fqn := range batch { - single, skip, ferr := fetchOne(fqn) - if ferr != nil { - return ferr + // Split sparse misses breadth-first so valid subsets are retained in batches. + // Bound failed batch probes before reverting to single-value lookups when many + // values are missing. This keeps dense misses close to the original call count. + const maxFailedBatchProbes = 8 + const splitDivisor = 2 + for start := 0; start < len(normalizedFQNs); start += maxEntitleableFQNsPerRequest { + end := min(start+maxEntitleableFQNsPerRequest, len(normalizedFQNs)) + pending := [][]string{normalizedFQNs[start:end]} + failedProbes := 0 + for len(pending) > 0 { + batch := pending[0] + pending = pending[1:] + if failedProbes >= maxFailedBatchProbes && len(batch) > 1 { + for _, fqn := range batch { + single, skip, err := fetchOne(fqn) + if err != nil { + return nil, nil, err + } + if skip { + continue + } + if err := process(single, []string{fqn}); err != nil { + return nil, nil, err + } + } + continue } - if skip { + resp, err := getBatch(batch) + if err == nil { + if err := process(resp, batch); err != nil { + return nil, nil, err + } continue } - if perr := process(single, []string{fqn}); perr != nil { - return perr + if connect.CodeOf(err) != connect.CodeNotFound { + return nil, nil, fmt.Errorf("failed to get entitleable attributes by fqns: %w", err) } - } - return nil - } - - for start := 0; start < len(normalizedFQNs); start += maxEntitleableFQNsPerRequest { - end := min(start+maxEntitleableFQNsPerRequest, len(normalizedFQNs)) - if err := processBatch(normalizedFQNs[start:end]); err != nil { - return nil, nil, err + if len(batch) == 1 { + continue + } + failedProbes++ + middle := len(batch) / splitDivisor + pending = append(pending, batch[:middle], batch[middle:]) } } diff --git a/service/internal/access/v2/entitleable_batch_test.go b/service/internal/access/v2/entitleable_batch_test.go new file mode 100644 index 0000000000..75c3b97f71 --- /dev/null +++ b/service/internal/access/v2/entitleable_batch_test.go @@ -0,0 +1,64 @@ +package access + +import ( + "errors" + "fmt" + "testing" + + "connectrpc.com/connect" + "github.com/opentdf/platform/protocol/go/policy" + attrs "github.com/opentdf/platform/protocol/go/policy/attributes" + "github.com/stretchr/testify/require" +) + +func TestEntitleableBatchSplitsSparseMisses(t *testing.T) { + const definition = "https://scale.example/attr/department" + const total = 250 + for _, missing := range []int{-1, 0, total - 1, total} { + t.Run(fmt.Sprintf("missing-%d", missing), func(t *testing.T) { + fqns := make([]string, total) + for i := range fqns { + fqns[i] = fmt.Sprintf("%s/value/%d", definition, i) + } + fake := &fakeAttributesClient{respFunc: func(req *attrs.GetEntitleableAttributesByFqnsRequest) (*attrs.GetEntitleableAttributesByFqnsResponse, error) { + for _, fqn := range req.GetFqns() { + if missing == total || (missing >= 0 && fqn == fqns[missing]) { + return nil, connect.NewError(connect.CodeNotFound, errors.New("missing value")) + } + } + resp := &attrs.GetEntitleableAttributesByFqnsResponse{ + Definitions: map[string]*attrs.GetEntitleableAttributesByFqnsResponse_EntitleableDefinition{definition: {Rule: policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_ANY_OF}}, + FqnEntitleableAttributes: make(map[string]*attrs.GetEntitleableAttributesByFqnsResponse_EntitleableAttribute), + } + for _, fqn := range req.GetFqns() { + resp.FqnEntitleableAttributes[fqn] = &attrs.GetEntitleableAttributesByFqnsResponse_EntitleableAttribute{DefinitionFqn: definition, Value: &attrs.GetEntitleableAttributesByFqnsResponse_EntitleableValue{ValueId: fqn, Fqn: fqn}} + } + return resp, nil + }} + definitions, _, err := fetchEntitleableAttributes(t.Context(), newSDKWithAttributes(fake), fqns) + require.NoError(t, err) + switch missing { + case -1: + require.Len(t, fake.requests, 1) + require.Len(t, definitions[0].GetValues(), total) + case total: + require.Empty(t, definitions) + require.LessOrEqual(t, len(fake.requests), total+8) + default: + require.Len(t, definitions, 1) + require.Len(t, definitions[0].GetValues(), total-1) + require.LessOrEqual(t, len(fake.requests), 17) + } + }) + } +} + +func TestEntitleableBatchDoesNotRetryOtherFailures(t *testing.T) { + expected := connect.NewError(connect.CodeUnavailable, errors.New("policy unavailable")) + fake := &fakeAttributesClient{respFunc: func(*attrs.GetEntitleableAttributesByFqnsRequest) (*attrs.GetEntitleableAttributesByFqnsResponse, error) { + return nil, expected + }} + _, _, err := fetchEntitleableAttributes(t.Context(), newSDKWithAttributes(fake), []string{"https://scale.example/attr/a/value/one", "https://scale.example/attr/a/value/two"}) + require.ErrorIs(t, err, expected) + require.Len(t, fake.requests, 1) +}