feat(policy): Include Triggers in GET/LISTable reqs #2704

c-r33d · 2025-09-08T21:41:52Z

Proposed Changes

Return Obligation Triggers for the following routes:

GetObligation
GetObligationByFQNs
ListObligations
GetObligationValue
GetObligationValuesByFQNs

Checklist

I have added or updated unit tests
I have added or updated integration tests (if appropriate)
I have added or updated documentation

Testing Instructions

…igation-definitions-sql-crud

…gers-values

gemini-code-assist

Summary of Changes

Hello @c-r33d, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the policy service by integrating obligation triggers into its core retrieval and management APIs. The changes enable clients to not only create and update obligation values with associated triggers but also to retrieve these triggers when querying obligations or obligation values. This ensures that the policy system provides a richer and more functional representation of policy decisioning logic.

Highlights

API Enhancement: The policy API now includes obligation triggers in responses for GetObligation, GetObligationByFQNs, ListObligations, GetObligationValue, and GetObligationValuesByFQNs requests. This provides a more complete view of policy obligations and their associated triggers.
Trigger Management: The CreateObligationValueRequest and UpdateObligationValueRequest now support including ValueTriggerRequest objects, allowing for the creation and replacement of obligation triggers directly through these API calls.
Database Schema and Query Updates: Underlying database queries have been updated to fetch and return obligation trigger data as part of the obligation and obligation value retrieval processes. A new deleteAllObligationTriggersForValue query was added to support the 'replace all' behavior during updates.
Improved Error Handling: A new specific error type, ErrInvalidOblTriParam, has been introduced to provide clearer feedback when invalid parameters are provided during obligation trigger operations.
Comprehensive Testing: Extensive integration and unit tests have been added or updated to validate the new trigger functionality across various API endpoints and data manipulation scenarios.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Data flows, a stream so wide,Triggers now, no longer hide.Policy's truth, revealed anew,In every GET, for me and you.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a feature to include obligation triggers in various GET and LIST API responses, which is a valuable addition. The implementation includes extensive new integration tests, which is great. However, I've identified a critical bug in the trigger update logic that could lead to unexpected behavior when removing triggers. I've also suggested some improvements for code maintainability by reducing duplication in both the SQL queries and the test code. Addressing these points will improve the robustness and long-term health of the codebase.

service/policy/db/obligations.go

service/integration/obligations_test.go

service/policy/db/queries/obligations.sql

github-actions · 2025-09-08T21:46:28Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	181.526977ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	97.420353ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	348.921093ms
Throughput	286.60 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	36.585077566s
Average Latency	364.432559ms
Throughput	136.67 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.864156442s
Average Latency	257.655897ms
Throughput	193.32 requests/second

github-actions · 2025-09-08T22:20:59Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	173.234499ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	99.01093ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	346.804164ms
Throughput	288.35 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	38.069543453s
Average Latency	378.691156ms
Throughput	131.34 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	26.058525976s
Average Latency	259.582693ms
Throughput	191.88 requests/second

github-actions · 2025-09-09T13:02:15Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	178.230411ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	91.966079ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	350.407206ms
Throughput	285.38 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	37.253034044s
Average Latency	369.89312ms
Throughput	134.22 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.931894165s
Average Latency	257.960852ms
Throughput	192.81 requests/second

github-actions · 2025-09-09T13:03:57Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	168.228133ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	99.567338ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	347.762687ms
Throughput	287.55 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	36.444145032s
Average Latency	361.747023ms
Throughput	137.20 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	25.45319861s
Average Latency	253.495272ms
Throughput	196.44 requests/second

service/policy/db/queries/obligations.sql

service/integration/obligations_test.go

