You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Peter Rozsa (Code Review)" <ge...@cloudera.org> on 2026/08/26 12:45:54 UTC
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Peter Rozsa has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24755
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
IMPALA-15144: Fetch and store credentials from REST catalog
Extract vended storage credentials from an Iceberg REST catalog's
LoadTableResponse and store them in the Iceberg table object.
A new Credential class is added: a boundary type holding a location
prefix plus a config map. Its toHadoopConfig() maps platform-specific
keys to Hadoop-compatible ones.
Credential extraction happens after table loading via
extractCredential(); each credential is then mapped to Hadoop-compatible
configurations.
VendedCredentialsFileIO is added as a wrapper FileIO because the
underlying Iceberg-supplied HadoopFileIO caches the filesystem and
stores the credential per filesystem, which can be a problem on
multi-cloud tables or tables with files that have different ACL rules.
RESTCatalogProperties is extended with credential-vending-based property
handling, and the property translation mechanism is extracted into a
separate class to make it reusable for cloud-provider-specific config
key mapping.
Tests:
- ConfigTranslatorTests added
- CredentialTest added
Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Assisted-by: Claude Opus 4.8 (opencode)
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/RESTCatalogProperties.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslator.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
M fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
A fe/src/main/java/org/apache/impala/common/Credential.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java
M fe/src/test/java/org/apache/impala/catalog/iceberg/TestRESTCatalogProperties.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java
14 files changed, 1,242 insertions(+), 195 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/24755/1
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
IMPALA-15144: Fetch and store credentials from REST catalog
Extract vended storage credentials from an Iceberg REST catalog's
LoadTableResponse and store them in the Iceberg table object.
A new Credential class is added: a boundary type holding a location
prefix plus a config map. Its toHadoopConfig() maps platform-specific
keys to Hadoop-compatible ones.
Credential extraction happens after table loading via
extractCredential(); each credential is then mapped to Hadoop-compatible
configurations.
RESTCatalogProperties is extended with credential-vending-based property
handling, and the property translation mechanism is extracted into a
separate class to make it reusable for cloud-provider-specific config
key mapping.
Tests:
- ConfigTranslatorTests added
- CredentialTest added
Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Assisted-by: Claude Opus 4.8 (opencode)
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/RESTCatalogProperties.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslator.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
M fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
A fe/src/main/java/org/apache/impala/common/Credential.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java
M fe/src/test/java/org/apache/impala/catalog/iceberg/TestRESTCatalogProperties.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java
13 files changed, 1,146 insertions(+), 195 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/24755/5
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 2:
(4 comments)
Read through the credential path - extraction, translation, thrift - and the FileIO. Four comments; the two on VendedCredentialsFileIO are the ones I'd care about.
On coverage: IcebergRestCatalogTest already overrides RESTCatalogAdapter.execute(), and LoadTableResponse.Builder has addCredential() in 1.10.2, so having the test server vend a fake credential looks reachable, and it would cover extract() -> getCredentials() -> thrift end to end. Is that planned for a follow-up?
Also curious which side ends up reading expiry_ms - is that the patch that consumes the credentials in the backend?
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@49
PS2, Line 49: public class VendedCredentialsFileIO extends HadoopFileIO
Only the two newInputFile() overloads are overridden here. In Iceberg 1.10.2 the rest of HadoopFileIO's DelegateFileIO surface still resolves through getConf(): newOutputFile() ends in HadoopOutputFile.fromPath(path, getConf()), deleteFile(), listPrefix() and deletePrefix() go through Util.getFs(path, getConf()), and deleteFiles() fans out to the same deleteFile(). All of them use the shared FileSystem cache without the vended credential.
That surface is used in-tree - ImpalaIcebergDeleteOrphanFiles has table.io().deleteFile() - though I'm not sure a REST catalog table can reach it today. Either way the fallback is silent: on a bucket that needs the vended credential it fails, and on one the ambient credentials also cover it quietly uses those instead. Would it make sense to route them through fileSystemFor() too, or to throw there so the gap stays visible?
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@57
PS2, Line 57: private static final Cache<String, FileSystem> FS_CACHE =
closeEvicted() closes the FileSystem on eviction, but HadoopInputFile keeps the instance it was built with (private final FileSystem fs in 1.10.2) and newStream() calls fs.open() on that field, so the filesystem is used when the file is read, not when it is created. An eviction - by maximumSize=100, or after ten idle minutes on that key - can then close a FileSystem an outstanding InputFile still holds, and the read fails with "Filesystem closed" rather than anything credential-shaped.
Dropping the close avoids that at the cost of releasing the connection pool later; a refcount would be exact but more code. Do you have a preference?
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@135
PS2, Line 135: private Credential findCredential(String location) {
The match is a raw startsWith, so a credential vended for s3://bucket/tab also covers s3://bucket/table2/... . Longest-prefix makes that unlikely in practice - should the match require a path boundary anyway?
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java@37
PS2, Line 37: public class CredentialScheme {
Nothing exercises this class in the patch: ConfigTranslatorTest builds its own MapRenameRule/ProgrammaticRule instances with s3-like keys, so the registry itself - which Iceberg key becomes which fs.* key, the scheme parsing in forPrefix(), the expiry units - is uncovered. A small test per scheme, plus an unknown scheme returning null, would pin down what the backend ends up consuming.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 16:07:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@910
PS2, Line 910: for (Credential cred : icebergTable.getCredentials()) {
> Two facts and then it's yours and Zoltan's call. The file is documented as
COPY TESTCASE is broken for Iceberg tables (IMPALA-12843), but users could still invoke it to dump metadata information about the tables, so I think the right call is to be defensive here, and not let credentials creep into diagnostic dumps.
We should also add a comment here, because it's not trivial why we restrict it to DESCRIPTOR_ONLY. It would be nice to have a more generic mechanism for this, e.g. annotating thrift fields as sensitive and explicitly redacting them from logs/dumps. But it's out of the scope of this change.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Aug 2026 07:12:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 2:
(10 comments)
Left a few comments, but looks great overall!
http://gerrit.cloudera.org:8080/#/c/24755/2/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/24755/2/common/thrift/CatalogObjects.thrift@701
PS2, Line 701: struct TCredential {
Please add comments for this structure and its members.
http://gerrit.cloudera.org:8080/#/c/24755/2/common/thrift/CatalogObjects.thrift@704
PS2, Line 704: expiry_ms
nit: 'expires_at_ms' is more clear and it's closer to what being used by different object stores
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@910
PS2, Line 910: type == ThriftObjectType.DESCRIPTOR_ONLY
Why is it only set for DESCRIPTOR_ONLY?
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@65
PS2, Line 65: IcebergRESTCatalog.extractCredentials
Credential.extract() in IcebergMetaProvider?
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@120
PS2, Line 120: (
nit: missing space
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@143
PS2, Line 143: Credential cred = new Credential(storageCred.prefix(), storageCred.config());
We could create a member list of Credential objects from storageCredentials_, and use it in findCredential() to avoid reconstructing the same credentials (and hadoop config) multiple times.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java@39
PS2, Line 39: private static final String S3_ACCESS_KEY_ID = "s3.access-key-id";
Can you add a link to the source of these configs, e.g. is it https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L3873 ?
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java@65
PS2, Line 65: private static final String GCS_OAUTH2_TOKEN = "gcs.oauth2.token";
Please add a link to the source of these configs.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java@33
PS2, Line 33: public interface TranslationRule {
Nice refactoring!
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java
File fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java@93
PS2, Line 93: }
Unit tests could be added for
* toHadoopConfig()
* getExpiryMs()
* identity()
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Aug 2026 11:19:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 3:
(3 comments)
> Patch Set 3:
>
> (2 comments)
>
> Went through PS3: the FileIO, the translate layer, the metaprovider wiring and the new tests. Checked the Iceberg side against iceberg-core 1.10.2 rather than from memory.
>
> The first thing I'd look at is the DESCRIPTOR_ONLY condition that came out of getTIcebergTable(), since the FULL form is what COPY TESTCASE serializes into a file and the vended keys now travel with it - details in the thread on FeIcebergTable. Beyond that: the write half of the FileIO still resolves through getConf(), which meets the REST INSERT path landing in 24682; the prefix match still has no path boundary; and nothing closes the cached FileSystems now that the removal listener is gone. The rest are nits.
Thanks, Aleksandr!
I'd like to ask you to make your AI reviews less verbose, and please check every claim that the AI states as it could be misleading.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@910
PS2, Line 910: (Credential cred : icebergTable.getCrede
> The condition had a reason, even if it wasn't written down: DESCRIPTOR_ONLY
Wow, your AI gave the same output as my AI gave me before pushing PS3, but the reality is that COPY TESTCASE was never used for bug reports and also the vended credentials are short-living credentials, so those would be expired when the theoretical bug report is created.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@49
PS2, Line 49: * for it, rather than whatever the shared Hadoop FileSystem cache happens to hold.
> That holds for everything reaching this FileIO today, but it stops holding
Let's handle REST catalog writing support and this patch separately, I would like to focus on the credential vending part in this patch, and if the REST writing support lands before this one, I'll adjust it
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@57
PS2, Line 57: private static final long FS_CACHE_TTL_MINUTES = 10;
> That settles the use-after-close. What's left is that nothing closes them a
Yes, it's intended, this is the Hadoop-like behavior, and I think the review history is sufficient for now.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Aug 2026 21:19:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
IMPALA-15144: Fetch and store credentials from REST catalog
Extract vended storage credentials from an Iceberg REST catalog's
LoadTableResponse and store them in the Iceberg table object.
A new Credential class is added: a boundary type holding a location
prefix plus a config map. Its toHadoopConfig() maps platform-specific
keys to Hadoop-compatible ones.
Credential extraction happens after table loading via
extractCredential(); each credential is then mapped to Hadoop-compatible
configurations.
RESTCatalogProperties is extended with credential-vending-based property
handling, and the property translation mechanism is extracted into a
separate class to make it reusable for cloud-provider-specific config
key mapping.
Tests:
- ConfigTranslatorTests added
- CredentialTest added
Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Assisted-by: Claude Opus 4.8 (opencode)
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/RESTCatalogProperties.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslator.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
M fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
A fe/src/main/java/org/apache/impala/common/Credential.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java
M fe/src/test/java/org/apache/impala/catalog/iceberg/TestRESTCatalogProperties.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java
13 files changed, 1,147 insertions(+), 195 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/24755/6
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 6: Code-Review+2
(1 comment)
Carrying Zoltan's +2
http://gerrit.cloudera.org:8080/#/c/24755/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/24755/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@910
PS5, Line 910: Credentials must not be serialized on the FULL path: CO
> nit: The comment doesn't add any additional information on top of the sourc
Done
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Sep 2026 13:08:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23814/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 13:13:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
IMPALA-15144: Fetch and store credentials from REST catalog
Extract vended storage credentials from an Iceberg REST catalog's
LoadTableResponse and store them in the Iceberg table object.
A new Credential class is added: a boundary type holding a location
prefix plus a config map. Its toHadoopConfig() maps platform-specific
keys to Hadoop-compatible ones.
Credential extraction happens after table loading via
extractCredential(); each credential is then mapped to Hadoop-compatible
configurations.
VendedCredentialsFileIO is added as a wrapper FileIO because the
underlying Iceberg-supplied HadoopFileIO caches the filesystem and
stores the credential per filesystem, which can be a problem on
multi-cloud tables or tables with files that have different ACL rules.
RESTCatalogProperties is extended with credential-vending-based property
handling, and the property translation mechanism is extracted into a
separate class to make it reusable for cloud-provider-specific config
key mapping.
Tests:
- ConfigTranslatorTests added
- CredentialTest added
Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Assisted-by: Claude Opus 4.8 (opencode)
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/RESTCatalogProperties.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslator.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
M fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
A fe/src/main/java/org/apache/impala/common/Credential.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java
M fe/src/test/java/org/apache/impala/catalog/iceberg/TestRESTCatalogProperties.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java
14 files changed, 1,353 insertions(+), 195 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/24755/3
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 1:
(3 comments)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/24755/1/fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java
File fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java:
http://gerrit.cloudera.org:8080/#/c/24755/1/fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java@89
PS1, Line 89: assertEquals("none", translate(rule, ImmutableMap.of("session", "none")).get("session"));
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/24755/1/fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java@124
PS1, Line 124: Map<String, String> out = translate(rule, ImmutableMap.of("adls.sas-token.acct1", "TOKEN"));
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/24755/1/fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java@171
PS1, Line 171: new TranslationRule.RenameRule("uri"), ImmutableMap.of("uri", "x", "unknown", "y"));
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 12:48:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23885/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Sep 2026 13:00:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Sep 2026 17:41:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/13975/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Sep 2026 13:09:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23886/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Sep 2026 13:32:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
IMPALA-15144: Fetch and store credentials from REST catalog
Extract vended storage credentials from an Iceberg REST catalog's
LoadTableResponse and store them in the Iceberg table object.
A new Credential class is added: a boundary type holding a location
prefix plus a config map. Its toHadoopConfig() maps platform-specific
keys to Hadoop-compatible ones.
Credential extraction happens after table loading via
extractCredential(); each credential is then mapped to Hadoop-compatible
configurations.
RESTCatalogProperties is extended with credential-vending-based property
handling, and the property translation mechanism is extracted into a
separate class to make it reusable for cloud-provider-specific config
key mapping.
Tests:
- ConfigTranslatorTests added
- CredentialTest added
Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Assisted-by: Claude Opus 4.8 (opencode)
Reviewed-on: http://gerrit.cloudera.org:8080/24755
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/RESTCatalogProperties.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslator.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
M fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
A fe/src/main/java/org/apache/impala/common/Credential.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java
M fe/src/test/java/org/apache/impala/catalog/iceberg/TestRESTCatalogProperties.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java
13 files changed, 1,147 insertions(+), 195 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 8
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
IMPALA-15144: Fetch and store credentials from REST catalog
Extract vended storage credentials from an Iceberg REST catalog's
LoadTableResponse and store them in the Iceberg table object.
A new Credential class is added: a boundary type holding a location
prefix plus a config map. Its toHadoopConfig() maps platform-specific
keys to Hadoop-compatible ones.
Credential extraction happens after table loading via
extractCredential(); each credential is then mapped to Hadoop-compatible
configurations.
VendedCredentialsFileIO is added as a wrapper FileIO because the
underlying Iceberg-supplied HadoopFileIO caches the filesystem and
stores the credential per filesystem, which can be a problem on
multi-cloud tables or tables with files that have different ACL rules.
RESTCatalogProperties is extended with credential-vending-based property
handling, and the property translation mechanism is extracted into a
separate class to make it reusable for cloud-provider-specific config
key mapping.
Tests:
- ConfigTranslatorTests added
- CredentialTest added
Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Assisted-by: Claude Opus 4.8 (opencode)
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/RESTCatalogProperties.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslator.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
A fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
M fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
A fe/src/main/java/org/apache/impala/common/Credential.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/CredentialTest.java
M fe/src/test/java/org/apache/impala/catalog/iceberg/TestRESTCatalogProperties.java
A fe/src/test/java/org/apache/impala/catalog/iceberg/translate/ConfigTranslatorTest.java
14 files changed, 1,245 insertions(+), 195 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/24755/2
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23831/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Aug 2026 16:07:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23813/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 13:11:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 7
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Sep 2026 13:09:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@910
PS2, Line 910: if (type == ThriftObjectType.DESCRIPTOR_ONLY) {
> It's AI slop, I removed it.
The condition had a reason, even if it wasn't written down: DESCRIPTOR_ONLY is what reaches the executors, while the FULL form of a LocalIcebergTable has exactly one consumer - FeCatalogUtils.feTableToThrift() (FeCatalogUtils.java:431, its Iceberg branch at 463), reached only from CopyTestCaseStmt.getTestCaseData(). With the condition gone, COPY TESTCASE TO '<dir>' over a REST-backed table writes fs.s3a.access.key, fs.s3a.secret.key and the session token into the testcase file - the file people attach to bug reports. It needs VIEW_METADATA on the table plus ALL on the target URI (CopyTestCaseStmt.java:106 and 122-129), so it also turns metadata access into storage access.
Bringing the condition back with a comment naming the reason would cover it; so would filtering the credentials out in feTableToThrift(), if you'd rather keep getTIcebergTable() uniform.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@49
PS2, Line 49: public class VendedCredentialsFileIO extends HadoopFileIO
> It's only used during scanning, no table operation will ever use this IO, i
That holds for everything reaching this FileIO today, but it stops holding inside the window this patch lands in. 24682 adds a coordinator-side INSERT finalizer: Frontend.finalizeIcebergDml() takes the table from the same IcebergMetaProvider, IcebergUtil.getIcebergTransaction() calls newTransaction() on it, and the Append goes through SnapshotProducer, which writes the manifest (newManifestOutputFile) and the manifest list (manifestListPath) via ops.io().newOutputFile(). In 1.10.2 that ends in HadoopFileIO.newOutputFile -> HadoopOutputFile.fromPath(path, getConf()) - the shared FileSystem cache, not the vended credential. deleteFile/listPrefix/deletePrefix/deleteFiles resolve through getConf() as well; those are out of reach for a REST table today, since IcebergRESTCatalog only exists under IcebergMetaProvider and catalogd has no REST catalog type, but the INSERT path isn't.
fileSystemFor() already returns the right FileSystem and HadoopOutputFile.fromPath(Path, FileSystem) exists, so newOutputFile is a couple of lines; the delete/list side is more work. The other route is to let the mutating methods throw here and pick them up in a follow-up, so a write can't quietly go out under ambient credentials. Both work for me - the case worth not leaving open is a manifest written by the wrong identity, since that surfaces from inside a commit. Worth noting that vending and REST INSERT haven't met in a test yet either.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@57
PS2, Line 57: private static final Cache<String, FileSystem> FS_CACHE =
> I removed the removalListener
That settles the use-after-close. What's left is that nothing closes them at all now: newFileSystem() uses FileSystem.newInstance(), so every entry owns its instance, and an entry dropped by maximumSize=100 or by the ten-minute idle expiry is orphaned - for S3A that's an S3 client and its thread pools alive until GC. Is that the intended trade? Closing on eviction after a grace period, or refcounting the outstanding InputFiles, would avoid it; a line in the class comment saying the instances are deliberately never closed would be enough for me too.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@135
PS2, Line 135: private Credential findCredential(String location) {
> Done
I think this one slipped through: PS3 moved the longest-prefix search into a presorted list, but the filter is still location.startsWith(cred.getPrefix()) (line 159 in PS3), so a credential vended for s3://bucket/tab keeps covering s3://bucket/table2/... . Requiring the next character to be '/' - or the prefix to end with one - would settle it.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/CredentialScheme.java@37
PS2, Line 37: public class CredentialScheme {
> It's used in the Credential ctor, I added translation-related tests
The new CredentialTest cases cover what I was after on the translation side: both spellings, the provider only when a session token is present, dropped keys, expiry parsing, identity. Two small ones are still open if you want them - s3n is registered but never exercised, and schemeOf() lowercases the scheme while no test passes an uppercase prefix.
The wider gap is one level up: VendedCredentialsFileIO has no test at all, so the part that picks the credential - longest prefix wins, a credential translating to nothing is skipped, no match falls back to super - isn't pinned anywhere. findCredential() is private; making it package-private would make that testable without a real FileSystem.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Aug 2026 20:37:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 3:
(2 comments)
Went through PS3: the FileIO, the translate layer, the metaprovider wiring and the new tests. Checked the Iceberg side against iceberg-core 1.10.2 rather than from memory.
The first thing I'd look at is the DESCRIPTOR_ONLY condition that came out of getTIcebergTable(), since the FULL form is what COPY TESTCASE serializes into a file and the vended keys now travel with it - details in the thread on FeIcebergTable. Beyond that: the write half of the FileIO still resolves through getConf(), which meets the REST INSERT path landing in 24682; the prefix match still has no path boundary; and nothing closes the cached FileSystems now that the removal listener is gone. The rest are nits.
http://gerrit.cloudera.org:8080/#/c/24755/3/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java:
http://gerrit.cloudera.org:8080/#/c/24755/3/fe/src/main/java/org/apache/impala/catalog/iceberg/translate/TranslationRule.java@196
PS3, Line 196: class PrefixRewriteRule implements TranslationRule {
With the GCS and ADLS schemes gone, PrefixRewriteRule is only reached from ConfigTranslatorTest. Keep it with a line saying it's the hook for the ADLS per-account keys, or drop it until that scheme returns?
http://gerrit.cloudera.org:8080/#/c/24755/3/fe/src/main/java/org/apache/impala/common/Credential.java
File fe/src/main/java/org/apache/impala/common/Credential.java:
http://gerrit.cloudera.org:8080/#/c/24755/3/fe/src/main/java/org/apache/impala/common/Credential.java@126
PS3, Line 126: identity_ = Integer.toHexString(toHadoopConfig().hashCode());
identity() is a 32-bit map hashCode, and it's what separates two credentials for the same bucket in FS_CACHE - a collision hands back a FileSystem authenticated as someone else, without a trace. Hashing.sha256() over the sorted key=value pairs costs the same at these sizes.
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Aug 2026 20:37:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15144: Fetch and store credentials from REST catalog
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@910
PS2, Line 910: if (type == ThriftObjectType.DESCRIPTOR_ONLY) {
> Wow, your AI gave the same output as my AI gave me before pushing PS3, but
Two facts and then it's yours and Zoltan's call. The file is documented as the way to hand metadata to a developer - impala_copy_testcase.xml: "export table or view metadata involved in a query, allowing developers to replay the metadata in a local cluster for testing and debugging purposes". And vended credentials aren't always short-lived: this patch supports static keys with no session token, where getExpiryMs() returns 0 and nothing expires - testToHadoopConfigWithoutSessionTokenKeepsDefaultProvider is that shape. I read the path rather than ran the export against a REST table, so weigh it as such.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java:
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@49
PS2, Line 49: public class VendedCredentialsFileIO extends HadoopFileIO
> Let's handle REST catalog writing support and this patch separately, I woul
Fair enough, scoping it that way is your call. Ordering may go the other way round though: 24735 is in its submit run and 24682 is right behind it, so the adjustment would land on this side.
http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@57
PS2, Line 57: private static final Cache<String, FileSystem> FS_CACHE =
> Yes, it's intended, this is the Hadoop-like behavior, and I think the revie
One correction to what I wrote: "alive until GC" was wrong. newInstance() goes through Cache.getUnique() -> getInternal(), which puts the instance into the static FileSystem.CACHE map, and only close() takes it out again (hadoop-common 3.3.6 and 3.1.1). So an evicted entry is unreachable from here and still pinned for the process lifetime, S3Client and the two S3AFileSystem pools included - Hadoop's own cache never evicts, so nothing accumulates there. The rate follows loads, not tenants: a LocalCatalog per statement and no cache in IcebergMetaProvider means a REST loadTable per statement, and a rotating session token makes that a new identity and a new instance each time. Still the trade you want?
--
To view, visit http://gerrit.cloudera.org:8080/24755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe
Gerrit-Change-Number: 24755
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Nandor Kollar <ko...@gmail.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Aug 2026 22:04:42 +0000
Gerrit-HasComments: Yes