fix: SSH Tunnel creation with dynamic form (apache#24196)

preset-io · Jul 3, 2023 · 226c7f8 · 226c7f8
1 parent 946aacc
commit 226c7f8
Show file tree

Hide file tree

Showing 11 changed files with 184 additions and 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
@@ -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/...frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/...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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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],
   );