Skip to content

Conversation

ryanulit
Copy link
Contributor

Proposed Changes

adds sqlc queries for obligation_definitions CRUD + DB client methods and integration tests

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

@github-actions github-actions bot added comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) size/s labels Jul 10, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @ryanulit, I'm Gemini Code Assist1! 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 lays the groundwork for managing obligation_definitions within the policy service. It introduces the necessary SQL queries for full CRUD functionality and sets up the corresponding database client methods and an integration test suite, preparing the system for complete implementation of these operations.

Highlights

  • New CRUD Operations for Obligation Definitions: This pull request introduces the foundational SQL queries and corresponding database client methods for Create, Read (Get/List), Update, and Delete (CRUD) operations on obligation_definitions.
  • Integration Test Suite Setup: A new integration test suite (ObligationsSuite) has been added to cover future testing of these obligation_definitions CRUD operations, although all tests are currently skipped as the table is not yet fully implemented.
  • SQLC Query Definitions: New SQL queries (createObligationDefinition, getObligationDefinition, listObligationDefinitions, updateObligationDefinition, deleteObligationDefinition) have been defined using sqlc for managing obligation_definitions in the database.
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 is currently in preview and 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 to provide feedback.

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.


SQL queries new, Obligations now defined, Data starts to flow.

Footnotes

  1. 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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This PR adds the foundational CRUD queries, DB client methods, and integration tests for obligation definitions. My review focuses on correctness issues in the new SQL queries and naming consistency in the DB client methods. I've identified a couple of critical issues in the SQL queries that would cause them to fail, related to missing GROUP BY clauses and incorrect total counts for pagination. I've also suggested some renames in the Go code for better consistency.

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 339.577747ms
Throughput 294.48 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.639570973s
Average Latency 364.85625ms
Throughput 136.46 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.608115624s
Average Latency 255.018553ms
Throughput 195.25 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 404.686614ms
Throughput 247.10 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.526805388s
Average Latency 363.621531ms
Throughput 136.89 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.507934858s
Average Latency 254.253537ms
Throughput 196.02 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 352.3201ms
Throughput 283.83 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 35.574554951s
Average Latency 353.447131ms
Throughput 140.55 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 24.658263651s
Average Latency 245.715484ms
Throughput 202.77 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 363.96159ms
Throughput 274.75 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.366065598s
Average Latency 401.899041ms
Throughput 123.87 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 28.436491145s
Average Latency 283.058661ms
Throughput 175.83 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 340.014283ms
Throughput 294.11 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.308676182s
Average Latency 361.316393ms
Throughput 137.71 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.359529156s
Average Latency 252.608949ms
Throughput 197.16 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 351.178679ms
Throughput 284.76 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.105527581s
Average Latency 369.296861ms
Throughput 134.75 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.902188295s
Average Latency 258.023194ms
Throughput 193.03 requests/second

Copy link
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 356.747262ms
Throughput 280.31 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 38.372753939s
Average Latency 381.90276ms
Throughput 130.30 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.389793456s
Average Latency 262.940818ms
Throughput 189.47 requests/second

Copy link
Contributor

github-actions bot commented Sep 3, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 359.624572ms
Throughput 278.07 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.56769122s
Average Latency 362.80089ms
Throughput 136.73 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.342007102s
Average Latency 252.352388ms
Throughput 197.30 requests/second

c-r33d
c-r33d previously approved these changes Sep 3, 2025
jakedoublev
jakedoublev previously approved these changes Sep 4, 2025
jp-ayyappan
jp-ayyappan previously approved these changes Sep 4, 2025
Copy link

@jp-ayyappan jp-ayyappan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

strantalis
strantalis previously approved these changes Sep 4, 2025
@jakedoublev jakedoublev added this pull request to the merge queue Sep 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 4, 2025
Copy link
Contributor

github-actions bot commented Sep 4, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 346.973107ms
Throughput 288.21 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.240265804s
Average Latency 390.089396ms
Throughput 127.42 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.809160718s
Average Latency 257.107102ms
Throughput 193.73 requests/second

@alkalescent alkalescent added this pull request to the merge queue Sep 4, 2025
Merged via the queue into main with commit c194e35 Sep 4, 2025
32 checks passed
@alkalescent alkalescent deleted the DSPX-1344-obligation-definitions-sql-crud branch September 4, 2025 19:48
github-merge-queue bot pushed a commit that referenced this pull request Sep 12, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.9.0](protocol/go/v0.8.0...protocol/go/v0.9.0)
(2025-09-11)


### Features

* **policy:** add FQN of obligation definitions/values to protos
([#2703](#2703))
([45ded0e](45ded0e))
* **policy:** Add obligation triggers
([#2675](#2675))
([22d0837](22d0837))
* **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:** obligations + values CRUD
([#2545](#2545))
([c194e35](c194e35))


### Bug Fixes

* **deps:** update protovalidate to v0.14.2 to use new buf validate
MessageOneofRule
([#2698](#2698))
([1cae18e](1cae18e))

---
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>
github-merge-queue bot pushed a commit that referenced this pull request Sep 17, 2025
🤖 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&gt;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>
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.8.0](sdk/v0.7.0...sdk/v0.8.0)
(2025-09-19)


### Features

* **policy:** obligations + values CRUD
([#2545](#2545))
([c194e35](c194e35))


### Bug Fixes

* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.3.0 to
0.5.0 in /sdk ([#2693](#2693))
([b511048](b511048))
* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.5.0 to
0.6.0 in /sdk ([#2712](#2712))
([74956bf](74956bf))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.7.0 to
0.8.0 in /sdk ([#2692](#2692))
([fac2ef2](fac2ef2))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.8.0 to
0.9.0 in /sdk ([#2724](#2724))
([e07cc91](e07cc91))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.9.0 to
0.10.0 in /sdk
([#2737](#2737))
([f4a8d1d](f4a8d1d))
* **sdk:** newGranter nil check
([#2729](#2729))
([a1bebc5](a1bebc5))

---
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants