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-1457 Rename principal key on user API level to just a key #154

Open
wants to merge 26 commits into
base: PG-1457-key-management-funcs-renaming
Choose a base branch
from

Conversation

artemgavrilov
Copy link
Collaborator

@artemgavrilov artemgavrilov commented Mar 19, 2025

PG-1457

This PR replaces principal key with just a key on user API level, as it's the only key that user can directly interact with.

This PR is made on top of #126

@@ -201,12 +201,12 @@ Use these functions to create a new principal key for a specific scope such as a

Princial keys are stored on key providers by the name specified in this function - for example, when using the Vault provider, after creating a key named "foo", a key named "foo" will be visible on the Vault server at the specified mount point.

### pg_tde_set_principal_key
###
Copy link
Collaborator

Choose a reason for hiding this comment

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

You accidentally deleted this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep

BTW this PR is made on top of #126
So it contains a lot of commits from there, please review 126 first

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@artemgavrilov artemgavrilov changed the base branch from release-17.4 to PG-1457-key-management-funcs-renaming March 20, 2025 10:13
@nastena1606 nastena1606 temporarily deployed to PG-1457-rename-principal-key - tde-RC PR #154 March 20, 2025 10:22 — with Render Destroyed
@artemgavrilov artemgavrilov marked this pull request as ready for review March 20, 2025 13:30
@artemgavrilov artemgavrilov requested a review from jeltz March 20, 2025 13:30
@@ -201,12 +201,12 @@ Use these functions to create a new principal key for a specific scope such as a

Princial keys are stored on key providers by the name specified in this function - for example, when using the Vault provider, after creating a key named "foo", a key named "foo" will be visible on the Vault server at the specified mount point.

### pg_tde_set_principal_key_using_database_key_provider
### pg_tde_set_key_using_database_key_provider

Creates or rotates the principal key for the current database using the specified database key provider and key name.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we keep the notion of a principal key?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably yes. They still principal by meaning, but they are only available to the user.

What do you think @dutow ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, it is still called principal key, we just simplified the names of the functions.

@@ -85,7 +85,7 @@ TDEXlogCheckSane(void)
if (key == NULL)
{
ereport(ERROR,
(errmsg("WAL encryption can only be enabled with a properly configured principal key. Disable pg_tde.wal_encrypt and create one using pg_tde_set_server_principal_key_using_global_key_provider() or pg_tde_set_principal_key_using_global_key_provider() before enabling it.")));
(errmsg("WAL encryption can only be enabled with a properly configured principal key. Disable pg_tde.wal_encrypt and create one using pg_tde_set_server_key_using_global_key_provider() or pg_tde_set_key_using_global_key_provider() before enabling it.")));
Copy link
Member

Choose a reason for hiding this comment

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

It's not critical but there is an inconsistency - we use parenthesis with function names here but don't use it in pg_tde_map.c and pg_tde_event_capture.c

@artemgavrilov artemgavrilov force-pushed the PG-1457-key-management-funcs-renaming branch from 3b48f45 to abbcdac Compare April 4, 2025 14:36
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.

5 participants