Skip to content

fix: support env-var auth config for REST catalog (JSON string decode + flat properties)#3423

Open
GayathriSrividya wants to merge 2 commits into
apache:mainfrom
GayathriSrividya:fix/issue-3422-rest-auth-env-var-string-decode
Open

fix: support env-var auth config for REST catalog (JSON string decode + flat properties)#3423
GayathriSrividya wants to merge 2 commits into
apache:mainfrom
GayathriSrividya:fix/issue-3422-rest-auth-env-var-string-decode

Conversation

@GayathriSrividya
Copy link
Copy Markdown

@GayathriSrividya GayathriSrividya commented May 28, 2026

Summary

Fixes #3422.

This PR implements the fix designed by @kevinjqliu in the issue comment.

RestCatalog._create_session() expected auth to be a dict, but values loaded from environment variables always arrive as strings. This caused auth initialization to fail silently or with a confusing AttributeError for all pluggable auth types (basic, oauth2, google, entra, custom) when configured via PYICEBERG_CATALOG__<NAME>__AUTH.

Fixes

Implementation follows the tolerant auth parsing order suggested by @kevinjqliu:

1. JSON string decode (primary fix)

If the auth property value is a str, JSON-parse it before processing.
This unblocks:

export PYICEBERG_CATALOG__REST__AUTH='{"type":"oauth2", "oauth2":{"client_id":"id","client_secret":"secret","token_url":"https://auth.example/token"}}'

2. Flat env-var property support (alternative fix)

Supports the canonical PyIceberg env-var style — no JSON blob required, no quoting/escaping 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://auth.example/token

The env-var parser maps these to flat properties (auth.type, auth.oauth2.client-id, etc.). _create_session now checks for auth.type when the auth dict is absent, builds the config dict from the flat auth.* properties, and converts kebab-case keys to snake_case to match AuthManager constructor parameters.

Tests

Five new regression tests added to tests/catalog/test_rest.py (covering the test cases specified by @kevinjqliu):

Test What it covers
test_rest_catalog_with_basic_auth_as_json_string JSON string → Basic auth
test_rest_catalog_with_oauth2_auth_as_json_string JSON string → OAuth2 auth
test_rest_catalog_with_invalid_json_auth_string Invalid JSON → descriptive ValueError
test_rest_catalog_with_basic_auth_flat_properties Flat auth.* props → Basic auth
test_rest_catalog_with_oauth2_auth_flat_properties Flat auth.* props → OAuth2 auth

All 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 on main.

Copy link
Copy Markdown

@abnobdoss abnobdoss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, just a few suggestions!

Comment thread pyiceberg/catalog/rest/__init__.py
Comment thread pyiceberg/catalog/rest/__init__.py
Comment thread pyiceberg/catalog/rest/__init__.py Outdated
Comment thread tests/catalog/test_rest.py Outdated
@rambleraptor
Copy link
Copy Markdown
Contributor

rambleraptor commented May 29, 2026

Thanks for writing this up! I've run into similar issues before and I think this will be a huge help!

Can you run make lint and re-push?

@GayathriSrividya GayathriSrividya force-pushed the fix/issue-3422-rest-auth-env-var-string-decode branch from 21c2a2a to 927017f Compare May 30, 2026 08:37
@GayathriSrividya
Copy link
Copy Markdown
Author

Thanks! I ran make lint locally and re-pushed the changes. CI is green now — please take another look when you have a chance.

Copy link
Copy Markdown

@abnobdoss abnobdoss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: REST catalog auth cannot be configured via environment variables unless auth JSON strings are decoded

3 participants