Skip to content

feat!: add extensions for postgres database credentials - #192

Open
erika-wallace wants to merge 2 commits into
mainfrom
ew/postgres-database-credentials
Open

erika-wallace wants to merge 2 commits into
mainfrom
ew/postgres-database-credentials

Conversation

@erika-wallace

@erika-wallace erika-wallace commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR splits classic-tier (Essential/Standard/Premium) Postgres credential management out of postgres-database.ts into a new sibling module, src/resources/data/postgres-database/credentials.ts.

  • Moves postgres-database.ts to postgres-database/index.ts (renamed, now a directory). The resource/attachment/destroy logic stays here; credential-specific logic moves to the new credentials.ts.
  • Adds postgres-database/credentials.ts with createCredentials, repairDefault, deleteCredential, getUrl, and rotateCredential.
  • Updates import paths across database-credential/get-url.ts, database-credential/index.ts, internal/postgres-utils.ts/.test.ts, and link.ts to point at the new postgres-database/index.js / postgres-database/credentials.js locations.
  • Exports the new credential functions/types in extensions/data.ts from postgres-database/credentials.js, plus CredentialConnectionInfo from database-credential/index.js.
  • Adds the ./resources/data/postgres-database subpath export to package.json.
  • Updates examples/pg.ts's listCredentials call site to match the new options-object signature.

Breaking Change:

  • listCredentials now returns {addon, credentials} instead of a bare CredentialInfo[], and takes an options object ({addonIdentity, signal}) instead of a positional addonIdentity argument.

Type of Change

Breaking Changes (major semver update)

  • Add a ! after your change type to denote a change that breaks current behavior

Feature Additions (minor semver update)

  • feat: Introduces a new feature to the codebase

Patch Updates (patch semver update)

  • fix: Bug fix
  • deps: Dependency upgrade
  • revert: Revert a previous commit
  • chore: Change that does not affect production code
  • refactor: Refactoring existing code without changing behavior
  • test: Add/update/remove tests

Testing

Notes:

Steps:

  1. Passing CI suffices

Screenshots (if applicable)

Related Issues

GUS work item: W-23386493

@erika-wallace
erika-wallace requested a review from a team as a code owner November 10, 2026 21:01
@erika-wallace erika-wallace changed the title feat! add extensions for postgres database credentials feat!: add extensions for postgres database credentials Nov 10, 2026

@jdodson jdodson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey,

AI found some blockers:

  • Essential-tier regression: getUrl and rotateCredential reject named credentials only on legacy Essential databases. Current Essential databases can reach the API instead. src/resources/data/postgres-database/credentials.ts:186-187,281-283

  • Required SDK helper missing: This PR does not add data.database.listAttachments(dbInput, {appIdentity}). The existing data.postgresDatabase.listAttachments is a different, Advanced-tier-only method. src/resources/data/database.ts:113-138; existing method: src/resources/data/postgres-database/index.ts:174-188

  • Credential API arguments don’t match the requested SDK contract: Methods take appIdentity positionally and accept addonIdentity in options, rather than raw database input with {appIdentity} options. src/resources/data/postgres-database/credentials.ts:594-597,654-665

@erika-wallace

erika-wallace commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor Author
  • Essential-tier regression: getUrl and rotateCredential reject named credentials only on legacy Essential databases. Current Essential databases can reach the API instead. src/resources/data/postgres-database/credentials.ts:186-187,281-283
  • Required SDK helper missing: This PR does not add data.database.listAttachments(dbInput, {appIdentity}). The existing data.postgresDatabase.listAttachments is a different, Advanced-tier-only method. src/resources/data/database.ts:113-138; existing method: src/resources/data/postgres-database/index.ts:174-188
  • Credential API arguments don’t match the requested SDK contract: Methods take appIdentity positionally and accept addonIdentity in options, rather than raw database input with {appIdentity} options. src/resources/data/postgres-database/credentials.ts:594-597,654-665

@jdodson

Thanks for the review. This was my reasoning for each, but I can change any of them if necessary.

Essential-tier regression: The original CLI commands being migrated here (pg:credentials:url, pg:credentials:rotate) both guarded with isLegacyEssentialDatabase only, so getUrl/rotateCredential preserve that exact behavior. The broader isEssentialTier check on createCredentials/repairDefault/deleteCredential matches those commands' own original guards — each function's tier check mirrors what its specific CLI command already did.

Required SDK helper missing: data.database.listAttachments would be a thin wrapper around platform.addOnAttachment.listByAddOn to do add-on resolution + listing attachments. Resolution already happens in the deleteCredential extension, so there wasn't a need for a separate extension.

Credential API arguments don't match the requested SDK contract: resolvePgDatabase requires an app, while an add-on identity is optional. Therefore appIdentity is required in the downstream extensions, with addonIdentity in the options.

@jdodson jdodson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved

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