Skip to content

ci: check generated service containers - #621

Merged
iamgp merged 63 commits into
mainfrom
GWP/generated-container-checks
Jul 27, 2026
Merged

iamgp merged 63 commits into
mainfrom
GWP/generated-container-checks

Conversation

@iamgp

@iamgp iamgp commented Jul 22, 2026 •

Copy link
Copy Markdown
Collaborator

Operator outcome

phlo plugin check --containers now exercises the installed public CLI against a disposable generated project, lints every generated Dockerfile with pinned Hadolint, scans every generated image with pinned Trivy, attributes all 31 Compose service entries to their owning packages, and fails closed on missing or changed evidence.

PR CI runs phlo plugin check --containers --remote-images: Hadolint checks the generated Dockerfiles, while Trivy scans the published GHCR images without rebuilding service images on every pull request.

What changed

  • Hardens the generated images and service pins across Alloy, ClickHouse, ClickStack, Dagster, Grafana, Loki, MinIO, Nessie, OAuth2 Proxy, Observatory, OpenMetadata, pgweb, Phlo API, PostgreSQL, PostgREST, Prometheus, Superset, and Trino.
  • Adds reproducible custom rebuilds where upstream stable images retain fixable HIGH/CRITICAL findings.
  • Publishes 21 package-owned service images as multi-architecture GHCR manifests from the release/manual publication workflow, so PR CI consumes packaged images instead of rebuilding them.
  • Runs local generated builds and scans sequentially in an isolated Buildx builder, removes task-created tags, restores any pre-existing operator tags, prunes task cache between images, and removes the builder on every exit path.
  • Binds every vulnerability waiver to one service, one exact image tag, and a SHA-256 of the complete (vulnerability ID, component) multiset. New, removed, or reassigned findings fail; scanner operational errors cannot be waived.
  • Builds all workspace wheels, hydrates the selected service wheelhouse, and installs the consumer environment with --find-links plus --no-index before invoking the public CLI.
  • Raises the local generated-container job timeout from 45 to 120 minutes; the complete local build-backed run takes about 65 minutes, while the remote-image PR check completes without service-image builds.

Complete service matrix

Fresh CI proof on head eb85f7d78900a02fafbff249dff6513e429d8648: all 31 Compose service entries were attributed and passed or matched an exact evidence-bound waiver. All 21 package-owned GHCR service images have successful multi-architecture manifests.

Passed locally: ClickHouse, ClickHouse setup, Dagster, Dagster daemon, Hasura, Loki, MinIO setup, Nessie, OAuth2 Proxy, Observatory, pgweb, Phlo API, PostgreSQL, PostgreSQL exporter, PostgreSQL volume setup, PostgREST, Prometheus, RustFS, RustFS setup, RustFS volume setup, Traefik.

Matched exact evidence-bound waivers locally: Alloy, Grafana, MinIO, OpenMetadata Elasticsearch, Superset, Trino.

ClickStack, OpenMetadata server, OpenMetadata MySQL, and OpenMetadata setup were registry-blocked in the final local pass. The fresh CI remote-image run completed all four: ClickStack matched its exact waiver, and the OpenMetadata entries passed.

Waivers

  • Alloy: three Docker module advisories retained by Alloy 1.18.0; two have no fix and one requires an incompatible Docker client major.
  • ClickStack: HyperDX 2.31.0 bundles source-built OTel binaries with grpc 1.81.1 and Go 1.26.4. Replacing them requires rebuilding the upstream OCB distribution; legacy MongoDB OpenSSL, root orchestration, and Next findings also remain.
  • Grafana: Trivy identifies two Tempo findings from stale pseudo-version metadata although the image rebuilds exact Tempo 2.10.7 source containing the fixes.
  • MinIO: archived source retains six MinIO-module advisories without public patches; OS, standard-library, and third-party Go findings are remediated.
  • OpenMetadata Elasticsearch: upstream 8.11.4 shades old Jackson versions, requires Tika 2.x, and retains one unpublished and one unfixed LZ4 advisory.
  • Superset: upstream requires three vulnerable Python versions and Debian 12 has no fixed packages for the remaining OS advisories.
  • Trino: Trino 483 requires Jetty 11 while the remaining HTTP advisory is fixed only in incompatible Jetty 12.

Each waiver digest is declared in .github/workflows/ci.yml; the checker emits the full finding evidence and observed digest.

Validation

  • uv run pytest --import-mode=importlib tests/cli/test_cli_plugin.py — 40 passed.
  • uv run ruff check ... and uv run ruff format --check ... — passed.
  • make actionlint — passed with actionlint 1.7.7.
  • make zizmor — no findings.
  • python3 scripts/release_golden_path.py — passed locally, including offline wheel installation, generated csv-batch startup, materialization, storage write/read, WAP, and cleanup.
  • Clean wheelhouse install with uv pip install --no-index --find-links ... — passed; installed phlo, version 0.12.1, and the built wheel was inspected for the committed checker implementation.
  • GHCR publication runs 30219724169, 30257097497, and 30265382788 — 21/21 unique package-owned multi-architecture image manifests published.
  • CI run 30268706704 — 22/22 checks passed, including containers / generated service files and python / release golden path.

The PR is approved, mergeable, review-clean, and all required GitHub checks are terminal and green on head eb85f7d78900a02fafbff249dff6513e429d8648.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The plugin check command validates generated service containers with Hadolint and Trivy. API and Dagster images use Alpine bases and non-root execution. CI runs the checks in a clean environment, while service images, dependencies, Postgres paths, and pgweb packaging are updated.

Changes

Generated container validation and service updates

