Fix register_table to properly initialize FileIO and refactor overall FileIO initialization to better reveal bugs

[dennishuo]

Description

The previous default initialization of a catalogFileIO that doesn't take into consideration any kind of subscoping is fundamentally a bug if it ever kicks in for real isolated-server scenarios where "application defaults" aren't appropriate for any storage interactions. Some unittest scenarios relied on this "default" initialization but this masked bugs where methods like registerTable really do need to go through the full subscoping flow, but silently "succeeded" in test cases just because the application default credentials leaked into the test environment.

Refactoring of FileIO initialization in BasePolarisCatalog now makes it more explicit as to when we expect to use defaults or when we require going through subscoping flows.

Conveniently, this also provides a clean way to configure a server to skip all subscoping flows entirely and always allow application defaults and/or table-config inheritance to define the credentials used to interact with storage. (E.g. provides a way to achieve https://github.com/apache/polaris/pull/77/files just by setting SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION=true)

Overview:

• Add SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION configuration option for integration tests that want to use "fake" s3 paths or server deployments that don't need to perform all the assumeRole subscoping and instead just use application defaults
• Refactor to properly inherit tableProperties in FileIO initialization and remove redundant callsites, ensure consistent usage of refreshIOWithCredentials
• No longer initialize catalogFileIO by default; only set it in BasePolarisCatalogTest where client/server behaviors are mixed
• Override registerTable to correctly use the subscoping flow to initialize a FileIO

Fixes https://github.com/apache/polaris/issues/156 and https://github.com/apache/polaris/pull/77

Type of change

Please delete options that are not relevant.

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• [ ] Test A
• [ ] Test B

Test Configuration:

• Firmware version:
• Hardware:
• Toolchain:
• SDK:

Checklist:

Please delete options that are not relevant.

• [x] I have performed a self-review of my code
• [x] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [x] My changes generate no new warnings
• [x] I have added tests that prove my fix is effective or that my feature works
• [x] New and existing unit tests pass locally with my changes
• [ ] Any dependent changes have been merged and published in downstream modules
• [x] If adding new functionality, I have discussed my implementation with the community using the linked GitHub issue

[dennishuo]

@cgpoh if you have time, could you please verify whether SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION: true satisfies your use case as an alternative to https://github.com/apache/polaris/pull/77 ?

@collado-mike you might be interested in validating whether my changes to dropTable(purge) make sense; as far as I can tell since TaskFileIOSupplier is what ultimately provides purge credentials, the inline credential-creation was extraneous

[cgpoh]

@dennishuo , thanks! I will try to verify when it's merge

[cgpoh]

Hi @dennishuo, I just tested and the fix doesn't satisfy my use case. For my use case, the Spark job should not provide Azure credentials as environment variables for it to execute properly. I don't want our Azure credentials to be exposed through Spark Job and hence, I prefer the credential vending mode where Polaris Catalog is the only place that knows about Azure credentials and we can apply RBAC policy to Spark job via spark.sql.catalog.catalog_name.credential

[dennishuo]

@cgpoh Thanks for the extra info! To confirm, you still want Polaris to provide Spark with the ability to access Azure storage, but can't use SAS_TOKEN, right? Did you want Polaris to return something like a "shared key" to Spark instead? Were you also going to add this proposed new credential-vending approach to https://github.com/apache/polaris/pull/77 ?

[cgpoh]

@dennishuo , yes. My company disable the permission for us to use SAS_TOKEN and thus we need to fallback to use environment variables for ADLS. Sorry, what is the proposed new credential vending approach?