Update spark_session_utils for Polaris auth (STS, AuthManager, executor propagation) - #810
Merged
Merged
Conversation
…, drop 401 password-fallback, add POLARIS_USE_STS skeleton
…is_token_to_spark, request_broker_polaris_token, ensure_fresh_polaris_token_via_broker; simplify ensure_fresh_polaris_user_token to refresh-only
… docstring to ensure_fresh_polaris_user_token
…session docstring
…spark-iceberg-polaris
s3.remote-signing-enabled=true requires Iceberg's S3 remote-signing client to know where to send sign requests. Without s3.signer.uri set, Spark fails with IllegalArgumentException: S3 signer service URI is required. Point it at the same REST catalog URI already configured for the session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Polaris 1.5.0 rejects the REST catalog's remote-signing delegation mode and has no /v1/aws/s3/sign route at all, so setting s3.signer.uri (the previous fix) can never work — it would 404 on the first signing call even with a correct URI. Switch to vended-credentials instead, the mode Trino already uses successfully against this same Polaris deployment: Polaris performs the S3 warehouse role AssumeRole itself and returns real short-lived AWS credentials in the table's REST config, so Spark's S3FileIO needs no signing round-trip. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- _update_configs_and_packages required add_jars with no default,
breaking test_create_spark_session.py (TypeError). Made add_jars
and add_packages optional, matching their existing None-safe
internal handling.
- _as_bool_str's own default ("true") disagreed with every
POLARIS_USE_AUTHMANAGER/POLARIS_USE_STS caller's os.getenv fallback
("false"), so an env var set to an empty string silently enabled
AuthManager/STS instead of leaving them off. Callers now pass their
intended default explicitly.
- _configure_iceberg_catalogs lost its S3 endpoint/path-style-access
override for the general create_spark_session() path when that
logic moved into create_minio_spark_session() only. Restored it
(opt-in via REMOTE_CATALOG_S3_PATH_STYLE_ACCESS) so non-AWS
S3-compatible endpoints work outside the MinIO wrapper too.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Thread resolved_use_authmanager/resolved_use_sts through to
_build_polaris_auth_configs as explicit params instead of letting
it re-derive them independently from env vars -- the two could
drift.
- Consolidate _update_configs_and_packages's three inline CSV
dedupe-merge blocks (add_jars, add_packages, update_configs'
special-cased keys) to reuse _append_csv_conf, which also fixes a
latent crash: the add_packages branch called
conf.get("spark.jars.packages") unconditionally, raising
AttributeError when that key was never set.
- Factor _build_polaris_auth_packages/_repositories's identical
CSV-env-var parsing into a shared _split_csv_env helper.
- Elevate the implicit "stop the active Spark session" side effect
of AuthManager mode to a warning explaining why, and document it
on the use_authmanager docstring, rather than a silent info log
when force_recreate_session wasn't explicitly requested.
- Elevate _apply_runtime_spark_configs' non-modifiable-key skip from
debug to warning, since it can mean a refreshed auth token/credential
never actually reaches an already-instantiated catalog client.
- Fix RemoteCatalog.warehouse_dir/catalog_type/catalog_uri defaults
binding at teehr.const's module-import time instead of reading the
env var live at instantiation (same class of bug already fixed
elsewhere in this PR for create_spark_session's own env reads).
- Redact token/secret/credential/password-bearing Spark conf keys in
log_session_config's debug_config=True dump instead of printing
them in plaintext.
- Investigated (not fixed, needs live cluster testing):
_collect_resolved_package_jars' Ivy jar-path guessing silently
drops a package with no per-package failure signal.
Adds tests/evaluations/test_spark_session_auth.py covering all of the
above at the pure-function level (no SparkSession/network required).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verified against a real KinD cluster (start_spark_cluster=True, use_authmanager=True): distributed writeTo(...).create() + read.table() across executors succeed identically with or without _collect_resolved_package_jars's manual Ivy jar-path guessing feeding spark.jars. Spark's native spark.jars.packages resolution already handles executor jar distribution now that teehr-iceberg-authmanager is a real Maven coordinate -- the manual re-discovery predated that and is no longer needed. Also removed _apply_runtime_spark_configs and its one call site. Confirmed dead in every case: for a newly-created session, its inputs were already applied to `conf` before SparkSession.builder.config(conf) .getOrCreate() ran, so the values were already set. For a reused existing session, Spark's own builder-level config-reuse logic already re-applies whatever's runtime-modifiable; whatever isn't modifiable can't be pushed in by either path, confirmed empirically -- every key in effective_configs reported non-modifiable once the debug->warning elevation (previous commit) made this visible for the first time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_set_spark_cluster_configuration's namespace resolution checked
`spark_namespace is None` for its file-based and "default" fallbacks,
but the first step (os.environ.get("TEEHR_NAMESPACE", "")) always
assigns a string, never None -- so when TEEHR_NAMESPACE isn't set,
spark_namespace becomes "" and the next two fallbacks never trigger,
silently leaving spark.kubernetes.namespace empty. Surfaced by a new
test running without TEEHR_NAMESPACE set: Spark's k8s client made
unscoped (cluster-wide) pod/service/PVC list-watch calls instead of
namespaced ones, failing with 403s that even a correctly-scoped
ServiceAccount wouldn't have permission for.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… state SparkConf() with loadDefaults=True inherits leftover spark.* system properties from the shared JVM gateway, which earlier tests in the same CI run populate with real Ivy-resolved jar paths after creating a real SparkSession. This broke the exact-list-equality assertion in test_update_configs_and_packages_add_jars_dedupes_and_strips_whitespace in CI, though it doesn't reproduce locally where those earlier tests fail before ever touching the shared JVM. Use loadDefaults=False so these pure config-merging unit tests start from a clean conf.
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.
Part of a larger effort to migrate the catalog stack from
iceberg-rest/iceberg-pgto Apache Polaris with Keycloak-driven access control. Companion PRs:RTIInternational/teehr-cloud-core#23(Polaris/Keycloak/broker implementation),rti-international/aws-project-amzifved-infrastructure#72(per-service IRSA roles), andRTIInternational/teehr-fved#75(deployment wiring).Summary
Rework
spark_session_utils.pyto support Polaris auth for Spark sessions.tests/evaluations/test_spark_session_auth.py) covering the auth-config-building logic.