Skip to content

fix(policy): make obligation trigger uniqueness client-aware - #3114

Merged
elizabethhealy merged 5 commits into
mainfrom
dspx-2237-obligation-trigger-unique-client-aware
Mar 9, 2026
Merged

fix(policy): make obligation trigger uniqueness client-aware#3114
elizabethhealy merged 5 commits into
mainfrom
dspx-2237-obligation-trigger-unique-client-aware

Conversation

@elizabethhealy

@elizabethhealy elizabethhealy commented Mar 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Updates obligation trigger DB uniqueness to account for optional PEP client_id scope.
  • Allows creating multiple obligation triggers for the same obligation/action/attribute tuple when client_id differs.
  • Preserves duplicate protection for same tuple + same client_id.

Problem

obligation_triggers previously enforced uniqueness on:

  • obligation_value_id
  • action_id
  • attribute_value_id

After client_id was introduced, this still blocked valid multi-PEP use cases where different client PEPs should each have their own trigger for the same obligation tuple.

Changes

  • Added migration:
    • service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.sql
  • Added migration documentation:
    • service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.md
  • Replaced legacy table-level uniqueness with partial unique indexes:
    • unscoped unique index on (obligation_value_id, action_id, attribute_value_id) where client_id IS NULL
    • scoped unique index on (obligation_value_id, action_id, attribute_value_id, client_id) where client_id IS NOT NULL
  • Migration handles historical Postgres-truncated legacy constraint names when dropping old constraints.

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 Mar 6, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 addresses a limitation in the obligation_triggers database uniqueness constraint that previously blocked valid multi-PEP (Policy Enforcement Point) use cases. By making the uniqueness logic client-aware, the system can now correctly handle scenarios where different PEP clients require their own triggers for the same obligation tuple, while still preventing true duplicates. This enhances the flexibility and correctness of obligation trigger management.

Highlights

  • Database Uniqueness Logic Updated: The uniqueness enforcement for obligation_triggers in the database was updated to consider the optional client_id scope.
  • Multi-PEP Use Cases Enabled: The system now allows for the creation of multiple obligation triggers for the same obligation/action/attribute tuple, provided their client_id values differ.
  • Duplicate Protection Maintained: Existing duplicate protection for triggers with the same tuple and client_id is preserved, preventing true duplicates.
  • Database Migration Introduced: A new database migration was added to implement these changes, including dropping old constraints and creating new partial unique indexes.
  • Integration Tests Added: New integration tests were introduced to validate the updated uniqueness behavior for both differing and identical client_id scenarios.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • service/integration/obligation_triggers_test.go
    • Added a new constant secondClientID for testing purposes.
    • Introduced Test_CreateObligationTrigger_SameTupleDifferentClients_Success to verify that triggers with different client IDs can be created for the same obligation tuple.
    • Introduced Test_CreateObligationTrigger_SameTupleSameClient_Fails to confirm that triggers with identical client IDs and obligation tuples still fail on creation.
  • service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.md
    • Documented the purpose and changes of the database migration, explaining the 'why' and 'how' of the uniqueness update.
  • service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.sql
    • Dropped several legacy unique constraints on the obligation_triggers table, handling historical truncated names.
    • Created a new partial unique index obligation_triggers_unscoped_unique_idx for triggers where client_id IS NULL.
    • Created a new partial unique index obligation_triggers_scoped_unique_idx for triggers where client_id IS NOT NULL.
Activity
  • No human activity has been recorded on this pull request yet.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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.


Old rules now give way, Client IDs bring new order, Data flows freely.

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.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request correctly updates the database uniqueness constraint for obligation triggers to be client-aware. The migration is robust, handling potential variations in legacy constraint names, and is well-documented. The accompanying integration tests effectively validate the new uniqueness logic for both scoped and unscoped triggers. My feedback includes a couple of minor suggestions to refactor the new test code for better readability and to reduce duplication.

Comment thread service/integration/obligation_triggers_test.go Outdated
Comment thread service/integration/obligation_triggers_test.go Outdated
@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 375.462138ms
Throughput 266.34 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.637624447s
Average Latency 395.055929ms
Throughput 126.14 requests/second

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 406.948362ms
Throughput 245.73 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.069134411s
Average Latency 398.460904ms
Throughput 124.78 requests/second

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 383.44752ms
Throughput 260.79 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.843814521s
Average Latency 406.629219ms
Throughput 122.42 requests/second

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 363.416717ms
Throughput 275.17 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.075342666s
Average Latency 388.431357ms
Throughput 127.96 requests/second

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

@elizabethhealy
elizabethhealy marked this pull request as ready for review March 6, 2026 22:30
@elizabethhealy
elizabethhealy requested review from a team as code owners March 6, 2026 22:30
c-r33d
c-r33d previously approved these changes Mar 9, 2026
@policy-bot-opentdf
policy-bot-opentdf Bot dismissed c-r33d’s stale review March 9, 2026 16:13