🤖 I have created a release *beep* *boop* --- ## [0.10.0](service/v0.9.0...service/v0.10.0) (2025-09-17) ### ⚠ BREAKING CHANGES * **policy:** Add manager column to provider configuration for multi-instance support ([#2601](#2601)) ### Features * **authz:** add obligation policy decision point ([#2706](#2706)) ([bb2a4f8](bb2a4f8)) * **core:** add service negation for op mode ([#2680](#2680)) ([029db8c](029db8c)) * **core:** Bump default write timeout. ([#2671](#2671)) ([6a233c1](6a233c1)) * **core:** Encapsulate>Encrypt ([#2676](#2676)) ([3c5a614](3c5a614)) * **core:** Lets key manager factory take context ([#2715](#2715)) ([8d70993](8d70993)) * **policy:** add FQN of obligation definitions/values to protos ([#2703](#2703)) ([45ded0e](45ded0e)) * **policy:** Add manager column to provider configuration for multi-instance support ([#2601](#2601)) ([a5fc994](a5fc994)) * **policy:** Add obligation triggers ([#2675](#2675)) ([22d0837](22d0837)) * **policy:** add protovalidate for obligation defs + vals ([#2699](#2699)) ([af5c049](af5c049)) * **policy:** Allow creation and update of triggers on Obligation Values ([#2691](#2691)) ([b1e7ba1](b1e7ba1)) * **policy:** Allow for additional context to be added to obligation triggers ([#2705](#2705)) ([7025599](7025599)) * **policy:** Include Triggers in GET/LISTable reqs ([#2704](#2704)) ([b4381d1](b4381d1)) * **policy:** obligations + values CRUD ([#2545](#2545)) ([c194e35](c194e35)) * use public AES protected key from lib/ocrypto ([#2600](#2600)) ([75d7590](75d7590)) ### Bug Fixes * **core:** remove extraneous comment ([#2741](#2741)) ([ada8da6](ada8da6)) * **core:** return services in the order they were registered ([#2733](#2733)) ([1d661db](1d661db)) * **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.3.0 to 0.6.0 in /service ([#2714](#2714)) ([00354b3](00354b3)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.7.0 to 0.9.0 in /service ([#2726](#2726)) ([9004368](9004368)) * **deps:** bump protocol/go to 0.10.0 in service ([#2734](#2734)) ([11e6201](11e6201)) * **deps:** update protovalidate to v0.14.2 to use new buf validate MessageOneofRule ([#2698](#2698)) ([1cae18e](1cae18e)) * **policy:** Registered Resources should consider actions correctly within Decision Requests ([#2681](#2681)) ([cf264a2](cf264a2)) * sanitize db schema identifiers ([#2682](#2682)) ([0d3dd94](0d3dd94)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>

ryanulit and others added 30 commits July 10, 2025 16:43

initial sql queries and client and test stubs

02b2b98

fix bugs in sql queries

804651d

add missing group by

ad0fc20

more query updates

db8dcb8

Merge branch 'main' into DSPX-1344-obligation-definitions-sql-crud

256ecf0

apply gemini feedback

46d99c8

lint fixes

5167a03

Merge branch 'main' of github.com:opentdf/platform into DSPX-1344-obl…

c3920b8

…igation-definitions-sql-crud

updating type signatures

6663475

again

c885126

create obligation working

c42246b

create by id or fqn

63c7430

refactor

c142cbf

w namespace

14ef263

revert

f765f4a

add namespace

b789347

values

c89ed9d

values sql

2b1f2bc

w namespace

379bfd4

gen sql

7b09596

seems okay

aa53db3

values actually working

b0805f6

combined query seems to be working

35b55bb

still need fqn

8159f79

create obligation done

c1d17c5

clean up

09f143c

test create obligation failures

4888f7e

splitOblFQN

aebff06

get obligation

b9bfa75

refactor

9a199d9

c-r33d added 5 commits September 8, 2025 12:29

fix tests.

4c6df16

Merge branch 'main' into DSPX-1346-crud-values-triggers

c6b2781

make error more user friendly.

99fc074

Merge branch 'DSPX-1346-crud-values-triggers' into DSPX-1346-get-trig…

701b224

…gers-values

add tests.

20d1f70

c-r33d requested review from a team as code owners September 8, 2025 21:41

github-actions bot added comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) docs Documentation size/l labels Sep 8, 2025

gemini-code-assist bot reviewed Sep 8, 2025

View reviewed changes

service/policy/db/obligations.go Show resolved Hide resolved

service/integration/obligations_test.go Outdated Show resolved Hide resolved

service/integration/obligations_test.go Outdated Show resolved Hide resolved

service/policy/db/queries/obligations.sql Show resolved Hide resolved

Merge branch 'main' into DSPX-1346-get-triggers-values

c572fdd

c-r33d added 2 commits September 9, 2025 07:58

gemini.

719b6dd

change name.

a8c891c

jakedoublev reviewed Sep 9, 2025

View reviewed changes

service/policy/db/queries/obligations.sql Show resolved Hide resolved

jakedoublev reviewed Sep 9, 2025

View reviewed changes

service/integration/obligations_test.go Show resolved Hide resolved

jakedoublev approved these changes Sep 9, 2025

View reviewed changes

c-r33d added this pull request to the merge queue Sep 9, 2025

Merged via the queue into main with commit b4381d1 Sep 9, 2025
32 checks passed

c-r33d deleted the DSPX-1346-get-triggers-values branch September 9, 2025 15:16

opentdf-automation bot mentioned this pull request Sep 9, 2025

chore(main): release service 0.10.0 #2672

Merged

feat(policy): Include Triggers in GET/LISTable reqs #2704

feat(policy): Include Triggers in GET/LISTable reqs #2704

Uh oh!

Conversation

c-r33d commented Sep 8, 2025

Proposed Changes

Checklist

Testing Instructions

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 8, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

github-actions bot commented Sep 8, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

github-actions bot commented Sep 9, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

github-actions bot commented Sep 9, 2025

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

TDF3 Benchmark Results:

NANOTDF Benchmark Results:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!