chore: modernize PostgreSQL connector for baton-admin by jugonzalez12 · Pull Request #51 · ConductorOne/baton-postgresql

jugonzalez12 · 2026-09-10T17:56:10Z

Upgrade baton-sdk from v0.3.9 to v0.29.0 and prepare the connector for baton-admin's managed CI and metadata generation workflows.

SDK and connector compatibility:

Raise the Go requirement to 1.25.2, update dependencies, and regenerate the vendored dependency tree.
Adapt configuration constraints to field.WithConstraints.
Migrate account creation and credential rotation to LocalCredentialOptions and pass context to password generation.
Move the enabled status from the deprecated user trait field to the resource status field, preserving the existing enabled-state behavior.

Credential-free capability generation:

Register a zero-value Postgresql connector as the default capabilities builder so metadata generation does not require a DSN or live database.
Allow Default on a nil client pool when constructing capability syncers.
Add a regression test for building the zero-value connector and listing its resource types without database access.
Refresh baton_capabilities.json with the upgraded SDK's output.

CI and test maintenance:

Update the PR and main workflows to Go 1.25.x, golangci-lint-action v9, and golangci-lint v2.11.4.
Adopt the baton-admin golangci-lint v2 configuration and resolve the resulting style findings.
Migrate integration tests from the removed C1Z manager API to NewStore and c1zstore.Store, including context-aware store cleanup.
Paginate test resource lookups so resources beyond the first 100 results can be found.

Validation:

Full Go test suite passed, including Postgres integration tests.
golangci-lint reported zero issues.
The connector built successfully; credential-free capabilities output matched baton_capabilities.json, and config output succeeded.
Normal execution without a DSN still failed required-flag validation.

Validation used local Go 1.27.1 and golangci-lint 2.13.2. The exact CI tool versions and GitHub Actions execution were not verified locally. Deployment of baton-admin's managed workflows remains a separate step.

Upgrade baton-sdk from v0.3.9 to v0.29.0 and prepare the connector for baton-admin's managed CI and metadata generation workflows. SDK and connector compatibility: - Raise the Go requirement to 1.25.2, update dependencies, and regenerate the vendored dependency tree. - Adapt configuration constraints to field.WithConstraints. - Migrate account creation and credential rotation to LocalCredentialOptions and pass context to password generation. - Move the enabled status from the deprecated user trait field to the resource status field, preserving the existing enabled-state behavior. Credential-free capability generation: - Register a zero-value Postgresql connector as the default capabilities builder so metadata generation does not require a DSN or live database. - Allow Default on a nil client pool when constructing capability syncers. - Add a regression test for building the zero-value connector and listing its resource types without database access. - Refresh baton_capabilities.json with the upgraded SDK's output. CI and test maintenance: - Update the PR and main workflows to Go 1.25.x, golangci-lint-action v9, and golangci-lint v2.11.4. - Adopt the baton-admin golangci-lint v2 configuration and resolve the resulting style findings. - Migrate integration tests from the removed C1Z manager API to NewStore and c1zstore.Store, including context-aware store cleanup. - Paginate test resource lookups so resources beyond the first 100 results can be found. Validation: - Full Go test suite passed, including Postgres integration tests. - golangci-lint reported zero issues. - The connector built successfully; credential-free capabilities output matched baton_capabilities.json, and config output succeeded. - Normal execution without a DSN still failed required-flag validation. Validation used local Go 1.27.1 and golangci-lint 2.13.2. The exact CI tool versions and GitHub Actions execution were not verified locally. Deployment of baton-admin's managed workflows remains a separate step.

github-actions · 2026-09-10T18:06:28Z

Connector PR Review: chore: modernize PostgreSQL connector for baton-admin

Blocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base a8f945bc2c10.
Review mode: full
View review run

Review Summary

