Skip to content

Conversation

@elizabethhealy
Copy link
Member

@elizabethhealy elizabethhealy commented Oct 2, 2025

Proposed Changes

  • There was an issue where services started in core mode were registered but not starting. It looks like they were registered with the namespace as the mode instead of the specified mode from the config. This PR corrects this and adds test checks to ensure services registered are also enabled.
  • Namespaces will be registered with the mode that caused them to be included -- if all is in the list then the namespaces will have mode all, if core,ers is the mode then policy will have mode core and entityresolution will have mode entityresolution ...

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 the size/s label Oct 2, 2025
@elizabethhealy
Copy link
Member Author

/gemini review

@elizabethhealy elizabethhealy changed the title fix(service): Fix mode negation and core mode fix(core): Fix mode negation and core mode Oct 2, 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.

Code Review

This pull request addresses a bug where services were not being correctly enabled in 'core' mode because they were registered with their namespace as the mode instead of the specified mode from the configuration. The fix in service/pkg/serviceregistry/serviceregistry.go correctly identifies and uses the intended mode for service registration, which resolves the issue. The accompanying test updates in service/pkg/server/services_test.go are a good addition to verify this fix. However, I've noticed a small issue in the new test logic where a function call intended for assertion is missing the actual assertion, rendering that part of the test ineffective. My feedback includes suggestions to correct this in the tests.

@elizabethhealy elizabethhealy marked this pull request as ready for review October 2, 2025 19:45
@elizabethhealy elizabethhealy requested a review from a team as a code owner October 2, 2025 19:45
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 344.402141ms
Throughput 290.36 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.684416832s
Average Latency 374.069991ms
Throughput 132.68 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.065885299s
Average Latency 259.560348ms
Throughput 191.82 requests/second

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 355.89422ms
Throughput 280.98 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.017518025s
Average Latency 398.02726ms
Throughput 124.95 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.70586144s
Average Latency 265.559753ms
Throughput 187.22 requests/second

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 344.992639ms
Throughput 289.86 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.559481552s
Average Latency 373.67505ms
Throughput 133.12 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.934945108s
Average Latency 258.27471ms
Throughput 192.79 requests/second

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

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

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 364.054273ms
Throughput 274.68 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.571998757s
Average Latency 373.870197ms
Throughput 133.08 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.1500136s
Average Latency 260.656299ms
Throughput 191.20 requests/second

@elizabethhealy elizabethhealy added this pull request to the merge queue Oct 3, 2025
Merged via the queue into main with commit de9807d Oct 3, 2025
33 checks passed
@elizabethhealy elizabethhealy deleted the dspx-1729-fix-platform-mode-negation branch October 3, 2025 14:39
github-merge-queue bot pushed a commit that referenced this pull request Oct 22, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.11.0](service/v0.10.0...service/v0.11.0)
(2025-10-22)


### Features

* **authz:** add obligation fulfillment logic to obligation PDP
([#2740](#2740))
([2f8d30d](2f8d30d))
* **authz:** audit logs should properly handle obligations
([#2824](#2824))
([874ec7b](874ec7b))
* **authz:** defer to request auth as decision/entitlements entity
([#2789](#2789))
([feb34d8](feb34d8))
* **authz:** obligations protos within auth service
([#2745](#2745))
([41ee5a8](41ee5a8))
* **authz:** protovalidate tests for new authz obligations fields
([#2747](#2747))
([73e6319](73e6319))
* **authz:** service logic to use request auth as entity identifier in
PDP decisions/entitlements
([#2790](#2790))
([6784e88](6784e88))
* **authz:** wire up obligations enforcement in auth service
([#2756](#2756))
([11b3ea9](11b3ea9))
* **core:** propagate token clientID on configured claim via interceptor
into shared context metadata
([#2760](#2760))
([0f77246](0f77246))
* **kas:** Add required obligations to kao metadata.:
([#2806](#2806))
([16fb26c](16fb26c))
* **policy:** add FQNs to obligation defs + vals
([#2749](#2749))
([fa2585c](fa2585c))
* **policy:** Add obligation support to KAS
([#2786](#2786))
([bb1bca0](bb1bca0))
* **policy:** List obligation triggers rpc
([#2823](#2823))
([206abe3](206abe3))
* **policy:** namespace root certificates
([#2771](#2771))
([beaff21](beaff21))
* **policy:** Proto - root certificates by namespace
([#2800](#2800))
([0edb359](0edb359))
* **policy:** Protos List obligation triggers
([#2803](#2803))
([b32df81](b32df81))
* **policy:** Return built obligations fqns with triggers.
([#2830](#2830))
([e843018](e843018))
* **policy:** Return obligations from GetAttributeValue calls
([#2742](#2742))
([aa9b393](aa9b393))


### Bug Fixes

* **core:** CORS
([#2787](#2787))
([a030ac6](a030ac6))
* **core:** deprecate policy WithValue selector not utilized by RPC
([#2794](#2794))
([c573595](c573595))
* **core:** deprecated stale protos and add better upgrade comments
([#2793](#2793))
([f2678cc](f2678cc))
* **core:** Don't require known manager names
([#2792](#2792))
([8a56a96](8a56a96))
* **core:** Fix mode negation and core mode
([#2779](#2779))
([de9807d](de9807d))
* **core:** resolve environment loading issues
([#2827](#2827))
([9af3184](9af3184))
* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.6.0 to
0.7.0 in /service
([#2812](#2812))
([a6d180d](a6d180d))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.12.0 to
0.13.0 in /service
([#2814](#2814))
([5e9c695](5e9c695))
* **deps:** bump github.com/opentdf/platform/sdk from 0.7.0 to 0.9.0 in
/service ([#2798](#2798))
([d6bc9a8](d6bc9a8))
* **deps:** bump github.com/opentdf/platform/sdk from 0.9.0 to 0.10.0 in
/service ([#2831](#2831))
([412dfd1](412dfd1))
* ECC key loading (deprecated)
([#2757](#2757))
([49990eb](49990eb))
* **policy:** Change to nil
([#2746](#2746))
([a449434](a449434))

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants