New Catalog Configuration: Credentials via config file#391
Open
sfc-gh-npuka wants to merge 5 commits into
Open
New Catalog Configuration: Credentials via config file#391sfc-gh-npuka wants to merge 5 commits into
sfc-gh-npuka wants to merge 5 commits into
Conversation
Until now an iceberg_catalog server carried its own client_id /
client_secret on the SERVER row. That gave every role on the
cluster the same credentials and forced operators to recreate the
server when secrets rotated. Move credentials to per-user state
and add a platform-provided file as a middle layer.
Credential resolution (lowest to highest priority):
1. pg_lake_iceberg.rest_catalog_* GUC defaults
2. iceberg_catalog SERVER options (non-secret only)
3. $PGDATA/catalogs.conf (user-created servers only)
4. pg_user_mapping options (user-created servers only)
The built-in pg_lake_rest_catalog (catalog='rest') stops after step 2
on purpose: its credentials live exclusively in the GUCs so the
built-in stays a single, global, instance-wide configuration with no
hidden per-user view. CREATE/ALTER USER MAPPING is rejected against
all three built-in long names; catalogs.conf is also ignored for the
built-in 'rest' catalog.
Notable pieces:
* iceberg_catalog_option_descs[] now carries a CATALOG_OPT_CTX_*
bitmask per option. The same table drives the validator, the
per-context "Valid options are: ..." hint, and the option->struct
applier. client_id / client_secret are USER MAPPING-only; scope is
accepted on both, with the USER MAPPING value winning because it is
applied last during resolution.
* RestCatalogOptions gains a umid field; the token cache key is now
(serverOid, umid) so different SET ROLEs in the same backend each
get their own user mapping's credentials. A USERMAPPINGOID syscache
callback invalidates cached tokens on CREATE/ALTER/DROP USER MAPPING.
* ValidateRestCatalogOptions now performs an early auth-type-aware
credentials check at resolution time:
- client_secret is always required
- client_id is required unless rest_auth_type='horizon'
Missing credentials surface as "no credentials found for REST
catalog ..." with ERRCODE_FDW_OPTION_NAME_NOT_FOUND. The per-field
checks inside FetchRestCatalogAccessToken are kept as defense in
depth and now carry the same errcode.
* New ProcessUtility handler RedactRestCatalogUserMappingSecrets
scrubs client_id / client_secret from queryString in place on
CREATE/ALTER USER MAPPING for any iceberg_catalog server (built-in
long names included). Handles plain '', E'', and U&'' literal
forms with their escape rules. Registered after the DDL validator
so it runs first (the handler list is prepend-LIFO), ensuring the
failing built-in-server path never leaks secrets into the ereport
context. DDL itself reads option values from DefElem->arg, so
pg_user_mapping still stores plaintext credentials -- only the
query string surfaces (pg_stat_statements, log_min_duration_statement,
ereport context) see the redacted form.
* New PGC_SIGHUP GUC pg_lake_iceberg.catalogs_conf_path lets
operators point at an absolute path; the default is the relative
'catalogs.conf' resolved against DataDir.
Tests:
* test_iceberg_catalog_server.py picks up the user-mapping DDL
surface (per-context option lists and hints, valid/invalid options
on each side, FOR CURRENT_USER with all three options,
per-role mappings on the same server, dependency-driven rejections,
built-in long-name blocks), the redaction handler (CREATE/ALTER,
'' escape, E'' escape, scope preservation, non-iceberg FDW skip,
built-in-rejection-after-redaction, plaintext storage preservation),
and credential resolution via catalogs.conf (file-only success,
USER MAPPING wins over the file, no-credentials-anywhere error,
scope from the file, absolute catalogs_conf_path, built-in
ignores the file).
* test_modify_iceberg_rest_table.py: client_id / client_secret are
dropped from the server-option-overrides-GUC parametrization (they
no longer belong on SERVER), and a new parametrized
test_user_mapping_credential_overrides_guc covers the same
resolution-order direction with USER MAPPING in step 4.
* test_writable_iceberg_common.py: the shared fixture for the writable
user-created REST server now puts credentials on a PUBLIC user
mapping, and drops the server with CASCADE to sweep up the mapping.
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
- Rename umid to userMappingOid throughout RestCatalogOptions and the token cache key for clarity. - Make GetValidCatalogOptionsHint context-aware: accept a contextBit parameter and only list options valid for the given context (SERVER vs USER MAPPING), so the error hint is precise. - Improve RedactUserMappingSecrets readability: rename single-letter variables (p -> cursor, qend -> queryEnd), introduce named booleans for escape detection and quoting rules. - Add whitespace to GetValidCatalogOptionsHint loop body. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
Strip the catalogs.conf config-file feature from the REST catalog credential resolution path. This removes: - CatalogsConfPath global and pg_lake_iceberg.catalogs_conf_path GUC - ReadCatalogsConfCredentials() and ApplyCatalogsConfOverrides() - The ApplyCatalogsConfOverrides() call in BuildRestCatalogOptionsFromServer - All $PGDATA/catalogs.conf references in error hints and comments - Six catalogs.conf-specific pytest tests and supporting fixtures - catalogs.conf examples from the upgrade SQL header comment The credential resolution order is now: 1. GUC defaults 2. Server options 3. pg_user_mapping options (user-created servers only) Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
b31f1b7 to
66b4cdd
Compare
Introduce a file-based credential source for REST catalog servers. The resolver reads $PGDATA/catalogs.conf (configurable via the pg_lake_iceberg.catalogs_conf_path PGC_SIGHUP GUC) using ParseConfigFp and extracts per-server client_id, client_secret, and scope via dotted-key syntax (e.g. "horizon.client_id = '...'"). This layer sits between server options and pg_user_mapping in the resolution order, giving platforms a way to inject credentials without requiring per-user DDL: 1. GUC defaults 2. Server options 3. $PGDATA/catalogs.conf credentials (user-created servers only) 4. pg_user_mapping options (user-created servers only) The built-in pg_lake_rest_catalog deliberately skips this layer (and user mappings) to remain a single global configuration. Adds ReadCatalogsConfCredentials(), ApplyCatalogsConfOverrides(), the catalogs_conf_path GUC, six pytest tests covering the new layer, and updates all error hints and comments to reflect the expanded resolution chain. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
66b4cdd to
29cf49e
Compare
- Rename catalogs_conf_path GUC to catalogs_conf_credentials_path and CatalogsConfPath to CatalogsConfCredentialsPath throughout. - Remove scope from the config file layer: ReadCatalogsConfCredentials now only recognizes client_id and client_secret. scope remains available on server options and user mapping options. - Remove test_scope_from_catalogs_conf_user_server (no longer applies). - Rename ApplyCatalogsConfOverrides to ApplyCatalogsConfCredentials - Add missing extern for CatalogsConfCredentialsPath in rest_catalog.h. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
4 tasks
730ff8f to
f301489
Compare
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.
Description
This change adds a file-based credential source for REST catalog servers, sitting between server options and
CREATE USER MAPPINGin the resolution chain. A platform operator can place credentials in$PGDATA/catalogs.confwithout requiring any per-user DDL, making it possible to provision catalog access at the infrastructure level rather than through SQL.This branch builds on top of
naisila/catalog_user_mapping(which moved credentials intoCREATE USER MAPPING).Platform-provided catalog
The operator creates the server via DDL and drops credentials into the config file — no user mapping is needed:
File location
The path is controlled by
pg_lake_iceberg.catalogs_conf_path, aPGC_SIGHUP, superuser-only GUC. It defaults tocatalogs.conf, which is resolved to$PGDATA/catalogs.conf. Absolute paths are used as-is.With this change, the full resolution chain becomes:
CREATE USER MAPPINGpg_user_mappingsyscache lookup for current user, fallback to PUBLIC$PGDATA/catalogs.confParseConfigFp; dotted keys likeserver.client_id = '...'rest_catalog_client_id,rest_catalog_client_secret(set during initialization)User mapping values overwrite whatever
catalogs.confprovided.catalogs.confvalues fill in only what server options didn't provide. GUCs are the implicit base layer from initialization. An error is raised if no credentials are found after all three sources.scopefollows the same layering: user mapping >catalogs.conf> server options > GUC.The built-in
pg_lake_rest_catalog(catalog'rest') skips both thecatalogs.confand user mapping phases — its credentials live exclusively in GUCs.Two new static functions in
rest_catalog.c:ReadCatalogsConfCredentialsreads the credentials file using PostgreSQL'sParseConfigFp. It matches entries where the dotted key prefix equals the server name and recognizes three keys:client_id,client_secret, andscope; unknown keys are silently ignored for forward compatibility. The file is re-read on every call — catalog operations are infrequent, and avoiding mtime tracking or SIGHUP wiring keeps the code simple. A missing file (ENOENT) is silently treated as "no credentials". I/O or permission errors are logged atWARNINGand likewise treated as "no credentials" so one bad config file does not break the entire catalog path.ApplyCatalogsConfOverridescallsReadCatalogsConfCredentialsand overlays any credentials found onto theRestCatalogOptionsstruct. Only fields actually present in the file are overwritten — unset entries leave whatever lower-priority layer (GUCs) populated.ApplyCatalogsConfOverridesis called fromBuildRestCatalogOptionsFromServerbetweenApplyServerOptionOverridesandApplyUserMappingOverrides, guarded by!IsBuiltinCatalogServerName(serverName).