Scanned the full PR diff for security and correctness across all 21 non-vendored files (vendored module content excluded from bulk review; go.mod, go.sum, and vendor/modules.txt were reviewed for consistency). The two prior findings that were actionable are addressed: scripts/get-baton.sh now verifies the downloaded archive against the release checksums file (matching the archive name specifically, so a stale artifact can't pass), and TestZeroValueConnectorBuildsWithoutDatabase now asserts GetMetadata and the GetCapabilities getter including both credential-detail blocks, which is what the credential-free capabilities.yaml depends on. The earlier pkg/postgres/client.go note still stands as written — Get remains unguarded against a nil receiver — and is not re-flagged here. No new blocking issues found.

I verified the risky parts of the SDK v0.3.9 → v0.29.0 jump against the vendored SDK rather than taking the description at face value: WithDefaultCapabilitiesConnectorBuilder is routed through ExtractDefaultConnector, so the capabilities subcommand skips required-flag validation and never touches the main run path; dropping WithStatus from the user trait in role.go is behavior-preserving because NewUserTrait backfills STATUS_ENABLED when unset, and the new resource-level Status is additive; conf.gen.go matches the SDK generator template byte-for-byte (not hand-edited); the released baton-sdk assets match the names get-baton.sh constructs; and no vendored module requires a Go version above the new go 1.25.2.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

pkg/connector/connector_test.go:58 — the zero-value capabilities test asserts only non-emptiness, so a future config-dependent ResourceSyncers() would silently shrink the auto-committed baton_capabilities.json.
pkg/connector/role.go:309 — Rotate was migrated to *v2.LocalCredentialOptions / crypto.GeneratePassword(ctx, …) but its CI step remains commented out, leaving it the one migrated provisioning path with no automated coverage.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/connector/connector_test.go`:
- Around line 46-70: TestZeroValueConnectorBuildsWithoutDatabase only asserts that
  the resource-type list and capabilities are non-empty. Because
  .github/workflows/capabilities.yaml now generates and auto-commits
  baton_capabilities.json from this zero-value connector with no database, a future
  change that makes Postgresql.ResourceSyncers() depend on config (for example
  gating the column syncer on includeColumns or the large-object syncer on
  includeLargeObjects) would silently drop resource types from the committed
  capabilities file without failing any test. Strengthen the test to assert the
  exact expected set of resource type IDs (column, database, function,
  large-object, procedure, role, schema, sequence, table, view), or build a
  fully-configured connector and assert its resource-type set equals the
  zero-value connector's.

In `pkg/connector/role.go`:
- Around line 286-324: Rotate's signature changed from *v2.CredentialOptions to
  *v2.LocalCredentialOptions and it now calls crypto.GeneratePassword(ctx,
  credentialOptions), but nothing exercises this path automatically: the
  "Rotate credentials for user" step at the bottom of .github/workflows/ci.yaml is
  still commented out, and there is no unit test for Rotate. CreateAccount received
  the same migration and is covered by CI. Re-enable the rotate step in ci.yaml --
  the new "Fetch user id" step already resolves a role id into NEW_USER_ID that can
  be passed to --rotate-credentials with --rotate-credentials-type role -- or add a
  test covering Rotate.

github-actions

No blocking issues found.

Use the baton CLI published with the baton-sdk version selected by go.mod instead of the legacy standalone release, which cannot read the upgraded connector's Pebble-backed C1Z files. Support the SDK's Linux tarballs and macOS zip archives, and fail installation on HTTP download errors. Make the lifecycle test explicitly use Pebble. Check grant membership with any() so unrelated principals and result ordering cannot produce false failures or false successes. Select Bash explicitly to enable pipefail for the job, and correct the shell options in the user ID lookup so errors from baton cannot be masked by successful jq commands. Remove the Postgres service and BATON_DSN from capability generation to exercise the credential-free, zero-value connector path in CI. Refresh .golangci.yml from the locally updated baton-admin source config with goimports enabled, restoring formatting checks after the v2 migration. The corresponding baton-admin source change must be committed separately. Validation: - All 14 lifecycle steps passed locally against Postgres 16 with Pebble and the baton-sdk v0.29.0 CLI, including grant/revoke/re-grant and account creation, role assignment, and deletion. - All 18 membership and producer-failure checks passed. - Actionlint, shellcheck, golangci-lint, and git diff checks passed. - Credential-free capability output matched baton_capabilities.json. - CGO-disabled builds passed for Linux and macOS on amd64/arm64 and Windows amd64, matching the shared release templates' CGO settings. Lifecycle validation ran on macOS arm64, not the GitHub Ubuntu runner.

github-actions

No blocking issues found.

Reject empty or multiple role matches before writing NEW_USER_ID to the GitHub environment file, with clear lookup errors. Normalize Linux aarch64 to arm64 when selecting the SDK CLI release asset. Regenerate the config with baton-sdk v0.29.0, including GetString support for byte slices. Validation: all 14 Postgres/Pebble lifecycle steps passed with the rebuilt connector; lookup edge cases, platform asset selection, actionlint, shellcheck, and diff checks passed.

github-actions

No blocking issues found.

Verify the selected SDK CLI archive against its release SHA-256 manifest before extraction. Support sha256sum and the macOS shasum fallback, and reject missing checksums without accepting unrelated local files. Extend the zero-value connector regression test to exercise GetMetadata and the CLI's preferred GetCapabilities path, including account provisioning and credential rotation details. Validation: capability test, lint, shellcheck, and diff checks passed. Verified valid, corrupt, missing, and malformed checksum cases with both hash tools; checked the actual v0.29.0 release and confirmed failed downloads cannot fall back to stale files.

github-actions

No blocking issues found.

kans · 2026-09-10T19:27:05Z

+	// The capabilities subcommand runs without a DSN: it builds the zero-value
+	// connector instead of validating the required flag.
+	_, cmd, err := configschema.DefineConfiguration(ctx, "baton-postgresql", getConnector, cfg.Config,
+		connectorrunner.WithDefaultCapabilitiesConnectorBuilder(&connector.Postgresql{}))


I think we have a newer way of doing this - RunConnector.

we do! this connector is ancient and I figured the v2 migration was fit for another change, since the goal here is to get this connector baton-admin managed and stop drifting.

ggreer · 2026-09-10T19:26:28Z

          POSTGRES_PASSWORD: secretpassword
    env:
      BATON_LOG_LEVEL: debug
+      BATON_STORAGE_ENGINE: pebble


We default to pebble now, so this isn't really needed.

ggreer · 2026-09-10T19:27:03Z

@@ -77,19 +81,19 @@ jobs:
        run: ./baton-postgresql --grant-entitlement "${{ env.CONNECTOR_ENTITLEMENT }}" --grant-principal "${{ env.CONNECTOR_PRINCIPAL }}" --grant-principal-type "${{ env.CONNECTOR_PRINCIPAL_TYPE }}"

      - name: Check grant was granted


It can be in another PR, but we can replace this with the sync test action. Getting that test to pass will require changing main() to use RunConnector though, since it checks the exit code for bad credentials.

ggreer · 2026-09-10T19:29:25Z

 ARCH=$(uname -m)
 if [ "${ARCH}" = "x86_64" ]; then
  ARCH="amd64"
+elif [ "${ARCH}" = "aarch64" ]; then


This script can be replaced with the get baton action, but the sync action gets the baton utility, so again it's a change that doesn't need to happen in this PR.