Add IcebergTableCredentials back to IcebergSplit

[JohnFitzpatrick44]

Description

Previously, fileIoProperties were removed from IcebergSplit as part of the response to #28537.

Passing credentials through the split is not inherently risky, however. We currently do not surface any split info that isn't explicitly selected through APIs like ConnectorSplit#getSplitInfo (here) or SplitCompletedEvent (here).

Now that credentials are wrapped in a very clearly named IcebergTableCredentials object, it should be fine to pass this through the split. We may have been overzealous with what was removed. It may still be useful to be able to handle vended credentials from splits, rather than only at the table level.

I considered an alternative approach where we do dynamically update credentials using a similar and simplified version of how we handle dynamic filters. This proved to be quite complicated and invasive, however, and I would like to limit the amount of changes outside of the Iceberg connector.

This approach is much simpler, and shouldn't result in any credential leaks that we were initially responding to from the above issue.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

[findinpath]

[INFO] --- checkstyle:3.6.0:check (checkstyle) @ trino-spi ---
[INFO] There is 1 error reported by Checkstyle 13.4.1 with checkstyle/airbase-checks.xml ruleset.
Error:  src/main/java/io/trino/spi/connector/ConnectorPageSourceProvider.java:[19,8] (imports) UnusedImports: Unused import - java.util.Optional.
[INFO] ------------------------------------------------------------------------

[wendigo]

How this is more secure that the current implementation? What problem does this PR solve?

[findinpath]

How this is more secure that the current implementation? What problem does this PR solve?

This is less about security and more about use cases which require the fileIoProperties in the IcebergSplit.
The splits are deemed, at the time of this writing, secure, so having fileIoProperties packed within the IcebergSplit should not pose concerns.

One such use case which would benefit of having the fileIoProperties in the IcebergSplit is #28389
cc @sopel39

[wendigo]

@findinpath security comes first, not a single feature use case should impose security hole. I prefer worse but secure API

[sopel39]

"Add IcebergTableCredentials back to IcebergSplit"

Does it mean we send potentially same table credential info over and over again with each split? Should it be called IcebergSplitCredentials?

[JohnFitzpatrick44]

@wendigo

security comes first, not a single feature use case should impose security hole. I prefer worse but secure API

Fair point, but this approach is at least as secure as the current state. We no longer indiscriminately dump splits to query json (from PRs here and here), so the only remaining risk is toString, which isn't meant to have sensitive information anyway. cc @raunaqmorarka

As for motivation, being able to vend split-level credentials makes us significantly more flexible moving forward, and this approach moves us in a much better direction than trying to find a workaround to provide creds to tasks after scheduling.

@sopel39

Does it mean we send potentially same table credential info over and over again with each split? Should it be called IcebergSplitCredentials?

Yes, although this was the norm before #28537

Good suggestion for the name change, I'll consider better alternatives. Maybe a more general IcebergCredentials.

The interface is ConnectorTableCredentials, so best leave it as-is.

[findinpath]

Expecting code to raise a throwable.
	at io.trino.plugin.iceberg.catalog.rest.TestIcebergPolarisCatalogConnectorSmokeTest.testDropTableWithMissingDataFile(TestIcebergPolarisCatalogConnectorSmokeTest.java:197)

https://github.com/trinodb/trino/actions/runs/25135388739/job/73672490397?pr=29263

Related failure?

[JohnFitzpatrick44]

@findinpath passes locally, I'll try rerunning tests

[chenjian2664]

Adding credentials back to IcebergSplit would be more of an enhancement for connectors that support split-level specific credentials. Am I understanding that correctly?

[chenjian2664]

Do we need the IcebergTableCredentials in FilesTableSplit?

[chenjian2664]

I'm thinking about connectors that require, or can operate with, table-level credentials, in that case we probably still need this method

[JohnFitzpatrick44]

Closing to reconsider this approach