Invalidated by push of 9bd4f04

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 372.153621ms
Throughput 268.71 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.09451496s
Average Latency 389.176303ms
Throughput 127.90 requests/second

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

@elizabethhealy
elizabethhealy requested a review from c-r33d March 9, 2026 18:56
@elizabethhealy
elizabethhealy enabled auto-merge March 9, 2026 19:25
@elizabethhealy
elizabethhealy added this pull request to the merge queue Mar 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 9, 2026
@elizabethhealy
elizabethhealy added this pull request to the merge queue Mar 9, 2026
Merged via the queue into main with commit 9265bc3 Mar 9, 2026
39 checks passed
@elizabethhealy
elizabethhealy deleted the dspx-2237-obligation-trigger-unique-client-aware branch March 9, 2026 21:19
elizabethhealy added a commit to opentdf/otdfctl that referenced this pull request Mar 10, 2026
JBCongdon pushed a commit to JBCongdon/platform that referenced this pull request May 24, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.14.0](opentdf/platform@service/v0.13.0...service/v0.14.0)
(2026-04-21)


### ⚠ BREAKING CHANGES

* **sdk:** reclassify KAS 400 errors — distinguish tamper from
misconfiguration
([opentdf#3166](opentdf#3166))
* **policy:** optional namespace for RRs
([opentdf#3165](opentdf#3165))
* **policy:** Namespace subject mappings and subject condition sets.
([opentdf#3143](opentdf#3143))
* **policy:** Optional namespace on actions protos, NamespacedPolicy
feature flag ([opentdf#3155](opentdf#3155))
* **policy:** add namespaced actions schema and namespace-aware action
queries ([opentdf#3154](opentdf#3154))
* **policy:** only require namespace on GetAction if no id provided
([opentdf#3144](opentdf#3144))
* **policy:** add namespace field to Actions proto
([opentdf#3130](opentdf#3130))
* **policy:** namespace Registered Resources
([opentdf#3111](opentdf#3111))
* **policy:** add namespace field to RegisteredResource proto
([opentdf#3110](opentdf#3110))

### Features

* **authz:** Namespaced policy in decisioning
([opentdf#3226](opentdf#3226))
([0355934](opentdf@0355934))
* **cli:** migrate otdfctl into platform monorepo
([opentdf#3205](opentdf#3205))
([5177bec](opentdf@5177bec))
* fix tracing ([opentdf#3242](opentdf#3242))
([57e5680](opentdf@57e5680))
* **policy:** add GetObligationTrigger RPC
([opentdf#3318](opentdf#3318))
([d68e39d](opentdf@d68e39d))
* **policy:** add namespace field to Actions proto
([opentdf#3130](opentdf#3130))
([bedc9b3](opentdf@bedc9b3))
* **policy:** add namespace field to RegisteredResource proto
([opentdf#3110](opentdf#3110))
([04fd85d](opentdf@04fd85d))
* **policy:** add namespaced actions schema and namespace-aware action
queries ([opentdf#3154](opentdf#3154))
([c0443f1](opentdf@c0443f1))
* **policy:** add sort ListSubjectMappings API
([opentdf#3255](opentdf#3255))
([9d5d757](opentdf@9d5d757))
* **policy:** Add sort support listregisteredresources api
([opentdf#3312](opentdf#3312))
([91a3ff3](opentdf@91a3ff3))
* **policy:** add sort support to ListAttributes API
([opentdf#3223](opentdf#3223))
([ec3312f](opentdf@ec3312f))
* **policy:** add sort support to ListKeyAccessServer
([opentdf#3287](opentdf#3287))
([7fae2d7](opentdf@7fae2d7))
* **policy:** Add sort support to ListNamespaces API
([opentdf#3192](opentdf#3192))
([aac86cd](opentdf@aac86cd))
* **policy:** add sort support to listobligations api
([opentdf#3300](opentdf#3300))
([9221cac](opentdf@9221cac))
* **policy:** add sort support to ListSubjectConditionSets API
([opentdf#3272](opentdf#3272))
([9010f12](opentdf@9010f12))
* **policy:** add SortField proto and update PageRequest for sort
support ([opentdf#3187](opentdf#3187))
([6cf1862](opentdf@6cf1862))
* **policy:** Enforce same namespace when actions referenced downstream
([opentdf#3206](opentdf#3206))
([4b5463a](opentdf@4b5463a))
* **policy:** namespace Registered Resources
([opentdf#3111](opentdf#3111))
([6db1883](opentdf@6db1883))
* **policy:** Namespace subject mappings and condition sets
([opentdf#3172](opentdf#3172))
([6deed50](opentdf@6deed50))
* **policy:** Namespace subject mappings and subject condition sets.
([opentdf#3143](opentdf#3143))
([3006780](opentdf@3006780))
* **policy:** optional namespace for RRs
([opentdf#3165](opentdf#3165))
([8948018](opentdf@8948018))
* **policy:** rollback migration strategy for namespaced actions
([opentdf#3235](opentdf#3235))
([f7e5e01](opentdf@f7e5e01))
* **policy:** Seed existing namespaces with standard actions
([opentdf#3228](opentdf#3228))
([12136b0](opentdf@12136b0))
* **policy:** Seed namespaces with standard actions on creation +
namespaced actions for obligation triggers
([opentdf#3161](opentdf#3161))
([984d76b](opentdf@984d76b))


### Bug Fixes

* **ci:** Upgrade toolchain version to 1.25.8
([opentdf#3116](opentdf#3116))
([e1b7882](opentdf@e1b7882))
* **core:** do not concat slashes directly in url/file paths
([opentdf#3290](opentdf#3290))
([114c2a7](opentdf@114c2a7))
* **deps:** bump github.com/jackc/pgx/v5 from 5.7.5 to 5.9.0 in /service
([opentdf#3316](opentdf#3316))
([017362e](opentdf@017362e))
* **deps:** bump github.com/opentdf/platform/lib/identifier from 0.2.0
to 0.3.0 in /service
([opentdf#3162](opentdf#3162))
([8bc5dcd](opentdf@8bc5dcd))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.16.0 to
0.17.0 in /service
([opentdf#3125](opentdf#3125))
([29fec61](opentdf@29fec61))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.17.0 to
0.21.0 in /service
([opentdf#3220](opentdf#3220))
([e63add2](opentdf@e63add2))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.21.0 to
0.22.0 in /service
([opentdf#3248](opentdf#3248))
([1ebce73](opentdf@1ebce73))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.22.0 to
0.23.0 in /service
([opentdf#3271](opentdf#3271))
([3338b8e](opentdf@3338b8e))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.23.0 to
0.24.0 in /service
([opentdf#3321](opentdf#3321))
([78e6022](opentdf@78e6022))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.24.0 to
0.25.0 in /service
([opentdf#3333](opentdf#3333))
([3940bf8](opentdf@3940bf8))
* **deps:** bump github.com/opentdf/platform/sdk from 0.13.0 to 0.16.0
in /service ([opentdf#3356](opentdf#3356))
([5617077](opentdf@5617077))
* **deps:** bump
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp from
1.42.0 to 1.43.0 in /service
([opentdf#3282](opentdf#3282))
([046374a](opentdf@046374a))
* **deps:** bump go.opentelemetry.io/otel/sdk from 1.42.0 to 1.43.0 in
/service ([opentdf#3281](opentdf#3281))
([56b33f2](opentdf@56b33f2))
* **deps:** bump google.golang.org/grpc from 1.77.0 to 1.79.3 in
/service ([opentdf#3176](opentdf#3176))
([3289502](opentdf@3289502))
* **deps:** remove direct github.com/docker/docker dependency
([opentdf#3229](opentdf#3229))
([2becb27](opentdf@2becb27))
* **deps:** upgrade testcontainers-go to resolve vulns
([opentdf#3299](opentdf#3299))
([72c6f9b](opentdf@72c6f9b))
* **ers:** include standard JWT claims in claims mode entity resolution
([opentdf#3196](opentdf#3196))
([6d50da1](opentdf@6d50da1))
* **ers:** ldap multi-strategy ers
([opentdf#3117](opentdf#3117))
([d3aaf1a](opentdf@d3aaf1a))
* **policy:** deprecate ListAttributeValues in favor of existing
GetAttribute ([opentdf#3108](opentdf#3108))
([7e17c2d](opentdf@7e17c2d))
* **policy:** make obligation trigger uniqueness client-aware
([opentdf#3114](opentdf#3114))
([9265bc3](opentdf@9265bc3))
* **policy:** omit empty attribute values from create responses
([opentdf#3193](opentdf#3193))
([d298378](opentdf@d298378))
* **policy:** only require namespace on GetAction if no id provided
([opentdf#3144](opentdf#3144))
([10d0c0f](opentdf@10d0c0f))
* **policy:** Optional namespace on actions protos, NamespacedPolicy
feature flag ([opentdf#3155](opentdf#3155))
([c20f039](opentdf@c20f039))
* **policy:** order List* results by created_at
([opentdf#3088](opentdf#3088))
([ea90ac2](opentdf@ea90ac2))
* **sdk:** normalize issuer URL before OIDC discovery
([opentdf#3261](opentdf#3261))
([61f98c9](opentdf@61f98c9))
* **sdk:** reclassify KAS 400 errors — distinguish tamper from
misconfiguration
([opentdf#3166](opentdf#3166))
([f04a385](opentdf@f04a385))
* **sdk:** remove testcontainers from consumer dependency graph
([opentdf#3129](opentdf#3129))
([f17dcdd](opentdf@f17dcdd))

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

2 participants