Skip to content

CICD#10

Closed
NFUChen wants to merge 7 commits into
pgplex:mainfrom
NFUChen:cicd
Closed

CICD#10
NFUChen wants to merge 7 commits into
pgplex:mainfrom
NFUChen:cicd

Conversation

@NFUChen

@NFUChen NFUChen commented May 19, 2026

Copy link
Copy Markdown

No description provided.

@vercel

vercel Bot commented May 19, 2026

Copy link
Copy Markdown

@William-W-Chen is attempting to deploy a commit to the bytebase Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented May 19, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds container and Helm deployment support plus a new schema migration workflow. The main changes are:

  • New GHCR-focused publish workflow and Docker image changes.
  • New Helm chart and deployment values.
  • New metadata and migration ConnectRPC services.
  • Git-backed schema source sync and pgschema plan/apply helpers.
  • New migration panel in the SQL editor UI.

Confidence Score: 2/5

This should not be merged until the build, container, and migration configuration paths are fixed.

  • The Docker image can fail to build before publishing.
  • Env-based container configuration can fail at startup after switching to a non-root user.
  • The documented TOML schema source is parsed but ignored by the migration service.
  • Helm can expose database passwords through a ConfigMap.

Focus on Dockerfile, server/lib/config.ts, server/services/migration-service.ts, server/services/metadata-service.ts, server/lib/git.ts, and the Helm config templates.

Security Review

  • Secrets handling: helm-chart/templates/configmap.yaml stores pgconsole.toml database passwords in a ConfigMap.
  • Metadata integrity: server/services/metadata-service.ts uses an unqualified _pgconsole table, allowing search-path shadowing of migration metadata.

Important Files Changed

Filename Overview
server/services/migration-service.ts Adds migration planning and apply flow using metadata, git checkout, temp plan files, and pgschema.
server/services/metadata-service.ts Adds _pgconsole metadata CRUD for schema source configuration.
server/lib/git.ts Adds cached git checkout sync for schema repositories.
Dockerfile Bundles pgschema, installs git, and switches the runtime image to a non-root user.
helm-chart/templates/configmap.yaml Mounts the pgconsole TOML configuration from chart values.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  UI[MigrationPanel] --> RPC[MigrationService]
  RPC --> DB[_pgconsole metadata]
  RPC --> Git[syncRepo checkout]
  RPC --> PgSchema[pgschema plan/apply]
  PgSchema --> PlanStore[plan-store temp JSON]
  PgSchema --> Postgres[(PostgreSQL)]
Loading

Comments Outside Diff (1)

  1. server/lib/config.ts, line 20-36 (link)

    P1 Declare schema source

    loadConfigFromString now assigns schema_source onto each connection, and the new tests read conns[0].schema_source, but ConnectionConfig does not declare that property. A normal TypeScript build can fail with property/excess-field errors before the server bundle is produced.

Reviews (1): Last reviewed commit: "docs: update README.md for improved inst..." | Re-trigger Greptile

Comment on lines +84 to +90
const details = getConnectionDetails(conn)
const schemaSource = await getSchemaSource(details, user?.email)
if (!schemaSource) {
throw new ConnectError(
'No schema_source configured. Use SetMetadata to store a schema_source entry in the _pgconsole table.',
Code.FailedPrecondition,
)

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 Use configured source

The config parser and example TOML now support connections.schema_source, but this path only calls getSchemaSource, which reads the _pgconsole metadata table. A deployment that follows the documented TOML configuration will still receive No schema_source configured and cannot plan migrations until the same data is duplicated through metadata.

Comment thread Dockerfile
Comment on lines +17 to +20
ARG TARGETARCH
RUN unzip bin/pgschema-linux-${TARGETARCH}.zip -d /usr/local/bin \
&& mv /usr/local/bin/pgschema-*-linux-${TARGETARCH} /usr/local/bin/pgschema \
&& chmod +x /usr/local/bin/pgschema

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 Install unzip first

The builder stage is based on Alpine and does not install unzip before using it to extract the bundled pgschema archive. Docker builds can fail at this step with unzip: not found, so the GHCR image cannot be produced.

Suggested change
ARG TARGETARCH
RUN unzip bin/pgschema-linux-${TARGETARCH}.zip -d /usr/local/bin \
&& mv /usr/local/bin/pgschema-*-linux-${TARGETARCH} /usr/local/bin/pgschema \
&& chmod +x /usr/local/bin/pgschema
ARG TARGETARCH
RUN apk add --no-cache unzip \
&& unzip bin/pgschema-linux-${TARGETARCH}.zip -d /usr/local/bin \
&& mv /usr/local/bin/pgschema-*-linux-${TARGETARCH} /usr/local/bin/pgschema \
&& chmod +x /usr/local/bin/pgschema

Comment thread Dockerfile
ENV PORT=9876
EXPOSE 9876

USER pgconsole

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 Preserve env config

The image now runs as the pgconsole user, but the entrypoint writes PGCONSOLE_CONFIG to /etc/pgconsole.toml. That path is root-owned in this image, so containers started with env-based config fail before Node starts with a permission error.

Comment on lines +39 to +45
)
}
}

