feat(policy): Unique name for the key provider.

[c-r33d]

Proposed Changes

1.) Make the provider_name for the provider configuration unique.

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

[gemini-code-assist]

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!

Summary of Changes

Hello team,

Gemini or gemini-code-assist here to provide a summary of this pull request. The primary goal of this PR, as indicated by the title and description, is to enforce uniqueness for the provider_name field within the provider_config table in the policy service database. This ensures that each key provider configuration can be uniquely identified by its name.

The changes involve adding a unique constraint to the database schema via a new migration script, updating the schema documentation to reflect this change, and enhancing the integration tests for key management to verify the new unique constraint and improve test cleanup procedures.

Highlights

• Database Schema Change: A unique constraint has been added to the provider_name column in the provider_config table to enforce uniqueness.
• Database Migration: A new SQL migration script (20250605000000_unique_provider_name.sql) is included to apply and rollback the unique constraint on the provider_name column.
• Integration Test Enhancement: The key management integration tests (keymanagement_test.go) have been updated to include a new test case specifically for verifying that creating a provider config with a duplicate name fails as expected. Additionally, a helper function (deleteTestProviderConfigs) and defer statements were introduced to ensure proper cleanup of created test data after each test run.
• Schema Documentation Update: The schema ER diagram documentation (schema_erd.md) has been updated to mark the provider_name column as a Unique Key (UK) in the provider_config table. (Note: An unrelated change adding selector_values to condition_set was also included in this file).

Changelog

Click here to see the changelog

• service/integration/keymanagement_test.go
  • Added deleteTestProviderConfigs helper function for test cleanup (lines 50-56).
  • Updated existing tests to use deleteTestProviderConfigs with defer for cleanup (e.g., lines 59-60, 64-67, 107-110, 122-125, 144-147, 158-161, 197-200, 236-239).
  • Added Test_CreateProviderConfig_DuplicateName_Fails to verify unique name constraint (lines 89-104).
  • Modified createTestProviderConfig helper to accept provider name, config, and metadata as arguments (lines 317-324).
• service/policy/db/migrations/20250605000000_unique_provider_name.md
  • Added new migration documentation file with ER diagram showing unique constraint on provider_name.
• service/policy/db/migrations/20250605000000_unique_provider_name.sql
  • Added new SQL migration file.
  • Added +goose Up statement to add a unique constraint provider_config_provider_name_key to provider_config.provider_name (line 3).
  • Added +goose Down statement to drop the unique constraint (line 10).
• service/policy/db/schema_erd.md
  • Updated provider_config entry to mark provider_name as a Unique Key (UK) (line 140).
  • Added description for selector_values column in condition_set table (line 194).

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.

---

Name must stand alone,
One provider, one true name,
Database says so.

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]

Code Review

The pull request introduces a unique constraint on the provider_name field in the provider_config table. This ensures that each key provider has a unique name, preventing naming conflicts. The changes include database migrations, schema updates, and integration tests to validate the new constraint. Overall, the changes are well-structured and address the intended functionality. However, there are a few areas that could be improved for clarity and efficiency.

Summary of Findings

• Resource Cleanup: Several test cases create provider configurations but don't ensure they are always deleted, especially in case of panics. Using defer to call deleteTestProviderConfigs can prevent orphaned resources.
• Test Case Clarity: Some test cases lack comments explaining their purpose. Adding comments can improve readability and understanding of the test's intent.
• Test Case Efficiency: In several test cases, the append operation to create a slice of provider config IDs is unnecessary. Passing the ID directly to deleteTestProviderConfigs can simplify the code.

Merge Readiness

The pull request is almost ready for merging. Addressing the feedback regarding resource cleanup and test case clarity will improve the overall quality and reliability of the code. I am unable to directly approve the pull request, and users should have others review and approve this code before merging. I recommend addressing the medium severity issues before merging.

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	447.43015ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	298.078209ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	359.950442ms
Throughput	277.82 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	53.117646462s
Average Latency	528.40163ms
Throughput	94.13 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	4944
Failed Requests	56
Concurrent Requests	50
Total Time	42.700569033s
Average Latency	421.889563ms
Throughput	115.78 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	56 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	465.825546ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	293.501568ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	357.28818ms
Throughput	279.89 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	54.451458642s
Average Latency	540.792876ms
Throughput	91.82 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	4935
Failed Requests	65
Concurrent Requests	50
Total Time	44.573057879s
Average Latency	440.246608ms
Throughput	110.72 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	65 occurrences

Standard Benchmark Metrics Skipped or Failed