fix: support env-var auth config for REST catalog (JSON string decode + flat properties)#3423
Open
GayathriSrividya wants to merge 2 commits into
Conversation
abnobdoss
reviewed
May 29, 2026
abnobdoss
left a comment
There was a problem hiding this comment.
This looks good, just a few suggestions!
Contributor
|
Thanks for writing this up! I've run into similar issues before and I think this will be a huge help! Can you run |
21c2a2a to
927017f
Compare
Author
|
Thanks! I ran |
abnobdoss
reviewed
Jun 1, 2026
abnobdoss
left a comment
There was a problem hiding this comment.
Thanks for the updates! I took another look and may be missing it, but I still only see the basic/simple OAuth2 test cases covered for flat auth.* env vars.
Could we also cover the typed flat-auth cases, like OAuth2 numeric fields and Google/Entra scopes?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3422.
This PR implements the fix designed by @kevinjqliu in the issue comment.
RestCatalog._create_session()expectedauthto be adict, but values loaded from environment variables always arrive as strings. This caused auth initialization to fail silently or with a confusingAttributeErrorfor all pluggable auth types (basic,oauth2,google,entra,custom) when configured viaPYICEBERG_CATALOG__<NAME>__AUTH.Fixes
Implementation follows the tolerant auth parsing order suggested by @kevinjqliu:
1. JSON string decode (primary fix)
If the
authproperty value is astr, JSON-parse it before processing.This unblocks:
2. Flat env-var property support (alternative fix)
Supports the canonical PyIceberg env-var style — no JSON blob required, no quoting/escaping issues:
The env-var parser maps these to flat properties (
auth.type,auth.oauth2.client-id, etc.)._create_sessionnow checks forauth.typewhen theauthdict is absent, builds the config dict from the flatauth.*properties, and converts kebab-case keys to snake_case to matchAuthManagerconstructor parameters.Tests
Five new regression tests added to
tests/catalog/test_rest.py(covering the test cases specified by @kevinjqliu):test_rest_catalog_with_basic_auth_as_json_stringtest_rest_catalog_with_oauth2_auth_as_json_stringtest_rest_catalog_with_invalid_json_auth_stringValueErrortest_rest_catalog_with_basic_auth_flat_propertiesauth.*props → Basic authtest_rest_catalog_with_oauth2_auth_flat_propertiesauth.*props → OAuth2 authAll 13
test_rest_catalog_with_*tests pass; the 4 pre-existing failures (test_token_200,test_config_200,test_auth_header, etc.) are unrelated to this change and also fail onmain.