Skip to content

Transparent encryption#281

Open
urykhy wants to merge 3 commits into
colinmarc:masterfrom
urykhy:encryption
Open

Transparent encryption#281
urykhy wants to merge 3 commits into
colinmarc:masterfrom
urykhy:encryption

Conversation

@urykhy
Copy link
Copy Markdown

@urykhy urykhy commented Nov 10, 2021

Fixes #280

@urykhy urykhy marked this pull request as ready for review November 10, 2021 17:36
@colinmarc
Copy link
Copy Markdown
Owner

colinmarc commented Feb 5, 2022

Hi @urykhy, thanks for the huge contribution!

I am interested in supporting this feature, but I lack the context to understand if the implementation is correct. If you're willing to put a bit more work in, I think we could get this merged.

To start with, I would feel better if we could add some more tests. The tests you added are a good start, but they are far from comprehensively testing this behavior. I wonder if it would make sense to run the entire test suite again (as another job on gh actions) with /_test marked as an encryption zone, rather than trying to duplicate tests here and there. We also need tests for the case where we don't have the key available, to make sure our error messages and exit statuses make sense.

colinmarc
colinmarc reviewed Feb 5, 2022
View reviewed changes
Comment thread Makefile Outdated
colinmarc
colinmarc reviewed Feb 5, 2022
View reviewed changes
Comment thread aes_test.go Outdated
@urykhy urykhy force-pushed the encryption branch 9 times, most recently from 29f7c79 to 07e9b80 Compare February 13, 2022 20:27
colinmarc
colinmarc reviewed Feb 14, 2022
View reviewed changes
Comment thread .github/scripts/install-hdfs.sh Outdated
@urykhy urykhy force-pushed the encryption branch 4 times, most recently from ecd3fb3 to 07b180c Compare February 14, 2022 17:21
colinmarc
colinmarc reviewed Feb 14, 2022
View reviewed changes
Comment thread aes.go Outdated
colinmarc
colinmarc reviewed Feb 14, 2022
View reviewed changes
Comment thread kms.go
colinmarc
colinmarc reviewed Feb 14, 2022
View reviewed changes
Comment thread kms.go Outdated
@urykhy urykhy force-pushed the encryption branch 7 times, most recently from fc86aad to 27b129b Compare February 14, 2022 19:09
urykhy
urykhy commented Feb 15, 2022
View reviewed changes
Comment thread kms.go Outdated
@urykhy urykhy force-pushed the encryption branch 3 times, most recently from 1328f22 to 8e23a17 Compare February 26, 2022 11:30
@urykhy
Copy link
Copy Markdown
Author

urykhy commented Feb 26, 2022

We also need tests for the case where we don't have the key available, to make sure our error messages and exit statuses make sense.

test added.

@urykhy urykhy force-pushed the encryption branch 2 times, most recently from 2680dfb to 982b1e4 Compare February 26, 2022 12:14
colinmarc
colinmarc reviewed Feb 26, 2022
View reviewed changes
Comment thread file_writer.go Outdated
colinmarc
colinmarc reviewed Feb 26, 2022
View reviewed changes
Comment thread aes.go Outdated
@urykhy urykhy force-pushed the encryption branch 3 times, most recently from 00a4963 to 00d1c5b Compare March 7, 2022 07:00
colinmarc
colinmarc reviewed Mar 7, 2022
View reviewed changes
Comment thread file_reader.go Outdated
colinmarc
colinmarc reviewed Mar 7, 2022
View reviewed changes
Comment thread file_writer.go Outdated
@urykhy urykhy requested a review from colinmarc July 9, 2022 08:08
colinmarc
colinmarc requested changes Jul 9, 2022
View reviewed changes
Comment thread file_reader.go Outdated
Comment thread file_reader.go Outdated
Comment thread file_reader.go Outdated
Comment thread file_reader.go Outdated
Comment thread file_writer.go
Comment thread file_writer.go Outdated
Comment thread file_writer.go Outdated
Comment thread file_writer.go Outdated
Comment thread file_writer_test.go
Comment thread file_writer_test.go Outdated
colinmarc
colinmarc reviewed Jul 9, 2022
View reviewed changes
Comment thread file_reader.go
colinmarc
colinmarc reviewed Jul 9, 2022
View reviewed changes
Comment thread file_writer.go Outdated
@colinmarc
Copy link
Copy Markdown
Owner

colinmarc commented Jul 9, 2022

Thanks for sticking with this!

@urykhy urykhy requested a review from colinmarc October 30, 2022 10:46
@Yash9060
Copy link
Copy Markdown

Yash9060 commented Jun 20, 2023

@colinmarc Any plans to merge this pr ?

@urykhy urykhy mentioned this pull request Dec 10, 2023
Open
@urykhy
Copy link
Copy Markdown
Author

urykhy commented Aug 25, 2024

@colinmarc hello, any chances ? ;)

@baolsen baolsen mentioned this pull request Sep 23, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colinmarc colinmarc Awaiting requested review from colinmarc

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Transparent encryption

3 participants

@urykhy @colinmarc @Yash9060