diff --git a/apps/studio/components/interfaces/Integrations/Wrappers/DeleteWrapperModal.tsx b/apps/studio/components/interfaces/Integrations/Wrappers/DeleteWrapperModal.tsx index ca7418dcfd1..d6c35b2906d 100644 --- a/apps/studio/components/interfaces/Integrations/Wrappers/DeleteWrapperModal.tsx +++ b/apps/studio/components/interfaces/Integrations/Wrappers/DeleteWrapperModal.tsx @@ -45,7 +45,7 @@ export const DeleteWrapperModal = () => { const { mutateAsync: deleteFDW, isSuccess: isSuccessDelete } = useFDWDeleteMutation({ onSuccess: () => { - toast.success(`Successfully disabled ${selectedWrapper?.name} foreign data wrapper`) + toast.success('Connection deleted') setSelectedWrapperToDelete(null) }, }) @@ -84,16 +84,16 @@ export const DeleteWrapperModal = () => { > - {`Confirm to disable ${selectedWrapper?.name}`} + {`Delete connection ${selectedWrapper?.server_name}?`} - Are you sure you want to disable {selectedWrapper?.name}? This will also remove all - tables created with this wrapper. + This deletes this connection and its foreign tables. If this is the last connection + using the wrapper, its Vault secret and wrapper are also removed. Cancel - Confirm + Delete connection diff --git a/apps/studio/components/interfaces/Integrations/Wrappers/WrapperRow.tsx b/apps/studio/components/interfaces/Integrations/Wrappers/WrapperRow.tsx index 5ef2f2a5c7e..2fbcbdef192 100644 --- a/apps/studio/components/interfaces/Integrations/Wrappers/WrapperRow.tsx +++ b/apps/studio/components/interfaces/Integrations/Wrappers/WrapperRow.tsx @@ -14,9 +14,10 @@ import { useAsyncCheckPermissions } from '@/hooks/misc/useCheckPermissions' interface WrapperRowProps { wrapper: FDW + isShared: boolean } -export const WrapperRow = ({ wrapper }: WrapperRowProps) => { +export const WrapperRow = ({ wrapper, isShared }: WrapperRowProps) => { const { ref, id } = useParams() const { can: canManageWrappers } = useAsyncCheckPermissions( PermissionAction.TENANT_SQL_ADMIN_WRITE, @@ -39,11 +40,45 @@ export const WrapperRow = ({ wrapper }: WrapperRowProps) => { ) const _tables = formatWrapperTables(wrapper, integration?.meta) + const canEdit = canManageWrappers && !isShared + let editTooltip = 'Edit wrapper' + if (!canManageWrappers) editTooltip = 'You need additional permissions to edit wrappers' + else if (isShared) editTooltip = 'Shared wrappers cannot be edited in the dashboard' return ( {wrapper.name} +

+ Connection: {wrapper.server_name} +

+ {isShared && ( +

+ This wrapper is shared. To edit this connection, use ALTER SERVER on{' '} + {wrapper.server_name} or{' '} + ALTER FOREIGN TABLE in the{' '} + + SQL Editor + + . + {encryptedMetadata.length > 0 && ( + <> + {' '} + Edit this server's credentials in{' '} + + Vault + + . Changes to a secret used by other connections affect them too. + + )} +

+ )} {visibleMetadata.map((metadata) => (
{
} className="px-1.5" onClick={() => setSelectedWrapperToEdit(wrapper.id.toString())} tooltip={{ content: { side: 'bottom', - text: !canManageWrappers - ? 'You need additional permissions to edit wrappers' - : 'Edit wrapper', + text: editTooltip, }, }} /> @@ -145,7 +178,7 @@ export const WrapperRow = ({ wrapper }: WrapperRowProps) => { side: 'bottom', text: !canManageWrappers ? 'You need additional permissions to delete wrappers' - : 'Delete wrapper', + : 'Delete connection', }, }} /> diff --git a/apps/studio/components/interfaces/Integrations/Wrappers/WrapperTable.tsx b/apps/studio/components/interfaces/Integrations/Wrappers/WrapperTable.tsx index c458bd96eb9..83e840e1f32 100644 --- a/apps/studio/components/interfaces/Integrations/Wrappers/WrapperTable.tsx +++ b/apps/studio/components/interfaces/Integrations/Wrappers/WrapperTable.tsx @@ -1,6 +1,6 @@ import { useParams } from 'common' import { parseAsString, useQueryState } from 'nuqs' -import { useEffect, useMemo, useState } from 'react' +import { useEffect, useMemo, useRef, useState } from 'react' import { toast } from 'sonner' import { Card, @@ -35,7 +35,7 @@ export const WrapperTable = ({ isLatest = false }: WrapperTableProps) => { const [isClosingEditWrapper, setIsClosingEditWrapper] = useState(false) - const { data, isError } = useFDWsQuery({ + const { data, isError, isSuccess } = useFDWsQuery({ projectRef: ref, connectionString: project?.connectionString, }) @@ -49,14 +49,36 @@ export const WrapperTable = ({ isLatest = false }: WrapperTableProps) => { ) const [selectedWrapperIdToEdit, setSelectedWrapperToEdit] = useQueryState('edit', parseAsString) - const selectedWrapperToEdit = wrappers.find((w) => w.id.toString() === selectedWrapperIdToEdit) + const isSharedWrapper = (wrapper: (typeof wrappers)[number]) => + data?.some((other) => other.id !== wrapper.id && other.name === wrapper.name) ?? false + const selectedWrapper = wrappers.find((w) => w.id.toString() === selectedWrapperIdToEdit) + const isSelectedWrapperShared = selectedWrapper !== undefined && isSharedWrapper(selectedWrapper) + const selectedWrapperToEdit = isSelectedWrapperShared ? undefined : selectedWrapper + const openedWrapperId = useRef(null) useEffect(() => { - if (isError && !!selectedWrapperIdToEdit && !selectedWrapperToEdit) { - toast('Wrapper not found') + if (!selectedWrapperIdToEdit) { + openedWrapperId.current = null + } else if (selectedWrapperToEdit) { + openedWrapperId.current = selectedWrapperIdToEdit + } else if (isSuccess || isError) { + if (openedWrapperId.current !== selectedWrapperIdToEdit) { + toast( + isSelectedWrapperShared + ? 'Shared wrappers cannot be edited in the dashboard. Use the SQL Editor to edit this connection.' + : 'Wrapper not found' + ) + } setSelectedWrapperToEdit(null) } - }, [isError, selectedWrapperIdToEdit, selectedWrapperToEdit, setSelectedWrapperToEdit]) + }, [ + isError, + isSelectedWrapperShared, + isSuccess, + selectedWrapperIdToEdit, + selectedWrapperToEdit, + setSelectedWrapperToEdit, + ]) if (!integration || integration.type !== 'wrapper') { return ( @@ -81,9 +103,9 @@ export const WrapperTable = ({ isLatest = false }: WrapperTableProps) => { - {(isLatest ? wrappers.slice(0, 3) : wrappers).map((x) => { - return - })} + {(isLatest ? wrappers.slice(0, 3) : wrappers).map((x) => ( + + ))} key !== 'table_name' && key !== 'schema_name' && + key !== 'schema' && + key !== 'id' && key !== 'columns' && key !== 'index' && key !== 'is_new_schema' && @@ -298,7 +300,7 @@ export const getDeleteFDWSql = ({ wrapper, wrapperMeta, }: { - wrapper: { name: string } + wrapper: { id: number; name: string; server_name: string } wrapperMeta: SimplifiedWrapperMeta }): SafeSqlFragment => { const encryptedOptions = wrapperMeta.server.options.filter((option) => option.encrypted) @@ -309,41 +311,45 @@ export const getDeleteFDWSql = ({ return safeSql` do $$ begin - -- Old wrappers has an implicit dependency on pgsodium. For new wrappers - -- we use Vault directly. - if (select extversion from pg_extension where extname = 'wrappers') in ( - '0.1.0', - '0.1.1', - '0.1.4', - '0.1.5', - '0.1.6', - '0.1.7', - '0.1.8', - '0.1.9', - '0.1.10', - '0.1.11', - '0.1.12', - '0.1.14', - '0.1.15', - '0.1.16', - '0.1.17', - '0.1.18', - '0.1.19', - '0.2.0', - '0.3.0', - '0.3.1', - '0.4.0', - '0.4.1', - '0.4.2', - '0.4.3', - '0.4.4', - '0.4.5' + if not exists ( + select 1 from pg_catalog.pg_foreign_data_wrapper where fdwname = ${literal(wrapper.name)} ) then - delete from vault.secrets where key_id = (select id from pgsodium.valid_key where name = ${literal(key)}); + -- Old wrappers has an implicit dependency on pgsodium. For new wrappers + -- we use Vault directly. + if (select extversion from pg_extension where extname = 'wrappers') in ( + '0.1.0', + '0.1.1', + '0.1.4', + '0.1.5', + '0.1.6', + '0.1.7', + '0.1.8', + '0.1.9', + '0.1.10', + '0.1.11', + '0.1.12', + '0.1.14', + '0.1.15', + '0.1.16', + '0.1.17', + '0.1.18', + '0.1.19', + '0.2.0', + '0.3.0', + '0.3.1', + '0.4.0', + '0.4.1', + '0.4.2', + '0.4.3', + '0.4.4', + '0.4.5' + ) then + delete from vault.secrets where key_id = (select id from pgsodium.valid_key where name = ${literal(key)}); - delete from pgsodium.key where name = ${literal(key)}; - else - delete from vault.secrets where name = ${literal(key)}; + delete from pgsodium.key where name = ${literal(key)}; + else + delete from vault.secrets where name = ${literal(key)}; + end if; end if; end $$; ` @@ -351,8 +357,37 @@ export const getDeleteFDWSql = ({ const deleteEncryptedSecretsSql = joinSqlFragments(deleteEncryptedSecretsSqlArray, '\n') + const ensureSelectedServerSql = safeSql` + begin + if not exists ( + select 1 + from pg_catalog.pg_foreign_server s + join pg_catalog.pg_foreign_data_wrapper w on w.oid = s.srvfdw + where s.oid = ${literal(wrapper.id)} + and s.srvname = ${literal(wrapper.server_name)} + and w.fdwname = ${literal(wrapper.name)} + ) then + raise exception 'The selected foreign server no longer belongs to this wrapper.'; + end if; + end + ` + const sql = safeSql` - drop foreign data wrapper if exists ${ident(wrapper.name)} cascade; + do ${literal(ensureSelectedServerSql)}; + + drop server if exists ${ident(wrapper.server_name)} cascade; + + do $$ + begin + if not exists ( + select 1 + from pg_catalog.pg_foreign_server s + join pg_catalog.pg_foreign_data_wrapper w on w.oid = s.srvfdw + where w.fdwname = ${literal(wrapper.name)} + ) then + execute format('drop foreign data wrapper if exists %I cascade', ${literal(wrapper.name)}); + end if; + end $$; ${deleteEncryptedSecretsSql} ` @@ -366,11 +401,25 @@ export const getUpdateFDWSql = ({ formState, tables, }: { - wrapper: { name: string } + wrapper: { id: number; name: string; server_name: string } wrapperMeta: SimplifiedWrapperMeta formState: { [k: string]: string } tables: any[] }): SafeSqlFragment => { + const ensureWrapperIsNotSharedSql = safeSql` + do $$ + begin + if exists ( + select 1 + from pg_catalog.pg_foreign_server s + join pg_catalog.pg_foreign_data_wrapper w on w.oid = s.srvfdw + where w.fdwname = ${literal(wrapper.name)} + and s.srvname <> ${literal(wrapper.server_name)} + ) then + raise exception 'This wrapper is used by another server and cannot be edited here.'; + end if; + end $$; + ` const deleteWrapperSql = getDeleteFDWSql({ wrapper, wrapperMeta }) const createWrapperSql = getCreateFDWSql({ wrapperMeta, @@ -382,6 +431,8 @@ export const getUpdateFDWSql = ({ }) const sql = safeSql` + ${ensureWrapperIsNotSharedSql} + ${deleteWrapperSql} ${createWrapperSql} diff --git a/packages/pg-meta/test/sql/studio/fdw.test.ts b/packages/pg-meta/test/sql/studio/fdw.test.ts index bf8613b4c4a..741c6f9c3de 100644 --- a/packages/pg-meta/test/sql/studio/fdw.test.ts +++ b/packages/pg-meta/test/sql/studio/fdw.test.ts @@ -1,6 +1,6 @@ import { expect, test } from 'vitest' -import { getCreateFDWSql } from '../../../src' +import { getCreateFDWSql, getDeleteFDWSql, getUpdateFDWSql } from '../../../src' import { literal } from '../../../src/pg-format' const baseArgs = { @@ -54,3 +54,75 @@ test('encrypted server options still resolve their value through Vault unchanged expect(sql).toContain("api_secret ''%s''") expect(sql).toContain('vault.create_secret') }) + +test('deleting a wrapper row drops its server and preserves shared wrapper dependencies', () => { + const sql = getDeleteFDWSql({ + wrapper: { id: 42, name: 'bigquery_fdw', server_name: 'selected_bigquery_server' }, + wrapperMeta: { + handlerName: 'big_query_fdw_handler', + validatorName: 'big_query_fdw_validator', + server: { options: [{ name: 'sa_key_id', encrypted: true }] }, + }, + }) + + expect(sql).toContain('where s.oid = 42') + expect(sql).toContain("and s.srvname = ''selected_bigquery_server''") + expect(sql).toContain("and w.fdwname = ''bigquery_fdw''") + expect(sql.indexOf('raise exception')).toBeLessThan(sql.indexOf('drop server')) + expect(sql).toContain('drop server if exists selected_bigquery_server cascade') + expect(sql).toContain("where w.fdwname = 'bigquery_fdw'") + expect(sql).toContain("execute format('drop foreign data wrapper if exists %I cascade'") + expect(sql).not.toContain('drop foreign data wrapper if exists "bigquery_fdw" cascade') + expect(sql).toContain("where fdwname = 'bigquery_fdw'") +}) + +test('editing a shared wrapper fails before any server is dropped', () => { + const sql = getUpdateFDWSql({ + wrapper: { id: 42, name: 'bigquery_fdw', server_name: 'selected_bigquery_server' }, + wrapperMeta: { + handlerName: 'big_query_fdw_handler', + validatorName: 'big_query_fdw_validator', + server: { options: [] }, + }, + formState: { wrapper_name: 'bigquery_fdw', server_name: 'selected_bigquery_server' }, + tables: [], + }) + + expect(sql).toContain("s.srvname <> 'selected_bigquery_server'") + expect(sql.indexOf('raise exception')).toBeLessThan(sql.indexOf('drop server')) +}) + +test('editing an existing foreign table excludes catalog fields from its options', () => { + const sql = getUpdateFDWSql({ + wrapper: { id: 42, name: 'bigquery_fdw', server_name: 'bigquery_server' }, + wrapperMeta: { + handlerName: 'big_query_fdw_handler', + validatorName: 'big_query_fdw_validator', + server: { options: [{ name: 'project_id', encrypted: false }] }, + }, + formState: { + wrapper_name: 'bigquery_fdw', + server_name: 'bigquery_server', + project_id: 'example-project', + }, + tables: [ + { + id: 171639, + schema: 'qa_bq', + schema_name: 'qa_bq', + table_name: 'orders', + columns: [{ name: 'id', type: 'text' }], + is_new_schema: false, + index: 0, + table: 'orders', + location: 'US', + }, + ], + }) + + expect(sql).toContain('create foreign table qa_bq.orders') + expect(sql).toContain('"table" \'orders\'') + expect(sql).toContain("location 'US'") + expect(sql).not.toContain('id 171639') + expect(sql).not.toContain("schema 'qa_bq'") +})