export const metadataServiceHandlers: ServiceImpl<typeof MetadataService> = {
async initMetadataTable(req, context) {
if (!req.connectionId) {

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 security Qualify metadata table

All metadata operations use the unqualified _pgconsole table and the existence check only matches relname. In databases with a custom search_path, pgconsole can create the table in one schema and later read another schema's _pgconsole; a writable schema earlier in the search path could also shadow the metadata table and change the schema_source used for migrations.

Comment thread server/lib/git.ts
Comment on lines +27 to +32
export async function syncRepo(connectionId: string, repo: string, branch?: string): Promise<{ commitHash: string }> {
// Serialize concurrent sync requests for the same connection
const existing = syncLocks.get(connectionId)
if (existing) {
return existing
}

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 Key sync by source

The in-flight sync lock is keyed only by connectionId. If one request starts syncing main and the schema source is changed to another branch or repo before a second request arrives, the second request returns the first promise and plans against the old source instead of the newly requested one.

Comment thread server/lib/git.ts
Comment on lines +47 to +58
if (exists) {
const cachedUrl = repoDirUrls.get(repoDir)
if (cachedUrl && cachedUrl !== repo) {
await rm(repoDir, { recursive: true, force: true })
}
}

const stillExists = await access(join(repoDir, '.git')).then(() => true).catch(() => false)

if (stillExists) {
await exec('git', ['fetch', 'origin', ...(branch ? [branch] : [])], repoDir)
await exec('git', ['reset', '--hard', branch ? `origin/${branch}` : 'FETCH_HEAD'], repoDir)

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 Check persisted remote

Repo URL changes are detected only through the in-memory repoDirUrls map. After a process restart with an existing checkout on disk, the map is empty, so changing schema_source.repo for the same connection reuses the old checkout and fetches the old origin instead of replacing it with the new repository.

Comment on lines +106 to +108
const repoDir = getRepoDir(req.connectionId)
const schemaFilePath = validateSchemaPath(repoDir, schemaPath)
const outputJsonPath = join(tmpdir(), `pgconsole-plan-${randomUUID()}.json`)

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 Remove temp plans

Each plan writes a JSON file under /tmp, but plan removal and TTL eviction only delete the in-memory map entry. Repeated planning leaves DDL plan files behind for the lifetime of the container or host cleanup, which can consume disk and retain schema details longer than the 30-minute plan lifetime.

Comment on lines +7 to +9
data:
pgconsole.toml: |
{{- .Values.config | nindent 4 }}

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 security Store secrets safely

The chart renders the entire pgconsole.toml into a ConfigMap, while the provided values and README examples include database passwords in that config. Anyone with ConfigMap read access in the namespace can read those credentials; this should be modeled as a Secret or split so sensitive values are mounted from Secrets.

@NFUChen NFUChen changed the title Cicd CICD May 19, 2026
@NFUChen NFUChen closed this May 19, 2026
@NFUChen NFUChen deleted the cicd branch May 19, 2026 06:21
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