This is an automated email from the ASF dual-hosted git repository. diegopucci pushed a commit to branch diego/ch78628/fix-disabled-ssh-toggle in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8e9b13dd0461be8589fece8c091d5aa06c3f87a1 Author: geido <[email protected]> AuthorDate: Fri Feb 16 16:10:11 2024 +0200 Remove tunnel via database update --- .../src/features/databases/DatabaseModal/index.tsx | 33 ++++-------- superset-frontend/src/features/databases/types.ts | 6 +-- superset/commands/database/update.py | 58 +++++++++++++++------- 3 files changed, 52 insertions(+), 45 deletions(-) diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx index 98e7cab415..46b090f465 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx @@ -596,7 +596,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ const SSHTunnelSwitchComponent = extensionsRegistry.get('ssh_tunnel.form.switch') ?? SSHTunnelSwitch; - const [useSSHTunneling, setUseSSHTunneling] = useState<boolean>(false); + const [useSSHTunneling, setUseSSHTunneling] = useState<boolean | undefined>( + undefined, + ); let dbConfigExtraExtension = extensionsRegistry.get( 'databaseconnection.extraOption', @@ -724,7 +726,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ setSSHTunnelPrivateKeys({}); setSSHTunnelPrivateKeyPasswords({}); setConfirmedOverwrite(false); - setUseSSHTunneling(false); + setUseSSHTunneling(undefined); onHide(); }; @@ -850,27 +852,10 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ setLoading(true); - // tunnel config is kept until db is saved for UX convenience (toggling on and off) // strictly checking for false as an indication that the toggle got unchecked - if (isSSHTunneling && dbToUpdate?.parameters?.ssh === false) { - if (db?.id && dbFetched?.ssh_tunnel) { - // the db and ssh tunnel exist, should be removed - try { - await SupersetClient.delete({ - endpoint: `/api/v1/database/${db.id}/ssh_tunnel/`, - }); - } catch (e) { - addDangerToast( - t('There was an error removing the SSH tunnel configuration'), - ); - setLoading(false); - console.error(e); - return; - } - } - handleClearSSHTunnelConfig(); - // remove ssh tunnel from payload - dbToUpdate.ssh_tunnel = undefined; + if (isSSHTunneling && useSSHTunneling === false) { + // remove ssh tunnel + dbToUpdate.ssh_tunnel = null; } if (db?.id) { @@ -1325,8 +1310,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ }, [sshPrivateKeyPasswordNeeded]); useEffect(() => { - if (isSSHTunneling) { - setUseSSHTunneling(!!db?.parameters?.ssh); + if (isSSHTunneling && db?.parameters?.ssh !== undefined) { + setUseSSHTunneling(db?.parameters?.ssh); handleClearValidationErrors(); } }, [db?.parameters?.ssh]); diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts index dbbd661890..2f481d8315 100644 --- a/superset-frontend/src/features/databases/types.ts +++ b/superset-frontend/src/features/databases/types.ts @@ -112,7 +112,7 @@ export type DatabaseObject = { }; // SSH Tunnel information - ssh_tunnel?: SSHTunnelObject; + ssh_tunnel?: SSHTunnelObject | null; }; export type DatabaseForm = { @@ -236,8 +236,8 @@ export interface ExtraJson { version?: string; } -type ParametersChangeValueType = HTMLInputElement & { - value: string | boolean; +type ParametersChangeValueType = Partial<Omit<HTMLInputElement, 'value'>> & { + value?: string | boolean; }; type ParametersChangeType<T = ParametersChangeValueType> = diff --git a/superset/commands/database/update.py b/superset/commands/database/update.py index edc0ba1b98..b891c8f157 100644 --- a/superset/commands/database/update.py +++ b/superset/commands/database/update.py @@ -30,8 +30,10 @@ from superset.commands.database.exceptions import ( DatabaseUpdateFailedError, ) from superset.commands.database.ssh_tunnel.create import CreateSSHTunnelCommand +from superset.commands.database.ssh_tunnel.delete import DeleteSSHTunnelCommand from superset.commands.database.ssh_tunnel.exceptions import ( SSHTunnelCreateFailedError, + SSHTunnelDeleteFailedError, SSHTunnelingNotEnabledError, SSHTunnelInvalidError, SSHTunnelUpdateFailedError, @@ -70,32 +72,52 @@ class UpdateDatabaseCommand(BaseCommand): database = DatabaseDAO.update(self._model, self._properties, commit=False) database.set_sqlalchemy_uri(database.sqlalchemy_uri) - if ssh_tunnel_properties := self._properties.get("ssh_tunnel"): + existing_ssh_tunnel_model = DatabaseDAO.get_ssh_tunnel(database.id) + + if "ssh_tunnel" in self._properties: if not is_feature_enabled("SSH_TUNNELING"): db.session.rollback() raise SSHTunnelingNotEnabledError() - existing_ssh_tunnel_model = DatabaseDAO.get_ssh_tunnel(database.id) - if existing_ssh_tunnel_model is None: - # We couldn't found an existing tunnel so we need to create one - try: - CreateSSHTunnelCommand(database, ssh_tunnel_properties).run() - except (SSHTunnelInvalidError, SSHTunnelCreateFailedError) as ex: - # So we can show the original message - raise ex - except Exception as ex: - raise DatabaseUpdateFailedError() from ex - else: - # We found an existing tunnel so we need to update it + + if not self._properties.get("ssh_tunnel") and existing_ssh_tunnel_model: + # We need to remove the existing tunnel try: - UpdateSSHTunnelCommand( - existing_ssh_tunnel_model.id, ssh_tunnel_properties - ).run() - except (SSHTunnelInvalidError, SSHTunnelUpdateFailedError) as ex: - # So we can show the original message + DeleteSSHTunnelCommand(existing_ssh_tunnel_model.id).run() + except SSHTunnelDeleteFailedError as ex: raise ex except Exception as ex: raise DatabaseUpdateFailedError() from ex + if ssh_tunnel_properties := self._properties.get("ssh_tunnel"): + if existing_ssh_tunnel_model is None: + # We couldn't found an existing tunnel so we need to create one + try: + CreateSSHTunnelCommand( + database, ssh_tunnel_properties + ).run() + except ( + SSHTunnelInvalidError, + SSHTunnelCreateFailedError, + ) as ex: + # So we can show the original message + raise ex + except Exception as ex: + raise DatabaseUpdateFailedError() from ex + else: + # We found an existing tunnel so we need to update it + try: + UpdateSSHTunnelCommand( + existing_ssh_tunnel_model.id, ssh_tunnel_properties + ).run() + except ( + SSHTunnelInvalidError, + SSHTunnelUpdateFailedError, + ) as ex: + # So we can show the original message + raise ex + except Exception as ex: + raise DatabaseUpdateFailedError() from ex + # adding a new database we always want to force refresh schema list # TODO Improve this simplistic implementation for catching DB conn fails try:
