11 apache polaris planning and implementation - #23
Open
mgdenno wants to merge 64 commits into
Open
Conversation
…erhub token encryption; fixing catalog access
…nning-and-implementation
…nning-and-implementation
Polaris's /v1/config endpoint echoes the catalog's entire raw properties map back to every REST client, unconditionally, regardless of the client's requested X-Iceberg-Access-Delegation mode. Setting s3.remote-signing-enabled=true here made every client's Iceberg S3FileIO try to use remote signing, which Polaris doesn't implement (no /v1/aws/s3/sign route) — breaking clients that correctly ask for vended-credentials instead. Drop the property; ensure_catalog() will PUT the corrected properties onto the existing catalog on next run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The jupyter SA's IAM role trust was recently narrowed to exclude it, breaking Spark session creation. It was never load-bearing for real data access: Iceberg reads go through the Polaris broker/AuthManager (pure OAuth, no AWS creds), and icechunk/gridded reads go through xpublish-api's own scoped role. Removing the annotation lets the credential fallback in spark_session_utils.py degrade to anonymous instead of throwing AssumeRoleWithWebIdentity AccessDenied. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Nothing consumes its credentials anywhere in teehr-cloud-core, teehr-fved, or teehr — real Spark→Polaris auth goes through the AuthManager/broker per-user delegated path instead. It was planned for a headless Prefect batch client-credentials flow but never wired up, and its lack of a realm role assignment (unlike trino-polaris and prefect-polaris) would have made it a live footgun if anything ever did start using it. Removing the client, its secret, and doc references rather than fixing the missing role. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Stop BROKER_SESSION_SIGNING_SECRET and BROKER_REFRESH_TOKEN_ENCRYPTION_SECRET from collapsing to the same value as BROKER_OAUTH_CLIENT_SECRET (and each other). New dedicated broker-secrets K8s secret; config.py no longer cascades defaults between the three. - exchange_token_for_polaris_via_broker_session now verifies the request's user_id/session_id/realm against the stored session record and rejects mismatches, per the binding requirement in polaris-broker-api-contract.md. - /auth/polaris-token/session is no longer exempt from rate limiting — added InMemoryRateLimiter.check_by_key() for paths with no resolved AuthIdentity, keyed by client IP. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
spark-authmanager-enforcement (and spark-permission-enforcement) only exercise driver-only local mode via create_minio_spark_session(), so neither ever tested whether Spark executors actually get the Polaris auth env vars AuthManager needs, or can participate in a real distributed Iceberg write/read. That path was previously only checked manually via test_executor_auth.py (run by hand in Jupyter, not part of the automated suite). Adds spark_authmanager_executor_test.py, adapted from that manual script but with proper PASS/FAIL assertions and exit codes matching the other Garden test scripts' convention, and without the list_tables()/configurations table reads the manual version had -- those depend on warehouse seed data that may or may not exist when this runs as an isolated automated test; this one only touches a table it creates and drops itself. Uses create_spark_session() directly rather than create_minio_spark_session(), since the MinIO endpoint/path-style env vars are already set on this test container (matching how the real jupyter/spark pods pick them up) and go through the catalog-level S3 config path, not the Hadoop-level one the MinIO wrapper additionally sets (which Iceberg's S3FileIO doesn't use anyway). Not yet confirmed whether the test container's service account can actually create executor pods (RBAC for that is granted to the `spark` ServiceAccount; unclear if/how the test container's default SA gets equivalent permission) -- first run against the KinD cluster will tell us. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
create_spark_session(start_spark_cluster=True) reads os.environ["TEEHR_SPARK_IMAGE"] with no fallback when executor_image isn't passed explicitly. jupyterhub's garden.yaml sets this from build.teehr-spark-executor-image's output; the new Test action didn't, since it's a different container than the jupyter pod. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ault - Garden's type: container action schema has no serviceAccountName field (confirmed via its own "key not allowed" validation error, which lists every supported key). Switched to type: kubernetes-pod with a full podSpec, which does support it -- confirmed against a real KinD cluster that executor pods only get created successfully once this test container runs as the jupyter SA (the default SA in this namespace has no RBAC to create/list/watch pods, services, or PVCs; that's scoped to the jupyter/spark SAs specifically). - create_spark_session() (used here instead of create_minio_spark_session()) defaults remote_warehouse_dir to "" when REMOTE_WAREHOUSE_IDENTIFIER isn't set, unlike the minio wrapper which defaults it to the realm name -- Polaris's REST catalog rejects CREATE TABLE with "Please specify a warehouse" when it's empty. Set REMOTE_WAREHOUSE_IDENTIFIER=teehr explicitly to match. Confirmed passing end to end against a real KinD cluster: executor pods created, auth env vars (POLARIS_DEFAULT_REALM, POLARIS_BROKER_SESSION_TOKEN) present on all partitions, distributed write/read/drop of a 200k-row Iceberg table succeeded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ession create_minio_spark_session() defaulted remote_warehouse_dir to the realm name when REMOTE_WAREHOUSE_IDENTIFIER wasn't set; plain create_spark_session() defaults it to "" instead, which Polaris's REST catalog rejects with "Malformed request: Please specify a warehouse" on any catalog read/write. Both test scripts already set POLARIS_DEFAULT_REALM=teehr but not REMOTE_WAREHOUSE_IDENTIFIER, which only surfaced once they stopped going through the minio wrapper. Confirmed passing against a real KinD cluster: spark-permission-enforcement (admin/poweruser/user) and spark-authmanager-enforcement (admin/poweruser/user) both PASSED end to end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s-control doc - /auth/polaris-session's OpenAPI schema advertised ApiKeyAuth like the generic default, but the route requires identity.auth_type == "jwt" same as /auth/polaris-token, which already has this override. Added the matching security override. - docs/polaris-broker-api-contract.md documented a flat error payload shape; FastAPI actually wraps every HTTPException(detail=...) under a top-level "detail" key with no unwrapping handler registered. Updated the doc to match reality rather than changing response behavior (unknown whether any real consumer depends on the documented flat shape). - polaris-sync-principals-script.yaml's sync_principals.py had two redundant paths granting principal roles: one from the user's effective realm roles (matching Polaris's own regex mapper input exactly), and a second, less complete one hardcoded against raw Keycloak group names/paths -- including a hardcoded plural/singular special case (iceberg-catalog-admins group -> iceberg-catalog-admin role) that only worked by coincidence for the groups that exist today. Removed the group-based path entirely; the realm-role-based path already covers everything correctly (composite roles included) since it mirrors exactly what Polaris's regex reads from the JWT. - docs/polaris-access-control.md described Polaris reading a `groups` claim and matching plural/path-prefixed group names directly. The actual mechanism (polaris/manifests/polaris-config.yaml.tpl) reads realm_access.roles, populated via each Keycloak group's composite realmRoles mapping (keycloak-bootstrap/manifests/realm-configmap.yaml.tpl) -- a two-step chain the doc didn't mention. Rewrote to describe both steps accurately, including the plural-group/singular-role naming mismatch for iceberg-catalog-admins. Verified against a real KinD cluster: /openapi.json confirms /auth/polaris-session no longer advertises ApiKeyAuth; re-ran spark-permission-enforcement after redeploying the updated sync_principals.py -- still PASSED for all 3 users (admin/poweruser/user), confirming the realm-role-only path grants identical permissions to what the removed group-based path did. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mgdenno
marked this pull request as ready for review
September 1, 2026 21:47
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
2 participants
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:rti-international/aws-project-amzifved-infrastructure#72(per-service IRSA roles),RTIInternational/teehr-fved#75(deployment wiring), andRTIInternational/teehr#810(Spark session auth).Summary
Introduce Apache Polaris as the Iceberg REST catalog, replacing
iceberg-rest/iceberg-pg, with Keycloak-driven access control and a broker-mediated auth flow for long-lived Jupyter Spark sessions.realm_access.rolesJWT claim → Polaris principal-role regex mapper (group-level, no sync needed);polaris-sync-principals-scriptfor individual/table-level grants layered on top.TeehrBrokerAuthManager(Spark-side) + newteehr-apibroker endpoints (/auth/polaris-session,/auth/polaris-token,/auth/polaris-token/session) mint/rotate short-lived Polaris tokens from an encrypted, Postgres-backed delegated session store, without exposing refresh tokens to notebooks.polaris,polaris-bootstrap,polaris-pgmanifests; Trino/Spark/JupyterHub wiring updated for the new catalog;iceberg-rest/iceberg-pgremoved.spark-authmanager-executor-enforcement).polaris-access-control.md,polaris-broker-api-contract.md,polaris-migration-plan.md) to match actual implementation.