Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PG-1258: Implement pg_tde_verify_principal_key functions #43

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

dutow
Copy link
Collaborator

@dutow dutow commented Feb 5, 2025

These functions allow users to check if the keyring is accessible, and if it still returns the same key as stored in memory for the server.

Review note: PR is based on my previous PR, only the last commit is new.

@dutow dutow requested a review from dAdAbird as a code owner February 5, 2025 20:11
@dutow dutow force-pushed the pg1258 branch 2 times, most recently from a99eda3 to 5a18e58 Compare February 5, 2025 20:30
These functions allow users to check if the keyring is accessible,
and if it still returns the same key as stored in memory for the
server.
Copy link
Collaborator

@jeltz jeltz left a comment

Choose a reason for hiding this comment

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

Looks good but I think we should have a test case for it.

@dutow
Copy link
Collaborator Author

dutow commented Feb 6, 2025

@jeltz There's a basic test for both the successful and failing case with the file provider. This is a simple function, that's all it is to it. I added it to the existing key_provider test suite, as this function sanity checks the configuration of a key provider.

Constructing a testcase for the other failing case ("Key returned by keyring and cached in pg_tde is different") would be a bit more complex, as that's not something that could realistically happen in a real world situation. I could force a test for it with the file keyring, but that would be a bit of a hack.

@jeltz
Copy link
Collaborator

jeltz commented Feb 7, 2025

Would have preferred a failing test but your call.

@jeltz
Copy link
Collaborator

jeltz commented Feb 7, 2025

Ignore my last emssage, somehow I missed the failing case. My apologies. Still approved of course.

@dutow dutow merged commit 6af8fdd into percona:TDE_REL_17_STABLE Feb 7, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants