Promote dbt module out of modules-experimental/ to production-suitable - #594
Open
RonaldHensbergen wants to merge 3 commits into
Open
Promote dbt module out of modules-experimental/ to production-suitable#594RonaldHensbergen wants to merge 3 commits into
RonaldHensbergen wants to merge 3 commits into
Conversation
The dbt module's productionSuitable: false flag reflected maturity gaps (no README, no demo profile wiring, no bespoke tests) rather than any inherent runtime limitation — unlike modules/secrets/vault, whose false flag is permanent because it only runs Vault in ephemeral dev mode. dbt-core itself, running against a real Postgres with the module's existing hardened container settings (read-only root, dropped capabilities, non-root user), has no such limitation. - modules-experimental/transformation/dbt/ -> modules/transformation/dbt/: drops productionSuitable: false and the "(experimental)" displayName/ description suffix. - Adds modules/transformation/dbt/README.md (Purpose/Known limitations/ Upstream docs/Configuration notes), matching the structure of other production module READMEs (e.g. modules/warehouse/postgres/README.md). - Adds profiles/local-dagster-postgres-superset-dbt/: the reference local-dagster-postgres-superset stack plus dbt wired to postgres via the sql-database contract, following the same "extend via a new profile" pattern used for local-dagster-postgres-superset-vault (rather than editing the base profile's composition). - Adds tests/test_dbt_hardening.py (9 tests) covering image hardening (digest pin, non-root user, pip uninstalled in both build stages), compose service hardening (read_only, cap_drop, tmpfs paths, one-shot restart policy, docs sidecar's service_completed_successfully dependency), the required targetDatabase.contractRef schema field, and the entrypoint's required connection env vars — closing the "zero bespoke test files" gap the module had as an experimental module. - Updates docs/architecture.md (Transformation layer row: dbt instead of "(planned)") and docs/roadmap.md (dbt module + new profile moved from the module list into Stable Components). - images/dbt/README.md: updated module path references and dropped the "It is experimental" note (the image build/publish/scan/sign workflow wiring was already identical to the production dagster/superset images). Testing: - make check (623 tests, ruff, yamllint, markdownlint) passes. - cds validate / cds test (plan+render stages) / cds render pass for the new local-dagster-postgres-superset-dbt profile; manually confirmed the rendered dbt-run service correctly resolves DBT_HOST/DBT_PORT/ DBT_DBNAME/DBT_USER/DBT_PASSWORD from the postgres.sql-database contract binding. - tests.test_module_isolation passes for the relocated module (no cross-module service-name hardcoding). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RonaldHensbergen
added
area:configuration
Reported when running the local-dagster-postgres-superset-dbt profile: dbt-run-1 | cp: cannot stat '/app/images/dbt/profiles.yml': Permission denied Root cause: images/dbt/Dockerfile's COPY --chmod=0444 images/dbt/profiles.yml /app/images/dbt/profiles.yml was the first COPY to reference the /app/images/dbt directory, so BuildKit created that directory tree itself to satisfy the destination path - and applied the requested --chmod (0444, no execute bit) to the newly created directories too, not just the file. That leaves the non-root dbt user unable to traverse into /app/images/dbt at all, so `cp` fails at stat() before it even gets to read permissions on the file itself. Reproduced in isolation with a minimal Dockerfile (a plain COPY --chmod into a not-yet-existing nested directory) and confirmed the identical "Permission denied" traversal failure; images/dagster/base/Dockerfile avoids this by design - it has an earlier plain COPY (without --chmod) that creates /app/images/dagster/ first, so its own `COPY --chmod=0444 .../workspace.yaml` only touches the file. Fix: copy profiles.yml to a flat path directly under /app (/app/profiles.yml.template) instead of a new nested directory, so no directory creation is implicated by --chmod. /app already exists (via WORKDIR /app) with normal permissions, so only the file's mode is affected. Updated entrypoint.sh's `cp` source path to match. Testing: - make check (623 tests, ruff, yamllint, markdownlint) passes. - docker build -f images/dbt/Dockerfile . succeeds locally; manually verified as the non-root uid 999 user that /app/profiles.yml.template is now readable and copyable (previously reproduced the exact "Permission denied" failure with the old nested path in an isolated minimal Dockerfile). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The dbt-target named volume is shared between dbt-run (writes dbt artifacts) and the dbt-docs nginx sidecar (serves them read-only). Docker populates a first-referenced named volume from whichever image has content at the mount path. Mounting the shared volume at nginx's stock docroot (/usr/share/nginx/html) risked seeding the volume with nginx's own root-owned default index.html/50x.html, permanently blocking dbt-run's non-root (uid 999) writes if the nginx container was ever created/started before dbt-run's. Move the mount target to /usr/share/nginx/dbt-docs, a path with no default content in the nginx:alpine image, so the volume always populates empty regardless of container creation order. Updated nginx.conf's root directive to match, and added a regression test asserting the shared mount never targets nginx's stock docroot. Verified with a real docker build + docker create/start reproduction: the new mount path populates empty (no leftover nginx defaults) even when the nginx container is created first, and a uid 999 write to the shared volume is correctly visible from the nginx container.
7 tasks
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
1 participant
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.
Summary
The dbt module's
productionSuitable: falseflag reflected maturity gaps — no README, no demo profile wiring, no bespoke tests — rather than any inherent runtime limitation. That's a different situation frommodules/secrets/vault, whoseproductionSuitable: falseis permanent (it only ever runs Vault in ephemeral dev mode). dbt-core itself, running against a real Postgres with the module's existing hardened container settings (read-only root, dropped capabilities, non-root user), has no such limitation, so this PR closes the actual gaps and promotes it tomodules/.Module relocation
modules-experimental/transformation/dbt/→modules/transformation/dbt/: dropsproductionSuitable: falseand the(experimental)displayName/description suffix.modules/transformation/dbt/README.md(new): Purpose/Known limitations/Upstream docs/Configuration notes, matching the structure of other production module READMEs (e.g.modules/warehouse/postgres/README.md).Demo profile
profiles/local-dagster-postgres-superset-dbt/(new): the referencelocal-dagster-postgres-supersetstack plusdbtwired topostgresvia thesql-databasecontract. Added as a new profile rather than editing the base profile's composition, following the same patternlocal-dagster-postgres-superset-vaultestablished for optional add-ons.Tests
tests/test_dbt_hardening.py(new, 9 tests): image hardening (digest pin, non-root user, pip uninstalled in both build stages), compose service hardening (read_only,cap_drop, tmpfs paths, one-shotrestart: "no", the docs sidecar'sservice_completed_successfullydependency), the requiredtargetDatabase.contractRefschema field, and the entrypoint's required connection env vars — closing the "zero bespoke test files" gap the module had while experimental.Docs
docs/architecture.md: Transformation layer row now listsdbtinstead of(planned).docs/roadmap.md: dbt module and new profile moved from the experimental/near-term list into Stable Components.images/dbt/README.md: updated module path references, dropped the "It is experimental" note (the image's build/publish/scan/sign workflow wiring was already identical to the production Dagster/Superset images — nothing there needed to change).Testing
make check— 623 tests, ruff, yamllint, markdownlint all pass.cds validate/cds test(plan+render stages) /cds renderpass for the newlocal-dagster-postgres-superset-dbtprofile; manually confirmed the rendereddbt-runservice correctly resolvesDBT_HOST/DBT_PORT/DBT_DBNAME/DBT_USER/DBT_PASSWORDfrom thepostgres.sql-databasecontract binding.python -m unittest tests.test_module_isolation -vpasses for the relocated module (no cross-module service-name hardcoding), confirming it holds up under the stricter scrutinymodules/(vs.modules-experimental/) modules get.