-
Notifications
You must be signed in to change notification settings - Fork 0
External file decryptor bindings #57
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
Changes from all commits
b986ba0
42f95bd
6af4c5f
bb658dd
e10e716
483ea2c
1faa390
2606f06
a858935
03bccda
bf9a0ea
2269958
eb30bd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,9 +24,11 @@ from pyarrow._parquet cimport (ParquetCipher, | |
| CFileEncryptionProperties, | ||
| CExternalFileEncryptionProperties, | ||
| CFileDecryptionProperties, | ||
| CExternalFileDecryptionProperties, | ||
| FileEncryptionProperties, | ||
| ExternalFileEncryptionProperties, | ||
| FileDecryptionProperties, | ||
| ExternalFileDecryptionProperties, | ||
| ParquetCipher_AES_GCM_V1, | ||
| ParquetCipher_AES_GCM_CTR_V1, | ||
| ParquetCipher_EXTERNAL_DBPA_V1) | ||
|
|
@@ -52,20 +54,16 @@ cdef class KmsConnectionConfig(_Weakrefable): | |
| @staticmethod | ||
| cdef wrap(const CKmsConnectionConfig& config) | ||
|
|
||
|
|
||
| cdef shared_ptr[CCryptoFactory] pyarrow_unwrap_cryptofactory(object crypto_factory) except * | ||
| cdef shared_ptr[CKmsConnectionConfig] pyarrow_unwrap_kmsconnectionconfig(object kmsconnectionconfig) except * | ||
| cdef shared_ptr[CEncryptionConfiguration] pyarrow_unwrap_encryptionconfig(object encryptionconfig) except * | ||
| cdef shared_ptr[CDecryptionConfiguration] pyarrow_unwrap_decryptionconfig(object decryptionconfig) except * | ||
| cdef shared_ptr[CExternalEncryptionConfiguration] pyarrow_unwrap_external_encryptionconfig(object encryptionconfig) except * | ||
|
|
||
|
|
||
| cdef class ExternalEncryptionConfiguration(EncryptionConfiguration): | ||
| cdef shared_ptr[CExternalEncryptionConfiguration] external_configuration | ||
| cdef inline shared_ptr[CExternalEncryptionConfiguration] unwrap_external(self) nogil | ||
| cdef shared_ptr[CExternalEncryptionConfiguration] pyarrow_unwrap_external_encryptionconfig(object externalencryptionconfig) except * | ||
|
|
||
| cdef class ExternalDecryptionConfiguration(DecryptionConfiguration): | ||
| cdef shared_ptr[CExternalDecryptionConfiguration] external_configuration | ||
| cdef inline shared_ptr[CExternalDecryptionConfiguration] unwrap_external(self) nogil | ||
|
|
||
| cdef shared_ptr[CCryptoFactory] pyarrow_unwrap_cryptofactory(object crypto_factory) except * | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming moving the definitions here is just for readability and nothing functional, right?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. |
||
| cdef shared_ptr[CKmsConnectionConfig] pyarrow_unwrap_kmsconnectionconfig(object kmsconnectionconfig) except * | ||
| cdef shared_ptr[CEncryptionConfiguration] pyarrow_unwrap_encryptionconfig(object encryptionconfig) except * | ||
| cdef shared_ptr[CDecryptionConfiguration] pyarrow_unwrap_decryptionconfig(object decryptionconfig) except * | ||
| cdef shared_ptr[CExternalEncryptionConfiguration] pyarrow_unwrap_external_encryptionconfig(object externalencryptionconfig) except * | ||
| cdef shared_ptr[CExternalDecryptionConfiguration] pyarrow_unwrap_external_decryptionconfig(object externaldecryptionconfig) except * | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -419,13 +419,29 @@ cdef class ExternalDecryptionConfiguration(DecryptionConfiguration): | |
| __slots__ = () | ||
|
|
||
| def __init__(self, *, cache_lifetime=None, app_context=None, connection_config=None): | ||
| super().__init__(cache_lifetime=cache_lifetime) | ||
| # Initialize the pointer first so the get/set forwards work. | ||
| # Super init will run the setters/getters below so we need the pointer to exist. | ||
| self.external_configuration.reset(new CExternalDecryptionConfiguration()) | ||
| super().__init__(cache_lifetime=cache_lifetime) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change of order is a bit scary. Why is there a dependency on the subclass before calling the init of the superclass? Primarily for my understanding, but let's improve the comment.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated comment. Because the super class constructor will invoke the setters that are overridden below, so they need the pointer to be ready, otherwise we get a null crash. |
||
|
|
||
| self.external_configuration.get().cache_lifetime_seconds = \ | ||
| self.configuration.get().cache_lifetime_seconds | ||
|
|
||
| if app_context is not None: | ||
| self.app_context = app_context | ||
| if connection_config is not None: | ||
| self.connection_config = connection_config | ||
|
|
||
| """ Forward all attributes get/set methods to the superclass """ | ||
| """ The superclass already converts to/from bytes and does additional processing needed """ | ||
| @property | ||
| def cache_lifetime(self): | ||
| return DecryptionConfiguration.cache_lifetime.__get__(self) | ||
|
|
||
| @cache_lifetime.setter | ||
| def cache_lifetime(self, value): | ||
| DecryptionConfiguration.cache_lifetime.__set__(self, value) | ||
| self.external_configuration.get().cache_lifetime_seconds = value.total_seconds() | ||
|
|
||
| @property | ||
| def app_context(self): | ||
|
|
@@ -759,6 +775,39 @@ cdef class CryptoFactory(_Weakrefable): | |
| c_file_decryption_properties) | ||
| return FileDecryptionProperties.wrap(file_decryption_properties) | ||
|
|
||
| def external_file_decryption_properties( | ||
| self, | ||
| KmsConnectionConfig kms_connection_config, | ||
| ExternalDecryptionConfiguration decryption_config): | ||
| """Create file decryption properties. | ||
| Parameters | ||
| ---------- | ||
| kms_connection_config : KmsConnectionConfig | ||
| Configuration of connection to KMS | ||
| decryption_config : ExternalDecryptionConfiguration | ||
| Configuration of the decryption, such as cache timeout and the information on how to connect the external decryption service. | ||
| Returns | ||
| ------- | ||
| file_decryption_properties : ExternalFileDecryptionProperties | ||
| File decryption properties. | ||
| """ | ||
| cdef: | ||
| CExternalDecryptionConfiguration c_decryption_config | ||
| CResult[shared_ptr[CExternalFileDecryptionProperties]] \ | ||
| c_file_decryption_properties | ||
| if decryption_config is None: | ||
| c_decryption_config = CExternalDecryptionConfiguration() | ||
| else: | ||
| c_decryption_config = deref(decryption_config.unwrap_external().get()) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I lost a little track around here. If not can you add a comment at the if-else level on what this is doing.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Boilerplate check that we have even an empty decryption config to work with, in case none was sent. |
||
| with nogil: | ||
| c_file_decryption_properties = \ | ||
| self.factory.get().SafeGetExternalFileDecryptionProperties( | ||
| deref(kms_connection_config.unwrap().get()), | ||
| c_decryption_config) | ||
| file_decryption_properties = GetResultValue( | ||
| c_file_decryption_properties) | ||
| return ExternalFileDecryptionProperties.wrap_external(file_decryption_properties) | ||
|
|
||
| def remove_cache_entries_for_token(self, access_token): | ||
| self.factory.get().RemoveCacheEntriesForToken(tobytes(access_token)) | ||
|
|
||
|
|
@@ -800,4 +849,4 @@ cdef shared_ptr[CDecryptionConfiguration] pyarrow_unwrap_decryptionconfig(object | |
| cdef shared_ptr[CExternalDecryptionConfiguration] pyarrow_unwrap_external_decryptionconfig(object decryptionconfig) except *: | ||
| if isinstance(decryptionconfig, ExternalDecryptionConfiguration): | ||
| return (<ExternalDecryptionConfiguration> decryptionconfig).unwrap_external() | ||
| raise TypeError("Expected DecryptionConfiguration, got %s" % type(decryptionconfig)) | ||
| raise TypeError("Expected ExternalDecryptionConfiguration, got %s" % type(decryptionconfig)) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ from pyarrow._parquet cimport (ParquetCipher, | |
| CFileEncryptionProperties, | ||
| CExternalFileEncryptionProperties, | ||
| CFileDecryptionProperties, | ||
| CExternalFileDecryptionProperties, | ||
| ParquetCipher_AES_GCM_V1, | ||
| ParquetCipher_AES_GCM_CTR_V1, | ||
| ParquetCipher_EXTERNAL_DBPA_V1) | ||
|
|
@@ -126,6 +127,9 @@ cdef extern from "parquet/encryption/crypto_factory.h" \ | |
| shared_ptr[CFileDecryptionProperties] GetFileDecryptionProperties( | ||
| const CKmsConnectionConfig& kms_connection_config, | ||
| const CDecryptionConfiguration& decryption_config) except +* | ||
| shared_ptr[CExternalFileDecryptionProperties] GetExternalFileDecryptionProperties( | ||
| const CKmsConnectionConfig& kms_connection_config, | ||
| const CExternalDecryptionConfiguration& decryption_config) except +* | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😮💨 |
||
| void RemoveCacheEntriesForToken(const c_string& access_token) except + | ||
| void RemoveCacheEntriesForAllTokens() except + | ||
|
|
||
|
|
@@ -164,4 +168,8 @@ cdef extern from "arrow/python/parquet_encryption.h" \ | |
| CResult[shared_ptr[CFileDecryptionProperties]] \ | ||
| SafeGetFileDecryptionProperties( | ||
| const CKmsConnectionConfig& kms_connection_config, | ||
| const CDecryptionConfiguration& decryption_config) | ||
| const CDecryptionConfiguration& decryption_config) | ||
| CResult[shared_ptr[CExternalFileDecryptionProperties]] \ | ||
| SafeGetExternalFileDecryptionProperties( | ||
| const CKmsConnectionConfig& kms_connection_config, | ||
| const CExternalDecryptionConfiguration& decryption_config) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding, are the pxd modules compiled (so the change in params cause a compilation error if done wrong) or are these interpreted? If the latter, I'm assuming this is abundantly tested by unittests, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are compiled. If they're not included here, we get compilation errors.