szlta commented on code in PR #13334:
URL: https://github.com/apache/iceberg/pull/13334#discussion_r2219092353
##########
gcp-bundle/build.gradle:
##########
@@ -28,6 +28,7 @@ project(":iceberg-gcp-bundle") {
implementation "com.google.cloud:google-cloud-storage"
implementation "com.google.cloud:google-cloud-bigquery"
implementation "com.google.cloud:google-cloud-core"
+ implementation "com.google.cloud:google-cloud-kms"
Review Comment:
This is meant for engines that use our Iceberg GCP bundle to be able to work
with the proposed `GcpKeyManagementClient`.
However, when I tested this with Spark, I noticed an issue:
```
com.google.cloud.kms.v1.DecryptRequest$Builder.setCiphertext(Lcom/google/protobuf/ByteString;)Lcom/google/cloud/kms/v1/DecryptRequest$Builder;
java.lang.NoSuchMethodError:
com.google.cloud.kms.v1.DecryptRequest$Builder.setCiphertext(Lcom/google/protobuf/ByteString;)Lcom/google/cloud/kms/v1/DecryptRequest$Builder;
at
org.apache.iceberg.gcp.GcpKeyManagementClient.unwrapKey(GcpKeyManagementClient.java:80)
```
This happens because `DecryptRequest$Builder.setCiphertext` takes a
`ByteString` object, whose class is shaded and relocated in the `gcp-bundle`
(check line 53 in this file). This results in changing the signature of
`com.google.cloud.kms.v1.DecryptRequest$Builder.setCiphertext(Lcom/google/protobuf/ByteString;)Lcom/google/cloud/kms/v1/DecryptRequest$Builder;`
to
`com.google.cloud.kms.v1.DecryptRequest$Builder.setCiphertext(Lorg/apache/iceberg/gpc/shaded/com/google/protobuf/ByteString;)Lcom/google/cloud/kms/v1/DecryptRequest$Builder;`
in the bundle jar, hence the above error.
Engine apps not making use of the gcp-bundle jar (i.e. bringing their own
google-cloud-kms and protobuf dependencies) work without issues.
I was considering the application of the same relocation logic in `gcp`
module, and although that fixes the issue for `gcp-bundle` consumers, it causes
a `NoSuchMethodError` for non-bundle consumers, so I can't seem to find way
that ensures that the implementation works in both cases (like how the recently
merged `AwsKeyManagementClient` does work). That is, without removing the
relocation in line 53.
Please let me know what your opinion is @nastra, @amogh-jahagirdar, @pvary
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
