You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "MartinNowak (via GitHub)" <gi...@apache.org> on 2025/05/07 13:32:56 UTC

[PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

MartinNowak opened a new pull request, #46347:
URL: https://github.com/apache/arrow/pull/46347

   Please remove this line and the above text before creating your pull request.
   
   ### Rationale for this change
   
   Fixing #38914 to support uniform encryption when writing parquet files.
   
   ### What changes are included in this PR?
   
   Exposing EncryptionConfiguration.uniform_encryption to pyarrow.
   
   ### Are these changes tested?
   
   Yes, see included tests.
   
   ### Are there any user-facing changes?
   
   Addition of uniform_encryption parameter to EncryptionConfiguration.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on PR #46347:
URL: https://github.com/apache/arrow/pull/46347#issuecomment-2862664603

   > Thanks for the quick review @rok. That was already tested.
   
   Oh yeah, you're right.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "MartinNowak (via GitHub)" <gi...@apache.org>.
MartinNowak commented on code in PR #46347:
URL: https://github.com/apache/arrow/pull/46347#discussion_r2079186887


##########
python/pyarrow/tests/parquet/test_encryption.py:
##########
@@ -133,6 +133,32 @@ def test_encrypted_parquet_write_read(tempdir, data_table):
     assert data_table.equals(result_table)
 
 
+def test_uniform_encrypted_parquet_write_read(tempdir, data_table):
+    """Write an encrypted parquet, verify it's encrypted, and then read it."""
+    path = tempdir / PARQUET_NAME
+
+    # Encrypt the footer and all columns with the footer key,
+    encryption_config = pe.EncryptionConfiguration(
+        footer_key=FOOTER_KEY_NAME,
+        uniform_encryption=True,
+        encryption_algorithm="AES_GCM_V1",
+        cache_lifetime=timedelta(minutes=5.0),
+        data_key_length_bits=256)
+
+    kms_connection_config, crypto_factory = write_encrypted_file(
+        path, data_table, FOOTER_KEY_NAME, COL_KEY_NAME, FOOTER_KEY, b"",
+        encryption_config)

Review Comment:
   I followed a test below 
   https://github.com/apache/arrow/blob/b78e6e27ac0984fd368c86065261fdb2efa6054e/python/pyarrow/tests/parquet/test_encryption.py#L265-L267 and it seems indeed clearer to pass the name instead of e.g. `None` to ensure that the key is really not used.
   https://github.com/apache/arrow/blob/b78e6e27ac0984fd368c86065261fdb2efa6054e/python/pyarrow/tests/parquet/test_encryption.py#L84-L93



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on code in PR #46347:
URL: https://github.com/apache/arrow/pull/46347#discussion_r2077946945


##########
python/pyarrow/tests/parquet/test_encryption.py:
##########
@@ -133,6 +133,32 @@ def test_encrypted_parquet_write_read(tempdir, data_table):
     assert data_table.equals(result_table)
 
 
+def test_uniform_encrypted_parquet_write_read(tempdir, data_table):
+    """Write an encrypted parquet, verify it's encrypted, and then read it."""
+    path = tempdir / PARQUET_NAME
+
+    # Encrypt the footer and all columns with the footer key,
+    encryption_config = pe.EncryptionConfiguration(
+        footer_key=FOOTER_KEY_NAME,
+        uniform_encryption=True,
+        encryption_algorithm="AES_GCM_V1",
+        cache_lifetime=timedelta(minutes=5.0),
+        data_key_length_bits=256)
+
+    kms_connection_config, crypto_factory = write_encrypted_file(
+        path, data_table, FOOTER_KEY_NAME, COL_KEY_NAME, FOOTER_KEY, b"",
+        encryption_config)

Review Comment:
   Does `COL_KEY_NAME` need to be passed here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "MartinNowak (via GitHub)" <gi...@apache.org>.
MartinNowak commented on PR #46347:
URL: https://github.com/apache/arrow/pull/46347#issuecomment-2862242734

   > Should raise: "Either column_keys or uniform_encryption must be set". Could we add test for that?
   
   Thanks for the quick review @rok. That was already tested. https://github.com/apache/arrow/blob/b78e6e27ac0984fd368c86065261fdb2efa6054e/python/pyarrow/tests/parquet/test_encryption.py#L253-L267.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on PR #46347:
URL: https://github.com/apache/arrow/pull/46347#issuecomment-2862686001

   The [gdb issue](https://github.com/apache/arrow/actions/runs/14885209072/job/41802856726?pr=46347) seems unrelated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #46347:
URL: https://github.com/apache/arrow/pull/46347


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #46347:
URL: https://github.com/apache/arrow/pull/46347#discussion_r2083998536


##########
python/pyarrow/tests/parquet/test_encryption.py:
##########
@@ -133,6 +133,32 @@ def test_encrypted_parquet_write_read(tempdir, data_table):
     assert data_table.equals(result_table)
 
 
+def test_uniform_encrypted_parquet_write_read(tempdir, data_table):
+    """Write an encrypted parquet, verify it's encrypted, and then read it."""
+    path = tempdir / PARQUET_NAME
+
+    # Encrypt the footer and all columns with the footer key,
+    encryption_config = pe.EncryptionConfiguration(
+        footer_key=FOOTER_KEY_NAME,
+        uniform_encryption=True,
+        encryption_algorithm="AES_GCM_V1",
+        cache_lifetime=timedelta(minutes=5.0),
+        data_key_length_bits=256)

Review Comment:
   Perhaps add
   ```suggestion
           data_key_length_bits=256)
       assert encryption_config.uniform_encryption is True
   ```
   
   (and add a similar assert in `test_encrypted_parquet_write_read` above)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #46347:
URL: https://github.com/apache/arrow/pull/46347#discussion_r2086556265


##########
python/pyarrow/_parquet_encryption.pyx:
##########
@@ -103,6 +106,18 @@ cdef class EncryptionConfiguration(_Weakrefable):
                 [f"{k}: {', '.join(v)}" for k, v in value.items()])
             self.configuration.get().column_keys = tobytes(column_keys)
 
+    @property
+    def uniform_encryption(self):
+        """Encrypt footer and all columns with the same encryption key.

Review Comment:
   nit
   ```suggestion
           """Whether to encrypt footer and all columns with the same encryption key.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on PR #46347:
URL: https://github.com/apache/arrow/pull/46347#issuecomment-2876032937

   Assuming CI passes I'll merge @pitrou 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "MartinNowak (via GitHub)" <gi...@apache.org>.
MartinNowak commented on PR #46347:
URL: https://github.com/apache/arrow/pull/46347#issuecomment-2858789204

   > failing checks
   [Python / AMD64 Conda Python 3.10 Without Pandas (pull_request)](https://github.com/apache/arrow/actions/runs/14885209072/job/41802856726?pr=46347)
   Python / AMD64 Conda Python 3.10 Without Pandas (pull_request)Failing after 17m
   
   GDB test failures seem unrelated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GH-38914: [Python] Add EncryptionConfiguration.uniform_encryption [arrow]

Posted by "MartinNowak (via GitHub)" <gi...@apache.org>.
MartinNowak commented on code in PR #46347:
URL: https://github.com/apache/arrow/pull/46347#discussion_r2086472128


##########
python/pyarrow/tests/parquet/test_encryption.py:
##########
@@ -133,6 +133,32 @@ def test_encrypted_parquet_write_read(tempdir, data_table):
     assert data_table.equals(result_table)
 
 
+def test_uniform_encrypted_parquet_write_read(tempdir, data_table):
+    """Write an encrypted parquet, verify it's encrypted, and then read it."""
+    path = tempdir / PARQUET_NAME
+
+    # Encrypt the footer and all columns with the footer key,
+    encryption_config = pe.EncryptionConfiguration(
+        footer_key=FOOTER_KEY_NAME,
+        uniform_encryption=True,
+        encryption_algorithm="AES_GCM_V1",
+        cache_lifetime=timedelta(minutes=5.0),
+        data_key_length_bits=256)

Review Comment:
   Done @rok and @pitrou.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org