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

Added high level overview to documentation #112

Open
wants to merge 3 commits into
base: release-17.4
Choose a base branch
from
Open

Conversation

dutow
Copy link
Collaborator

@dutow dutow commented Mar 7, 2025

This is just a conversion of the google doc into markdown, with actualizing some of the outdated details in the document.

The last section (researc/investigation topics) is left out, as that doesn't make much sense in a public documentation.

* Temporary tables
* Write ahead log
* PG-994 System tables (not yet implemented)
* PG-993 Temporary files (not yet implemented)
Copy link
Member

Choose a reason for hiding this comment

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

Make sense to have PG-XXX as links or remove it

Copy link
Collaborator

Choose a reason for hiding this comment

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

I vote for removing it. We'll have to remove the links anyway once implemented


## Main components

Pg_tde consist of 3 main components:
Copy link
Member

Choose a reason for hiding this comment

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

It says 3 components but in following text is har to comprehend what those 3 are exactly are. Maybe, either add some kind of numeration in the text (not necessary numbered list, maybe just "Firstly, ...") or remove 3 from these sentence

Copy link
Collaborator

Choose a reason for hiding this comment

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

Reworded


### Two key hierarchy

`Pg_tde` uses a two level key structure, also found commonly in other database servers:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think pg_tde should start from the capital letter even in the beginning of a sentence etc. We don't do this across the rest of the docs anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make sense to add a digram how all those components interact with each other?


### Internal key storage

Internal keys, and generally `pg_tde` metadata is kept in a single directory in `$PGDATA/pg_tde`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Internal keys, and generally `pg_tde` metadata is kept in a single directory in `$PGDATA/pg_tde`.
Internal keys and generally `pg_tde` metadata are kept in a single directory in `$PGDATA/pg_tde`.


### Adding providers

Keyring providers can be added to either the GLOBAL or to the database specific scope.
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to elaborate on what the GLOBAL scope is and what it is for?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes as it is confusing

Copy link
Collaborator

Choose a reason for hiding this comment

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

But I'd ask a different question: do we need to have a User Interface section in the Architecture doc at all? It basically duplicates what we have in the whole doc set: how to set up pg_tde, what functions and params are there, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is structured differently, as it was/is intended as a single document overview of everything.

pg_tde_set_server_principal_key(‘key-name', ‘provider-name', ensure_new_key)
```

`Ensure_new_key` is a boolean parameter defaulting to false.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`Ensure_new_key` is a boolean parameter defaulting to false.
`ensure_new_key` is a boolean parameter defaulting to false.

Also, the `pg_tde_(grant/revoke)_key_management_to_role` function deals with only the specific permission for the above function:
it allows a user to change the key for the database, but not to modify the provider configuration.

### Creating tables
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Creating tables
### Creating encrypted tables

* Encrypted internal keys and internal key mapping to tables
* Information about the key providers

There's also a special global section marked with the OID 607, which includes the global key providers / global internal keys.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the global scope?


There's also a special global section marked with the OID 607, which includes the global key providers / global internal keys.

This is used by the WAL encryption, and can optionally be used by specific databases too, if global provider inheritance is enabled.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't they use the default provider for this purpose?
If not, let's explain the default scope and default provider here too.

dutow and others added 3 commits March 17, 2025 21:44
This is just a conversion of the google doc into markdown, with
actualizing some of the outdated details in the document.

The last section (researc/investigation topics) is left out, as that
doesn't make much sense in a public documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants