Skip to content

Commit 24870d4

Browse files
authored
Merge branch 'main' into feature/matchsubjectmappings
2 parents 93c22c6 + 069f939 commit 24870d4

File tree

7 files changed

+201
-58
lines changed

7 files changed

+201
-58
lines changed

.github/workflows/checks.yaml

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,14 @@ on:
99
- opened
1010
- synchronize
1111
- reopened
12-
paths-ignore:
13-
- '**/*.md'
14-
- LICENSE
15-
- CODEOWNERS
1612
push:
1713
branches:
1814
- main
19-
paths-ignore:
20-
- '**/*.md'
21-
- LICENSE
22-
- CODEOWNERS
2315
merge_group:
2416
branches:
2517
- main
2618
types:
2719
- checks_requested
28-
paths:
29-
- '**'
30-
- '!**/*.md'
31-
- 'docs/**'
32-
- '!LICENSE'
33-
- '!CODEOWNERS'
3420
workflow_call:
3521

3622
jobs:

.release-please-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@
44
"lib/flattening": "0.1.1",
55
"protocol/go": "0.2.18",
66
"sdk": "0.3.16",
7-
"service": "0.4.25"
7+
"service": "0.4.26"
88
}

CODEOWNERS

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,4 @@
1-
# CODEOWNERS
2-
3-
* @opentdf/maintainers
4-
5-
## Lib
6-
7-
/.github/ @opentdf/architecture @opentdf/maintainers
8-
/lib/ @opentdf/architecture @opentdf/maintainers
9-
/lib/ocrypto @opentdf/architecture
10-
11-
## General
12-
13-
.gitignore @opentdf/maintainers
14-
go.mod @opentdf/maintainers
15-
*.sum @opentdf/maintainers
16-
17-
### Documentation
18-
19-
*.md @opentdf/maintainers
20-
/docs/ @opentdf/maintainers
21-
/examples/ @opentdf/maintainers
22-
23-
## Services
24-
25-
/protocol/go/ @opentdf/architecture @opentdf/maintainers
26-
27-
### Policy
28-
29-
/service/migrations/ @opentdf/cli @opentdf/architecture
30-
/service/policy/ @opentdf/cli @opentdf/architecture
31-
32-
### Key Access Server
33-
34-
/service/kas/ @opentdf/kas @opentdf/architecture
35-
36-
## SDK
37-
38-
/sdk/ @opentdf/go-sdk @opentdf/architecture
39-
40-
## High Security Area
1+
# CODEOWNERS - code freeze
412

3+
* @opentdf/architecture
424
CODEOWNERS @opentdf/architecture @opentdf/security
43-
LICENSE @opentdf/architecture

service/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
# Changelog
22

