Skip to content

feat(discovery): support configured db schema#101

Merged
EfeDurmaz16 merged 1 commit into
mainfrom
fides-discovery-db-schema
May 6, 2026
Merged

feat(discovery): support configured db schema#101
EfeDurmaz16 merged 1 commit into
mainfrom
fides-discovery-db-schema

Conversation

@EfeDurmaz16

Copy link
Copy Markdown
Owner

Summary

  • add DISCOVERY_DB_SCHEMA support for discovery service connections and migrations
  • create the configured schema before running discovery migrations
  • set Docker Compose discovery to use a dedicated discovery schema by default
  • document DISCOVERY_DB_SCHEMA and TRUST_GRAPH_DB_SCHEMA deployment settings

Verification

  • pnpm --filter @fides/discovery-service test
  • pnpm --filter @fides/discovery-service lint
  • pnpm build
  • POSTGRES_PASSWORD=fides docker compose config
  • git diff --check

Note: local discovery storage tests skipped because DATABASE_URL is not set locally; CI provides Postgres and will run them.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a69b90aa0e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docker-compose.yml
DATABASE_URL: postgresql://${POSTGRES_USER:-fides}:${POSTGRES_PASSWORD:?Set POSTGRES_PASSWORD in .env}@postgres:5432/${POSTGRES_DB:-fides}
DISCOVERY_PORT: "3100"
DISCOVERY_DB_AUTO_MIGRATE: ${DISCOVERY_DB_AUTO_MIGRATE:-true}
DISCOVERY_DB_SCHEMA: ${DISCOVERY_DB_SCHEMA:-discovery}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep Compose upgrades on the existing discovery schema

For existing Docker Compose deployments with a persisted Postgres volume and no DISCOVERY_DB_SCHEMA in .env, this new default silently moves discovery from the previously used public schema to a fresh discovery schema. On upgrade, runDiscoveryMigrations will create empty tables there, so registered identities/agents appear to be lost until operators discover and override the variable or manually migrate data. Make the schema opt-in for Compose upgrades or provide an explicit migration/compatibility path.

Useful? React with 👍 / 👎.

assertValidSchemaName(schemaName)

const url = new URL(connectionString)
url.searchParams.set('options', `-c search_path=${schemaName},public`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Quote schema names in the search_path

When DISCOVERY_DB_SCHEMA contains uppercase letters, which assertValidSchemaName currently allows, this sends an unquoted search_path=Foo,public; PostgreSQL folds that to lowercase, while ensureConfiguredSchema creates the quoted schema "Foo". In that configuration the new schema is created but ignored, and migrations/queries fall back to public or a lowercase schema. Restrict the value to lowercase identifiers or quote/escape it consistently in the search path.

Useful? React with 👍 / 👎.

@EfeDurmaz16 EfeDurmaz16 merged commit 46ea063 into main May 6, 2026
16 of 18 checks passed
@EfeDurmaz16 EfeDurmaz16 deleted the fides-discovery-db-schema branch May 6, 2026 19:05
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.

2 participants