You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2023/07/03 03:48:56 UTC
[superset] branch master updated: fix: SSH Tunnel creation with dynamic form (#24196)
This is an automated email from the ASF dual-hosted git repository.
hugh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 226c7f807d fix: SSH Tunnel creation with dynamic form (#24196)
226c7f807d is described below
commit 226c7f807dd70239691dc3baaa4d4276a6a4f7c4
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Sun Jul 2 23:48:48 2023 -0400
fix: SSH Tunnel creation with dynamic form (#24196)
---
.../cypress/e2e/database/modal.test.ts | 8 +-
.../DatabaseConnectionForm/CommonParameters.tsx | 31 +++
.../DatabaseModal/DatabaseConnectionForm/index.tsx | 3 +
.../databases/DatabaseModal/index.test.tsx | 4 +
.../src/features/databases/DatabaseModal/index.tsx | 30 ++-
superset-frontend/src/features/databases/types.ts | 1 +
superset-frontend/src/types/Database.ts | 1 +
superset-frontend/src/views/CRUD/hooks.ts | 209 +++++++++++----------
superset/db_engine_specs/base.py | 4 +
tests/integration_tests/databases/api_tests.py | 12 ++
.../db_engine_specs/postgres_tests.py | 4 +
11 files changed, 184 insertions(+), 123 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts b/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts
index a3260250aa..3cc34cb64f 100644
--- a/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/database/modal.test.ts
@@ -62,8 +62,8 @@ describe('Add database', () => {
it('show error alerts on dynamic form for bad host', () => {
// click postgres dynamic form
cy.get('.preferred > :nth-child(1)').click();
- cy.get('input[name="host"]').focus().type('badhost');
- cy.get('input[name="port"]').focus().type('5432');
+ cy.get('input[name="host"]').focus().type('badhost', { force: true });
+ cy.get('input[name="port"]').focus().type('5432', { force: true });
cy.get('.ant-form-item-explain-error').contains(
"The hostname provided can't be resolved",
);
@@ -72,8 +72,8 @@ describe('Add database', () => {
it('show error alerts on dynamic form for bad port', () => {
// click postgres dynamic form
cy.get('.preferred > :nth-child(1)').click();
- cy.get('input[name="host"]').focus().type('localhost');
- cy.get('input[name="port"]').focus().type('123');
+ cy.get('input[name="host"]').focus().type('localhost', { force: true });
+ cy.get('input[name="port"]').focus().type('123', { force: true });
cy.get('input[name="database"]').focus();
cy.get('.ant-form-item-explain-error').contains('The port is closed');
});
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx
index 99a414012b..7078b49cbc 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import React from 'react';
+import { isEmpty } from 'lodash';
import { SupersetTheme, t } from '@superset-ui/core';
import { AntdSwitch } from 'src/components';
import InfoTooltip from 'src/components/InfoTooltip';
@@ -250,3 +251,33 @@ export const forceSSLField = ({
/>
</div>
);
+
+export const SSHTunnelSwitch = ({
+ isEditMode,
+ changeMethods,
+ db,
+}: FieldPropTypes) => (
+ <div css={(theme: SupersetTheme) => infoTooltip(theme)}>
+ <AntdSwitch
+ disabled={isEditMode && !isEmpty(db?.ssh_tunnel)}
+ checked={db?.parameters?.ssh}
+ onChange={changed => {
+ changeMethods.onParametersChange({
+ target: {
+ type: 'toggle',
+ name: 'ssh',
+ checked: true,
+ value: changed,
+ },
+ });
+ }}
+ data-test="ssh-tunnel-switch"
+ />
+ <span css={toggleStyle}>{t('SSH Tunnel')}</span>
+ <InfoTooltip
+ tooltip={t('SSH Tunnel configuration parameters')}
+ placement="right"
+ viewBox="0 -5 24 24"
+ />
+ </div>
+);
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx
index f6284ae67d..5dce73206f 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx
@@ -31,6 +31,7 @@ import {
portField,
queryField,
usernameField,
+ SSHTunnelSwitch,
} from './CommonParameters';
import { validatedInputField } from './ValidatedInputField';
import { EncryptedField } from './EncryptedField';
@@ -55,6 +56,7 @@ export const FormFieldOrder = [
'account',
'warehouse',
'role',
+ 'ssh',
];
export interface FieldPropTypes {
@@ -102,6 +104,7 @@ const FORM_FIELD_MAP = {
warehouse: validatedInputField,
role: validatedInputField,
account: validatedInputField,
+ ssh: SSHTunnelSwitch,
};
interface DatabaseConnectionFormProps {
diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx
index bcff047be3..385b771efe 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/index.test.tsx
@@ -124,6 +124,10 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
description: 'Additional parameters',
type: 'object',
},
+ ssh: {
+ description: 'Create SSH Tunnel',
+ type: 'boolean',
+ },
username: {
description: 'Username',
nullable: true,
diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
index 25fbadd3e1..1861cc46ed 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
@@ -763,15 +763,18 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
});
}
- // make sure that button spinner animates
- setLoading(true);
- const errors = await getValidation(dbToUpdate, true);
- if ((validationErrors && !isEmpty(validationErrors)) || errors) {
+ // only do validation for non ssh tunnel connections
+ if (!dbToUpdate?.ssh_tunnel) {
+ // make sure that button spinner animates
+ setLoading(true);
+ const errors = await getValidation(dbToUpdate, true);
+ if ((validationErrors && !isEmpty(validationErrors)) || errors) {
+ setLoading(false);
+ return;
+ }
+ // end spinner animation
setLoading(false);
- return;
}
- setLoading(false);
- // end spinner animation
const parameters_schema = isEditMode
? dbToUpdate.parameters_schema?.properties
@@ -1633,18 +1636,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
validationErrors={validationErrors}
getPlaceholder={getPlaceholder}
/>
- <SSHTunnelContainer>
- <SSHTunnelSwitchComponent
- isEditMode={isEditMode}
- dbFetched={dbFetched}
- disableSSHTunnelingForEngine={disableSSHTunnelingForEngine}
- useSSHTunneling={useSSHTunneling}
- setUseSSHTunneling={setUseSSHTunneling}
- setDB={setDB}
- isSSHTunneling={isSSHTunneling}
- />
- </SSHTunnelContainer>
- {useSSHTunneling && (
+ {db?.parameters?.ssh && (
<SSHTunnelContainer>{renderSSHTunnelForm()}</SSHTunnelContainer>
)}
</>
diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts
index 7b92819fdb..a7e4f59b58 100644
--- a/superset-frontend/src/features/databases/types.ts
+++ b/superset-frontend/src/features/databases/types.ts
@@ -69,6 +69,7 @@ export type DatabaseObject = {
warehouse?: string;
role?: string;
account?: string;
+ ssh?: boolean;
};
// Performance
diff --git a/superset-frontend/src/types/Database.ts b/superset-frontend/src/types/Database.ts
index c4491dbb99..575d69e2f2 100644
--- a/superset-frontend/src/types/Database.ts
+++ b/superset-frontend/src/types/Database.ts
@@ -27,4 +27,5 @@ export default interface Database {
server_cert: string;
sqlalchemy_uri: string;
catalog: object;
+ parameters: any;
}
diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts
index 7947494f31..b53731754f 100644
--- a/superset-frontend/src/views/CRUD/hooks.ts
+++ b/superset-frontend/src/views/CRUD/hooks.ts
@@ -740,123 +740,132 @@ export function useDatabaseValidation() {
null,
);
const getValidation = useCallback(
- (database: Partial<DatabaseObject> | null, onCreate = false) =>
- SupersetClient.post({
- endpoint: '/api/v1/database/validate_parameters/',
- body: JSON.stringify(transformDB(database)),
- headers: { 'Content-Type': 'application/json' },
- })
- .then(() => {
- setValidationErrors(null);
+ (database: Partial<DatabaseObject> | null, onCreate = false) => {
+ if (database?.parameters?.ssh) {
+ // when ssh tunnel is enabled we don't want to render any validation errors
+ setValidationErrors(null);
+ return [];
+ }
+
+ return (
+ SupersetClient.post({
+ endpoint: '/api/v1/database/validate_parameters/',
+ body: JSON.stringify(transformDB(database)),
+ headers: { 'Content-Type': 'application/json' },
})
- // eslint-disable-next-line consistent-return
- .catch(e => {
- if (typeof e.json === 'function') {
- return e.json().then(({ errors = [] }: JsonObject) => {
- const parsedErrors = errors
- .filter((error: { error_type: string }) => {
- const skipValidationError = ![
- 'CONNECTION_MISSING_PARAMETERS_ERROR',
- 'CONNECTION_ACCESS_DENIED_ERROR',
- ].includes(error.error_type);
- return skipValidationError || onCreate;
- })
- .reduce(
- (
- obj: {},
- {
- error_type,
- extra,
- message,
- }: {
- error_type: string;
- extra: {
- invalid?: string[];
- missing?: string[];
- name: string;
- catalog: {
+ .then(() => {
+ setValidationErrors(null);
+ })
+ // eslint-disable-next-line consistent-return
+ .catch(e => {
+ if (typeof e.json === 'function') {
+ return e.json().then(({ errors = [] }: JsonObject) => {
+ const parsedErrors = errors
+ .filter((error: { error_type: string }) => {
+ const skipValidationError = ![
+ 'CONNECTION_MISSING_PARAMETERS_ERROR',
+ 'CONNECTION_ACCESS_DENIED_ERROR',
+ ].includes(error.error_type);
+ return skipValidationError || onCreate;
+ })
+ .reduce(
+ (
+ obj: {},
+ {
+ error_type,
+ extra,
+ message,
+ }: {
+ error_type: string;
+ extra: {
+ invalid?: string[];
+ missing?: string[];
name: string;
- url: string;
- idx: number;
+ catalog: {
+ name: string;
+ url: string;
+ idx: number;
+ };
+ issue_codes?: {
+ code?: number;
+ message?: string;
+ }[];
};
- issue_codes?: {
- code?: number;
- message?: string;
- }[];
- };
- message: string;
- },
- ) => {
- if (extra.catalog) {
- if (extra.catalog.name) {
+ message: string;
+ },
+ ) => {
+ if (extra.catalog) {
+ if (extra.catalog.name) {
+ return {
+ ...obj,
+ error_type,
+ [extra.catalog.idx]: {
+ name: message,
+ },
+ };
+ }
+ if (extra.catalog.url) {
+ return {
+ ...obj,
+ error_type,
+ [extra.catalog.idx]: {
+ url: message,
+ },
+ };
+ }
+
return {
...obj,
error_type,
[extra.catalog.idx]: {
name: message,
+ url: message,
},
};
}
- if (extra.catalog.url) {
+ // if extra.invalid doesn't exist then the
+ // error can't be mapped to a parameter
+ // so leave it alone
+ if (extra.invalid) {
return {
...obj,
+ [extra.invalid[0]]: message,
error_type,
- [extra.catalog.idx]: {
- url: message,
- },
+ };
+ }
+ if (extra.missing) {
+ return {
+ ...obj,
+ error_type,
+ ...Object.assign(
+ {},
+ ...extra.missing.map(field => ({
+ [field]: 'This is a required field',
+ })),
+ ),
+ };
+ }
+ if (extra.issue_codes?.length) {
+ return {
+ ...obj,
+ error_type,
+ description: message || extra.issue_codes[0]?.message,
};
}
- return {
- ...obj,
- error_type,
- [extra.catalog.idx]: {
- name: message,
- url: message,
- },
- };
- }
- // if extra.invalid doesn't exist then the
- // error can't be mapped to a parameter
- // so leave it alone
- if (extra.invalid) {
- return {
- ...obj,
- [extra.invalid[0]]: message,
- error_type,
- };
- }
- if (extra.missing) {
- return {
- ...obj,
- error_type,
- ...Object.assign(
- {},
- ...extra.missing.map(field => ({
- [field]: 'This is a required field',
- })),
- ),
- };
- }
- if (extra.issue_codes?.length) {
- return {
- ...obj,
- error_type,
- description: message || extra.issue_codes[0]?.message,
- };
- }
-
- return obj;
- },
- {},
- );
- setValidationErrors(parsedErrors);
- return parsedErrors;
- });
- }
- // eslint-disable-next-line no-console
- console.error(e);
- }),
+ return obj;
+ },
+ {},
+ );
+ setValidationErrors(parsedErrors);
+ return parsedErrors;
+ });
+ }
+ // eslint-disable-next-line no-console
+ console.error(e);
+ })
+ );
+ },
[setValidationErrors],
);
diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index d117766754..0bacf30fa6 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -1891,6 +1891,10 @@ class BasicParametersSchema(Schema):
required=False,
metadata={"description": __("Use an encrypted connection to the database")},
)
+ ssh = fields.Boolean(
+ required=False,
+ metadata={"description": __("Use an ssh tunnel connection to the database")},
+ )
class BasicParametersType(TypedDict, total=False):
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index f2fe9380c3..568ba05934 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -2888,6 +2888,10 @@ class TestDatabaseApi(SupersetTestCase):
"description": "Additional parameters",
"type": "object",
},
+ "ssh": {
+ "description": "Use an ssh tunnel connection to the database",
+ "type": "boolean",
+ },
"username": {
"description": "Username",
"nullable": True,
@@ -2962,6 +2966,10 @@ class TestDatabaseApi(SupersetTestCase):
"description": "Additional parameters",
"type": "object",
},
+ "ssh": {
+ "description": "Use an ssh tunnel connection to the database",
+ "type": "boolean",
+ },
"username": {
"description": "Username",
"nullable": True,
@@ -3036,6 +3044,10 @@ class TestDatabaseApi(SupersetTestCase):
"description": "Additional parameters",
"type": "object",
},
+ "ssh": {
+ "description": "Use an ssh tunnel connection to the database",
+ "type": "boolean",
+ },
"username": {
"description": "Username",
"nullable": True,
diff --git a/tests/integration_tests/db_engine_specs/postgres_tests.py b/tests/integration_tests/db_engine_specs/postgres_tests.py
index 945e0667fc..2b543e8e25 100644
--- a/tests/integration_tests/db_engine_specs/postgres_tests.py
+++ b/tests/integration_tests/db_engine_specs/postgres_tests.py
@@ -512,6 +512,10 @@ def test_base_parameters_mixin():
"description": "Additional parameters",
"additionalProperties": {},
},
+ "ssh": {
+ "description": "Use an ssh tunnel connection to the database",
+ "type": "boolean",
+ },
},
"required": ["database", "host", "port", "username"],
}