3+
## [0.4.26](https://github.com/opentdf/platform/compare/service/v0.4.25...service/v0.4.26) (2024-10-17)
4+
5+
6+
### Bug Fixes
7+
8+
* use the right service namespace and update the tests ([#1665](https://github.com/opentdf/platform/issues/1665)) ([72ce62b](https://github.com/opentdf/platform/commit/72ce62b9b79a5b00d9723003789648246dd009b1))
9+
310
## [0.4.25](https://github.com/opentdf/platform/compare/service/v0.4.24...service/v0.4.25) (2024-10-15)
411

512

service/internal/auth/casbin.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ p, role:admin, /v1/token/authorization, *, allow
8989
p, role:standard, policy.*, read, allow
9090
p, role:standard, kasregistry.*, read, allow
9191
p, role:standard, kas.AccessService/Rewrap, *, allow
92-
p, role:standard, kas.AuthorizationService/GetDecisions, read, allow
93-
p, role:standard, kas.AuthorizationService/GetDecisionsByToken, read, allow
92+
p, role:standard, authorization.AuthorizationService/GetDecisions, read, allow
93+
p, role:standard, authorization.AuthorizationService/GetDecisionsByToken, read, allow
9494
## HTTP routes
9595
p, role:standard, /attributes*, read, allow
9696
p, role:standard, /namespaces*, read, allow

service/internal/auth/casbin_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,18 @@ func (s *AuthnCasbinSuite) Test_Enforcement() {
267267
resource: "/kas/v2/rewrap",
268268
action: "write",
269269
},
270+
{
271+
allowed: true,
272+
roles: standard,
273+
resource: "authorization.AuthorizationService/GetDecisions",
274+
action: "read",
275+
},
276+
{
277+
allowed: true,
278+
roles: standard,
279+
resource: "authorization.AuthorizationService/GetDecisionsByToken",
280+
action: "read",
281+
},
270282

271283
// undefined role
272284
{
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
# Pagination in policy LIST RPCs
2+
3+
## Table of Contents
4+
5+
- [Background](#background)
6+
- [Chosen Option](#chosen-option)
7+
- [Considered Options](#considered-options)
8+
- [LIMIT + OFFSET](#limit--offset)
9+
- [Keyset Pagination](#keyset-pagination)
10+
- [Cursor Pagination](#cursor-pagination)
11+
12+
## Background
13+
14+
At present, policy LIST RPCs are completely open-ended.
15+
16+
Attribute Namespaces, Definitions, and Values LIST calls may be filtered by _active_ state.
17+
18+
All Policy objects may be retrieved without quantity limits. This presents a challenge at scale if there
19+
are a very large number of any policy object in the platform database when responses become overwhelmingly
20+
large.
21+
22+
Introduction of a `limit` on retrieved items in LIST procedure call responses necessitates the simultaneous introduction of
23+
pagination. This ADR clarifies the unified approach we will take within policy service LIST RPCs
24+
and at the database level for this pagination.
25+
26+
## Chosen Option
27+
28+
[LIMIT + OFFSET](#limit--offset)
29+
30+
Because we do not know the likelihood of platforms running with Policy where any individual object has
31+
enough rows to experience the at-scale performance concerns of `offset` pagination, we will prefer
32+
this simple implementation for now and leave the door open for cursor-based pagination to solve the performance
33+
constraint should it be a realized problem in the future.
34+
35+
## Considered Options
36+
37+
### LIMIT + OFFSET
38+
39+
The simplest approach is a simple update to the proto for LIST RPCs and db queries to take in `limit` and `offset` with default values.
40+
41+
```proto
42+
message ListRequest {
43+
// ...existing fields omitted
44+
int32 limit = 3; // default depends on type of policy object
45+
int32 offset = 4; // default: 0
46+
}
47+
message ListResponse {
48+
// ...existing fields omitted
49+
int32 total = 5; // indication of total available for pagination
50+
}
51+
```
52+
53+
```sql
54+
-- subject-mappings example request:
55+
-- 'limit' 100
56+
-- 'offset' 100
57+
SELECT * FROM opentdf_policy.subject_mappings
58+
ORDER BY created_at
59+
LIMIT 100 OFFSET 100
60+
```
61+
62+
#### Pros & Cons
63+
64+
- :green_circle: Simple - support across any SQL database (just slightly different syntax)
65+
- :green_circle: Stateless - each request can independently paginate by specifying LIMIT / OFFSET
66+
- :green_circle: Flexibile - random-access pagination supported
67+
- :green_circle: Familiar - standard across LIST-type APIs
68+
- :yellow_circle: Create/Update/Delete of data between requests may throw off pages, but this is a relatively small concern when reads are exponentially more frequent than writes in Policy
69+
- :red_circle: Performance: large number of objects _or_ a high offset mean a lot of rows need to be scanned and discarded (skipped). However, (:yellow_circle:) we do not know how often the scale of policy objects will be large enough for this to be a problem
70+
71+
> [!NOTE]
72+
> Pagination is roughly Big O(n) time complexity as offset increases
73+
74+
### Keyset Pagination
75+
76+
We would index a column (the most obvious would be `created_at`) to use as the pagination key for
77+
querying, and facilitate pagination before/after any arbitrary timestamp.
78+
79+
```proto
80+
message ListRequest {
81+
// ...existing fields omitted
82+
int32 limit = 3; // default depends on type of policy object
83+
google.protobuf.Timestamp after = 4; // default: start_of_time
84+
int32 total = 5; // indication of total that can be paginated through
85+
}
86+
message ListResponse {
87+
// ...existing fields omitted
88+
int32 total = 5; // indication of total available for pagination
89+
}
90+
```
91+
92+
```sql
93+
-- subject-mappings example request:
94+
-- 'after' 2023-01-01
95+
-- 'limit' 100
96+
SELECT * FROM opentdf_policy.subject_mappings
97+
WHERE created_at > '2023-01-01' ORDER BY created_at LIMIT 100;
98+
```
99+
100+
#### Pros & Cons
101+
102+
- :green_circle: Support - supported across any SQL database (just slightly different syntax)
103+
- :green_circle: Speed - much faster in deep pages than OFFSET due to reduced scan row count
104+
- :yellow_circle: Reliability - provisioned policy may contain the same `created_at` timestamp
105+
- :red_circle: Flexibility - pagination is only forward of the `created_at` timestamp
106+
- :red_circle: Complexity - client must maintain state since response timestamps are required to drive subsequent request timestamp pagination, and pagination backwards is not supported
107+
- :red_circle: Complexity - reliance on timestamps introduces timezone differential confusion unless a parameter is also employed to localize the query
108+
109+
### Cursor Pagination
110+
111+
We would index a column (the most obvious would be `created_at`) to use as the pagination key for
112+
querying, but we would utilize an encoded cursor approach.
113+
114+
```proto
115+
message ListRequest {
116+
// ...existing fields omitted
117+
int32 limit = 3; // default depends on type of policy object
118+
string cursor = 4; // defaulted in API layer to cursor for encoded start_of_time
119+
}
120+
121+
message ListResponse {
122+
// ...existing fields and response data ommitted
123+
// cursors are encoded by the server as base64'd 'created_at' timestamps
124+
string previous_cursor = 4;
125+
string next_cursor = 4;
126+
int32 total = 5; // indication of total available for pagination
127+
}
128+
```
129+
130+
```sql
131+
-- subject-mappings example, request:
132+
-- 'after_cursor' 2023-01-01 00:00:00.000000+00
133+
-- 'limit' 100
134+
WITH Data AS (
135+
SELECT *
136+
FROM opentdf_policy.subject_mappings
137+
WHERE created_at >= '2023-01-01 00:00:00.000000+00'
138+
ORDER BY created_at
139+
LIMIT 101
140+
),
141+
NextPage AS (
142+
SELECT *
143+
FROM Data
144+
ORDER BY created_at
145+
LIMIT 100
146+
),
147+
PreviousPage AS (
148+
SELECT *
149+
FROM opentdf_policy.subject_mappings
150+
WHERE created_at < (SELECT MIN(created_at) FROM Data)
151+
ORDER BY created_at DESC
152+
LIMIT 101
153+
),
154+
CursorData AS (
155+
SELECT
156+
(SELECT MIN(created_at) FROM Data) AS first_item_created_at,
157+
(SELECT MAX(created_at) FROM NextPage) AS next_cursor_created_at,
158+
(SELECT MIN(created_at) FROM PreviousPage) AS previous_cursor_created_at
159+
)
160+
SELECT
161+
(SELECT json_agg(row_to_json(NextPage)) FROM NextPage) AS data,
162+
(SELECT json_build_object('created_at', next_cursor_created_at) FROM CursorData) AS next_cursor,
163+
(SELECT json_build_object('created_at', previous_cursor_created_at) FROM CursorData) AS previous_cursor
164+
FROM CursorData;
165+
```
166+
167+
#### Pros & Cons
168+
169+
- :green_circle: Support - supported across any SQL database (just different syntax)
170+
- :green_circle: Speed - much faster in deep pages than OFFSET due to reduced scan row count
171+
- :green_circle: Flexibility - pagination _a single page_ backward made possible by response `previous_cursor` value
172+
- :green_circle: Complexity - timestamp timezone differential is not a problem as cursors are server-determined and an API concern
173+
- :yellow_circle:/:red_circle: Reliability - provisioned policy will sometimes contain the same `created_at` timestamp, making it less than 100% reliable
174+
- :red_circle: New index on the `created_at` timestamp required which adds overhead but little value for management with
175+
time pretty much irrelevant to attributes except if required for sorting
176+
- :red_circle: Complexity - SQL queries become significantly more complex to build and read into responses
177+
- :red_circle: Flexibility - random access is still not supported without client state management and prior knowledge of forward pagination's historical cursors

0 commit comments

Comments
 (0)