Layer / File(s) Summary
Container check command
src/phlo/cli/commands/plugin/check.py, tests/cli/test_cli_plugin.py
Adds --containers, generated Dockerfile ownership checks, Hadolint and Trivy execution, structured results, failure aggregation, CLI output, and coverage for success and failure paths.
Runtime image hardening
packages/phlo-api/src/phlo_api/Dockerfile, packages/phlo-dagster/src/phlo_dagster/Dockerfile, packages/phlo-dagster/tests/*, tests/scripts/test_release_golden_path.py, packages/phlo-observatory/src/phlo_observatory/Dockerfile
Switches images to Alpine, pins uv, adjusts dependency installation, removes npm from Observatory, creates the phlo account, updates ownership, and switches runtime execution to that user.
CI container lane
.github/workflows/ci.yml
Builds and installs Phlo wheels in a clean environment, runs generated-container checks, and includes the result in CI status reporting and failure gating.
Service versions and dependencies
packages/phlo-*/src/**, packages/phlo-*/README.md, packages/phlo-iceberg/pyproject.toml
Updates service image defaults, Grafana dashboard plugin versions, documented configuration values, and Iceberg dependencies.
Postgres stack alignment
packages/phlo-postgres/src/**, registry/support/v1.json, scripts/recovery_drill.py, tests/scripts/test_recovery_drill.py
Updates PostgreSQL, Nessie, and Trino stack images, moves the data volume to /var/lib/postgresql, and aligns recovery-drill generation and tests.
Pgweb build integration
packages/phlo-pgweb/pyproject.toml, packages/phlo-pgweb/src/phlo_pgweb/*, packages/phlo-pgweb/tests/*
Packages a new pinned pgweb Dockerfile, changes the service to build locally, maps the Dockerfile into the generated context, and tests the build metadata.

Sequence Diagram(s)

sequenceDiagram
  participant CI
  participant PhloCLI
  participant GeneratedServices
  participant Docker
  participant Scanners
  CI->>PhloCLI: run plugin check --containers
  PhloCLI->>GeneratedServices: initialise and add services
  GeneratedServices-->>PhloCLI: return generated Dockerfiles and Compose file
  PhloCLI->>Docker: build, pull, inspect, and parse services
  PhloCLI->>Scanners: run Hadolint and Trivy
  Scanners-->>PhloCLI: return validation results
  PhloCLI-->>CI: return pass or aggregated failure
Loading

Possibly related PRs

  • phlohouse/phlo#484: Adjusts JSON and non-JSON output for the same plugin check command.
  • phlohouse/phlo#489: Modifies the same CI status aggregation and lane failure-gating workflow.
  • phlohouse/phlo#588: Touches the Dagster prerelease requirements handling updated here.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately summarises the main CI change around generated service container checks.
Description check ✅ Passed The description is clearly related to the changeset and matches the new CI and container-scanning workflow.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch GWP/generated-container-checks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@iamgp
iamgp marked this pull request as ready for review July 22, 2026 21:33
@github-actions

github-actions Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Cairn Quality Report

Commit: 8845d91 · View full report

Checker Status ✅ Passed ❌ Failed Items
ruff passed 0 0 0
ruff-format pass 0 0 1
pytest-3.11 passed 1434 0 1438
pytest-3.12 passed 1434 0 1438

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (2)
tests/cli/test_cli_plugin.py (1)

179-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Coverage gap: the _service_inventory() production path is never exercised.

Every test supplies service_files/service_names explicitly or stubs check_generated_containers, so the default no-argument invocation used by check_cmd (and therefore CI) — which derives owners from _service_inventory() — is untested. This is exactly where the dest-vs-.phlo-relative ownership mismatch flagged in check.py would surface. Consider a test that drives the inventory path with a real/fake service plugin.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/cli/test_cli_plugin.py` around lines 179 - 203, The test
test_plugin_check_containers_checks_generated_project must exercise
check_generated_containers without explicit service_files or service_names,
allowing check_cmd’s default inventory flow to call _service_inventory().
Provide a real or fake service plugin and assert the generated container
ownership is handled correctly, including the dest versus .phlo-relative path
behavior; do not stub the inventory path.
src/phlo/cli/commands/plugin/check.py (1)

157-182: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Pin the scanner images by digest/tag rather than :latest.

hadolint/hadolint:latest and aquasec/trivy:latest make this required CI lane non-reproducible: an upstream image change (new rules or a scanner bug) can flip results without any code change, and :latest weakens supply-chain guarantees for a security-scanning gate. Prefer a pinned version tag or digest.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/phlo/cli/commands/plugin/check.py` around lines 157 - 182, Replace the
mutable :latest references in the hadolint and trivy _run_command invocations
with pinned version tags or immutable image digests. Update both
hadolint/hadolint and aquasec/trivy while preserving the existing scanner
commands and arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/phlo-dagster/src/phlo_dagster/Dockerfile`:
- Around line 41-47: Update the installation flow after the bare phlo install so
the base defaults/Dagster dependencies are installed unconditionally, including
phlo[defaults], PHLO_DBT_REQUIREMENT, dagster-webserver, dagster-postgres, and
psycopg[binary]. Keep PHLO_PRERELEASE_REQUIREMENTS as a guard only for applying
additional prerelease pins, ensuring stable PHLO_VERSION builds still receive
the full runtime dependencies.

---

Nitpick comments:
In `@src/phlo/cli/commands/plugin/check.py`:
- Around line 157-182: Replace the mutable :latest references in the hadolint
and trivy _run_command invocations with pinned version tags or immutable image
digests. Update both hadolint/hadolint and aquasec/trivy while preserving the
existing scanner commands and arguments.

In `@tests/cli/test_cli_plugin.py`:
- Around line 179-203: The test
test_plugin_check_containers_checks_generated_project must exercise
check_generated_containers without explicit service_files or service_names,
allowing check_cmd’s default inventory flow to call _service_inventory().
Provide a real or fake service plugin and assert the generated container
ownership is handled correctly, including the dest versus .phlo-relative path
behavior; do not stub the inventory path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7a9d0816-a356-4ff8-83b6-f557e0129425

📥 Commits

Reviewing files that changed from the base of the PR and between 7b9861a and b21fe2b.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • packages/phlo-api/src/phlo_api/Dockerfile
  • packages/phlo-dagster/src/phlo_dagster/Dockerfile
  • src/phlo/cli/commands/plugin/check.py
  • tests/cli/test_cli_plugin.py
📜 Review details
🔇 Additional comments (10)
src/phlo/cli/commands/plugin/check.py (4)

5-32: LGTM!


52-80: LGTM!


228-289: LGTM!


45-49: 🎯 Functional Correctness

No change needed: dest paths are .phlo-relative.

The generated service files are declared with paths such as dagster/Dockerfile, and compose generation copies them under .phlo/<service>/...; the ownership lookup uses the same relative paths.

tests/cli/test_cli_plugin.py (1)

245-338: LGTM!

packages/phlo-api/src/phlo_api/Dockerfile (1)

19-45: LGTM!

packages/phlo-dagster/src/phlo_dagster/Dockerfile (2)

16-23: LGTM!


49-56: LGTM!

.github/workflows/ci.yml (2)

298-320: LGTM!


522-567: LGTM!

Comment thread packages/phlo-dagster/src/phlo_dagster/Dockerfile Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/phlo/cli/commands/plugin/check.py`:
- Around line 461-474: Update the missing_results comprehension in the
service-results validation flow to exclude services whose status is already
"failed", while retaining the existing image_scan check for other services. Keep
the failures appended by the preceding loop unchanged so build, pull, and
inspect errors are reported only once.
- Around line 264-269: Update the Compose service ownership validation in the
loop over compose_services to allow generated support services without
discoverable package owners, such as profiled setup and daemon companions.
Restrict the hard failure to services explicitly expected to have owners, or
otherwise bypass the check for recognized generated support services while
preserving errors for genuinely unexpected unowned services.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ca534a18-a8fd-4059-99ef-7fcca9506166

📥 Commits

Reviewing files that changed from the base of the PR and between b21fe2b and 82b9e9a.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • packages/phlo-dagster/src/phlo_dagster/Dockerfile
  • src/phlo/cli/commands/plugin/check.py
  • tests/cli/test_cli_plugin.py
  • tests/scripts/test_release_golden_path.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
  • GitHub Check: integration / required suites
  • GitHub Check: security / docs site generation
  • GitHub Check: containers / generated service files
  • GitHub Check: python / recovery continuity drill
  • GitHub Check: python / core tests (3.12)
  • GitHub Check: python / core tests (3.11)
  • GitHub Check: frontend / observatory
  • GitHub Check: python / release golden path
  • GitHub Check: docs / build
🧰 Additional context used
🪛 ast-grep (0.44.1)
tests/cli/test_cli_plugin.py

[info] 200-208: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"name": "test-project",
"services": {
"dagster": {"image": "example/dagster:1"},
"observatory": {"image": "example/observatory:1"},
},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)


[info] 361-369: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"name": "test-project",
"services": {
"one": {"image": "example/one:1"},
"two": {"image": "example/two:1"},
},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

🔇 Additional comments (6)
tests/cli/test_cli_plugin.py (1)

179-280: LGTM!

Also applies to: 283-317, 339-396

packages/phlo-dagster/src/phlo_dagster/Dockerfile (2)

43-46: Base defaults/Dagster requirements are now installed unconditionally with prerelease pins appended conditionally, addressing the earlier concern about stable PHLO_VERSION builds skipping the runtime dependency set. Bash arrays are safe here since the stage now uses SHELL ["/bin/bash", ...].

LGTM!


16-23: LGTM!

Also applies to: 48-58

src/phlo/cli/commands/plugin/check.py (2)

78-123: LGTM!


44-75: LGTM!

tests/scripts/test_release_golden_path.py (1)

795-802: LGTM!

Comment thread src/phlo/cli/commands/plugin/check.py Outdated
Comment thread src/phlo/cli/commands/plugin/check.py
@iamgp
iamgp force-pushed the GWP/generated-container-checks branch from 526daa1 to 156a09c Compare July 24, 2026 17:03

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/phlo-dagster/src/phlo_dagster/Dockerfile (1)

41-44: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Parse Requires-Dist with a PEP 508-aware parser.

The literal extra == 'defaults' match is quote-sensitive, and req.split(";")[0] discards everything after the first semicolon, including non-extra environment markers. Parse the marker part instead, support both single and double quoted marker values, and keep compound markers attached to the dependency so platform/Python-specific prerelease pins are not omitted or made unconditional.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/phlo-dagster/src/phlo_dagster/Dockerfile` around lines 41 - 44,
Update the PHLO_PRERELEASE_REQUIREMENTS extraction in the Dockerfile to use a
PEP 508-aware parser for each Requires-Dist entry instead of quote-sensitive
matching and req.split(";"). Select dependencies whose parsed marker includes
extra == "defaults" while preserving all other environment-marker conditions on
the dependency, and support both quote styles without making
platform/Python-specific prerelease pins unconditional.
🧹 Nitpick comments (1)
packages/phlo-observatory/src/phlo_observatory/Dockerfile (1)

24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid upgrading the entire Alpine base during the application build.

apk upgrade makes the image depend on repository state at build time and may introduce untested package changes. Prefer a refreshed/pinned base image or install only the required package.

Proposed change
-RUN apk upgrade --no-cache \
-    && apk add --no-cache docker-cli
+RUN apk add --no-cache docker-cli
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/phlo-observatory/src/phlo_observatory/Dockerfile` around lines 24 -
25, Remove the apk upgrade step from the Dockerfile build command and retain
only the installation of the required docker-cli package. Do not add broad
package upgrades; rely on the selected Alpine base image for its package
versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/phlo-postgres/src/phlo_postgres/service.yaml`:
- Line 9: Update the PostgreSQL service configuration around the image change to
include a tested PostgreSQL 16-to-18 data migration path, such as dump/restore
or pg_upgrade, before using postgres:18-alpine. Ensure existing data is migrated
rather than silently initializing a separate cluster; otherwise fail closed with
explicit backup and migration instructions.

---

Outside diff comments:
In `@packages/phlo-dagster/src/phlo_dagster/Dockerfile`:
- Around line 41-44: Update the PHLO_PRERELEASE_REQUIREMENTS extraction in the
Dockerfile to use a PEP 508-aware parser for each Requires-Dist entry instead of
quote-sensitive matching and req.split(";"). Select dependencies whose parsed
marker includes extra == "defaults" while preserving all other
environment-marker conditions on the dependency, and support both quote styles
without making platform/Python-specific prerelease pins unconditional.

---

Nitpick comments:
In `@packages/phlo-observatory/src/phlo_observatory/Dockerfile`:
- Around line 24-25: Remove the apk upgrade step from the Dockerfile build
command and retain only the installation of the required docker-cli package. Do
not add broad package upgrades; rely on the selected Alpine base image for its
package versions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7da1d011-84d3-42c6-86cc-0568f4f05fa5

📥 Commits

Reviewing files that changed from the base of the PR and between 864878a and 4fef812.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (41)
  • .github/workflows/ci.yml
  • packages/phlo-alloy/src/phlo_alloy/service.yaml
  • packages/phlo-api/src/phlo_api/Dockerfile
  • packages/phlo-dagster/src/phlo_dagster/Dockerfile
  • packages/phlo-dagster/tests/test_runtime_image_contract.py
  • packages/phlo-grafana/README.md
  • packages/phlo-grafana/src/phlo_grafana/dashboards/cascade-overview.json
  • packages/phlo-grafana/src/phlo_grafana/dashboards/infrastructure.json
  • packages/phlo-grafana/src/phlo_grafana/service.yaml
  • packages/phlo-hasura/README.md
  • packages/phlo-hasura/src/phlo_hasura/service.yaml
  • packages/phlo-iceberg/pyproject.toml
  • packages/phlo-loki/README.md
  • packages/phlo-loki/src/phlo_loki/service.yaml
  • packages/phlo-nessie/README.md
  • packages/phlo-nessie/src/phlo_nessie/service.yaml
  • packages/phlo-nessie/src/phlo_nessie/settings.py
  • packages/phlo-oauth2-proxy/src/phlo_oauth2_proxy/service.yaml
  • packages/phlo-observatory/src/phlo_observatory/Dockerfile
  • packages/phlo-openmetadata/src/phlo_openmetadata/openmetadata-mysql-setup.yaml
  • packages/phlo-openmetadata/src/phlo_openmetadata/openmetadata-setup.yaml
  • packages/phlo-openmetadata/src/phlo_openmetadata/service.yaml
  • packages/phlo-pgweb/src/phlo_pgweb/service.yaml
  • packages/phlo-postgres/src/phlo_postgres/exporter_service.yaml
  • packages/phlo-postgres/src/phlo_postgres/service.yaml
  • packages/phlo-postgres/src/phlo_postgres/volume_setup.yaml
  • packages/phlo-postgrest/README.md
  • packages/phlo-postgrest/src/phlo_postgrest/service.yaml
  • packages/phlo-prometheus/README.md
  • packages/phlo-prometheus/src/phlo_prometheus/service.yaml
  • packages/phlo-rustfs/src/phlo_rustfs/rustfs-setup.yaml
  • packages/phlo-superset/README.md
  • packages/phlo-superset/src/phlo_superset/service.yaml
  • packages/phlo-traefik/src/phlo_traefik/service.yaml
  • packages/phlo-trino/src/phlo_trino/service.yaml
  • registry/support/v1.json
  • scripts/recovery_drill.py
  • src/phlo/cli/commands/plugin/check.py
  • tests/cli/test_cli_plugin.py
  • tests/scripts/test_recovery_drill.py
  • tests/scripts/test_release_golden_path.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/scripts/test_release_golden_path.py
  • packages/phlo-dagster/tests/test_runtime_image_contract.py
  • .github/workflows/ci.yml
  • src/phlo/cli/commands/plugin/check.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: python / core tests (3.12)
  • GitHub Check: python / core tests (3.11)
  • GitHub Check: containers / generated service files
  • GitHub Check: python / release golden path
🧰 Additional context used
🪛 ast-grep (0.44.1)
tests/cli/test_cli_plugin.py

[info] 354-354: use jsonify instead of json.dumps for JSON output
Context: json.dumps(compose_config)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)


[info] 386-388: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{"name": "test-project", "services": {"one": {"image": "example/one:1"}}}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

🔇 Additional comments (38)
tests/cli/test_cli_plugin.py (2)

336-370: LGTM!


372-411: LGTM!

packages/phlo-api/src/phlo_api/Dockerfile (1)

1-1: LGTM!

Also applies to: 11-25, 43-55

packages/phlo-dagster/src/phlo_dagster/Dockerfile (2)

1-8: LGTM!

Also applies to: 10-14, 17-22, 50-51


55-58: 🩺 Stability & Availability

Apply ownership after the final COPY instructions.

The recursive chown runs before Lines 63-64 copy workspace.yaml and dagster.yaml, so those files remain root-owned even though the container runs as phlo. Move the copy operations before chown, or use COPY --chown=phlo:phlo; verify whether Dagster or its entrypoint needs to modify these files. (docs.docker.com)

packages/phlo-alloy/src/phlo_alloy/service.yaml (1)

7-7: LGTM!

packages/phlo-grafana/README.md (1)

26-26: LGTM!

packages/phlo-grafana/src/phlo_grafana/dashboards/cascade-overview.json (1)

70-70: LGTM!

Also applies to: 130-130, 190-190, 250-250, 372-372, 459-459

packages/phlo-grafana/src/phlo_grafana/dashboards/infrastructure.json (1)

90-90: LGTM!

Also applies to: 238-238, 374-374, 470-470, 570-570, 667-667

packages/phlo-grafana/src/phlo_grafana/service.yaml (1)

7-7: LGTM!

Also applies to: 46-46

packages/phlo-hasura/README.md (1)

26-26: LGTM!

packages/phlo-hasura/src/phlo_hasura/service.yaml (1)

7-7: LGTM!

Also applies to: 35-35

packages/phlo-pgweb/src/phlo_pgweb/service.yaml (1)

6-6: LGTM!

packages/phlo-postgres/src/phlo_postgres/exporter_service.yaml (1)

7-7: LGTM!

packages/phlo-postgrest/README.md (1)

26-26: LGTM!

packages/phlo-postgrest/src/phlo_postgrest/service.yaml (1)

7-7: LGTM!

Also applies to: 29-29

packages/phlo-iceberg/pyproject.toml (1)

12-13: LGTM!

packages/phlo-loki/README.md (1)

26-26: LGTM!

packages/phlo-loki/src/phlo_loki/service.yaml (1)

7-7: LGTM!

Also applies to: 38-38

packages/phlo-nessie/README.md (1)

25-25: LGTM!

packages/phlo-nessie/src/phlo_nessie/service.yaml (1)

6-6: LGTM!

Also applies to: 54-54

packages/phlo-nessie/src/phlo_nessie/settings.py (1)

30-30: LGTM!

packages/phlo-oauth2-proxy/src/phlo_oauth2_proxy/service.yaml (1)

7-7: LGTM!

packages/phlo-observatory/src/phlo_observatory/Dockerfile (1)

35-36: LGTM!

packages/phlo-openmetadata/src/phlo_openmetadata/openmetadata-mysql-setup.yaml (1)

7-7: LGTM!

packages/phlo-openmetadata/src/phlo_openmetadata/openmetadata-setup.yaml (1)

11-11: LGTM!

packages/phlo-openmetadata/src/phlo_openmetadata/service.yaml (1)

12-12: LGTM!

Also applies to: 54-54

packages/phlo-prometheus/README.md (1)

26-26: LGTM!

packages/phlo-prometheus/src/phlo_prometheus/service.yaml (1)

7-7: LGTM!

Also applies to: 42-42

packages/phlo-rustfs/src/phlo_rustfs/rustfs-setup.yaml (1)

7-7: LGTM!

packages/phlo-superset/README.md (1)

22-22: LGTM!

packages/phlo-superset/src/phlo_superset/service.yaml (1)

6-6: LGTM!

Also applies to: 49-49

packages/phlo-traefik/src/phlo_traefik/service.yaml (1)

7-7: LGTM!

packages/phlo-trino/src/phlo_trino/service.yaml (1)

6-6: LGTM!

Also applies to: 44-44

packages/phlo-postgres/src/phlo_postgres/volume_setup.yaml (1)

14-20: LGTM!

registry/support/v1.json (1)

31-36: LGTM!

scripts/recovery_drill.py (1)

26-28: LGTM!

Also applies to: 80-80

tests/scripts/test_recovery_drill.py (1)

25-28: LGTM!

Comment thread packages/phlo-postgres/src/phlo_postgres/service.yaml Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/phlo-pgweb/src/phlo_pgweb/Dockerfile`:
- Around line 13-18: Remove the unpinned apk upgrade step from the Dockerfile
package installation command, leaving the explicitly version-pinned packages in
the existing apk add invocation unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3cbd30ee-25e5-4a9b-a71c-d4f2ec7506e7

📥 Commits

Reviewing files that changed from the base of the PR and between 4fef812 and d98fd89.

📒 Files selected for processing (6)
  • packages/phlo-api/src/phlo_api/Dockerfile
  • packages/phlo-dagster/src/phlo_dagster/Dockerfile
  • packages/phlo-pgweb/pyproject.toml
  • packages/phlo-pgweb/src/phlo_pgweb/Dockerfile
  • packages/phlo-pgweb/src/phlo_pgweb/service.yaml
  • packages/phlo-pgweb/tests/test_pgweb_plugin.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/phlo-dagster/src/phlo_dagster/Dockerfile
  • packages/phlo-api/src/phlo_api/Dockerfile
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: python / release golden path
  • GitHub Check: containers / generated service files
🔇 Additional comments (5)
packages/phlo-pgweb/pyproject.toml (1)

45-45: LGTM!

packages/phlo-pgweb/src/phlo_pgweb/Dockerfile (1)

1-12: LGTM!

Also applies to: 19-24

packages/phlo-pgweb/src/phlo_pgweb/service.yaml (2)

6-8: 🔒 Security & Privacy

Narrow the Docker build context, or verify that the root context is required.

Because the generator stages this file under .phlo/pgweb/Dockerfile, context: . sends the entire generated .phlo tree to the Docker daemon. This Dockerfile does not copy context files, so using pgweb as the context with dockerfile: Dockerfile would avoid transferring unrelated configuration or sensitive files. Otherwise, add an explicit .dockerignore and test its contents.


27-29: LGTM!

packages/phlo-pgweb/tests/test_pgweb_plugin.py (1)

12-13: LGTM!

Comment thread packages/phlo-pgweb/src/phlo_pgweb/Dockerfile Outdated
@iamgp
iamgp force-pushed the GWP/generated-container-checks branch 6 times, most recently from 5404cf3 to aec8d84 Compare July 25, 2026 23:53
@iamgp
iamgp merged commit c51fda7 into main Jul 27, 2026
23 checks passed
@iamgp
iamgp deleted the GWP/generated-container-checks branch July 27, 2026 19:23
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.

1 participant