Skip to content

Update spark_session_utils for Polaris auth (STS, AuthManager, executor propagation) - #810

Merged
mgdenno merged 24 commits into
mainfrom
803-update-teehr-for-spark-iceberg-polaris
Sep 2, 2026
Merged

mgdenno merged 24 commits into
mainfrom
803-update-teehr-for-spark-iceberg-polaris

Conversation

@mgdenno

@mgdenno mgdenno commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Part of a larger effort to migrate the catalog stack from iceberg-rest/iceberg-pg to 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), and RTIInternational/teehr-fved#75 (deployment wiring).

Summary

Rework spark_session_utils.py to support Polaris auth for Spark sessions.

  • Add AuthManager and STS/vended-credentials support for Polaris-backed Iceberg catalogs; propagate auth to Spark executors in cluster mode.
  • Fix a namespace-fallback bug in cluster-mode config and remove dead code (manual Ivy jar re-discovery, no-op post-creation config reapply — confirmed unnecessary via live cluster testing).
  • Remove deprecated token-helper functions in favor of the AuthManager/broker flow.
  • Add a 22-test regression suite (tests/evaluations/test_spark_session_auth.py) covering the auth-config-building logic.

mgdenno and others added 23 commits July 30, 2026 11:05
…, 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
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>
@mgdenno mgdenno linked an issue Aug 28, 2026 that may be closed by this pull request
… 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.
@mgdenno
mgdenno requested review from jbkolze and samlamont August 28, 2026 20:05
@mgdenno
mgdenno marked this pull request as ready for review September 1, 2026 21:46
@mgdenno
mgdenno merged commit 5e64704 into main Sep 2, 2026
8 checks passed
@mgdenno
mgdenno deleted the 803-update-teehr-for-spark-iceberg-polaris branch September 11, 2026 14:53
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.

update teehr for spark iceberg polaris

1 participant