adamreeve commented on code in PR #51350:
URL: https://github.com/apache/arrow/pull/51350#discussion_r4021978909
##########
cpp/src/parquet/encryption/file_key_unwrapper.cc:
##########
@@ -133,25 +133,31 @@ KeyWithMasterId
FileKeyUnwrapper::GetDataEncryptionKey(const KeyMaterial& key_ma
return KeyWithMasterId(std::move(data_key), master_key_id);
}
+void FileKeyUnwrapper::EnableReadingKmsUrl() { read_kms_url_ = true; }
+
std::shared_ptr<KmsClient>
FileKeyUnwrapper::GetKmsClientFromConfigOrKeyMaterial(
const KeyMaterial& key_material) {
std::string& kms_instance_id = kms_connection_config_.kms_instance_id;
if (kms_instance_id.empty()) {
kms_instance_id = key_material.kms_instance_id();
if (kms_instance_id.empty()) {
throw ParquetException(
- "KMS instance ID is missing both in both kms connection
configuration and file "
+ "KMS instance ID is missing in both the KMS connection configuration
and file "
"key material");
}
}
std::string& kms_instance_url = kms_connection_config_.kms_instance_url;
if (kms_instance_url.empty()) {
- kms_instance_url = key_material.kms_instance_url();
- if (kms_instance_url.empty()) {
- throw ParquetException(
- "KMS instance ID is missing both in both kms connection
configuration and file "
- "key material");
+ if (read_kms_url_) {
+ kms_instance_url = key_material.kms_instance_url();
+ if (kms_instance_url.empty()) {
+ throw ParquetException(
+ "KMS instance URL is missing in both the KMS connection
configuration and "
+ "the file key material");
+ }
+ } else {
+ kms_instance_url = KmsClient::kKmsInstanceUrlDefault;
Review Comment:
I don't think this is an issue specific to the new `read_kms_url` flag. The
cache was already based on the ID and access token only. Whether that makes
sense is debatable, but I think changing that is out of scope for this PR. The
token would have already been sent due to the `read_kms_url=True` read, so this
doesn't seem like a major concern.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
