fix: support env-var auth config for REST catalog (JSON string decode + flat properties)#3423
Open
GayathriSrividya wants to merge 3 commits into
Conversation
added 3 commits
May 28, 2026 08:49
…auth config Fixes apache#3422. RestCatalog._create_session() expected auth to be a dict, but environment variables always produce string values. This caused auth initialization to fail for all pluggable auth types (basic, oauth2, google, entra, custom) when configured via PYICEBERG_CATALOG__<NAME>__AUTH. Two complementary fixes: 1. JSON string decode: if the 'auth' property is a string, JSON-parse it before processing. This supports: export PYICEBERG_CATALOG__REST__AUTH='{"type":"oauth2",...}' 2. Flat env-var property support: if 'auth' is absent but 'auth.type' is present, build the auth config from flat 'auth.*' properties. This is the canonical env-var style that avoids JSON-in-env quoting issues: export PYICEBERG_CATALOG__REST__AUTH__TYPE=oauth2 export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__CLIENT_ID=id export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__CLIENT_SECRET=secret export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__TOKEN_URL=https://... Kebab-case keys (e.g. 'client-id') produced by the env-var parser are normalised to snake_case ('client_id') to match AuthManager constructor parameters. Regression tests added for both code paths (basic and oauth2) as well as invalid JSON detection.
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.