feat(ssh_tunnel): SQLAlchemy Form UI #22513

Antonio-RiveroMartnez · 2022-12-22T18:01:49Z

SUMMARY

Add our a new SSHTunnelForm component that will be used to set tunnels for our Databases, right now only works when using SQLAlchemy URIs.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Make sure the SSH_TUNNELING feature flag is enabled
open the create database modal
click on SQL Alchemy link so the SQLA form is displayed
Enable the new SSH Tunnel switch
Make use of the form so you can connect a DB with a SSH Tunnel attached to it

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

…te-sshtunnelconfig-tbl

…erset into create-sshtunnelconfig-tbl

- Add visual test to confirm the new SSH Tunnel form fields are present if we click the toggle button

- Add extra UI tests for our SSH Tunnel Form

- Add new test to consider changing the login method used in SSH Tunnel Form

hughhhh · 2023-01-05T21:21:47Z

superset/databases/ssh_tunnel/dao.py

+        The masked values should be unmasked before the ssh tunnel is updated.
+        """
+        if properties.get("password") == PASSWORD_MASK:
+            properties["password"] = model.password


we should move this logic to the frontend, basically if the field hasn't been updated we shouldn't send it in the payload

Yup, our API supports PATCH kind of updates, however, the response of the GET already has the masked password thus it's stored in the reducer, so when calling the API to update let's say the server_port, we send the whole ssh_tunnel object, not just updated properties. Because of that, the API would assume you're trying to update the password when you were not. Pretty much like we are doing for other masked values in our forms.

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

- Hide the entire form behind the feature flag

- Fix TestConnection when editing a DB with SSH Tunnel attached to it - Add new unmask_password_info util method for SSH Tunnels

- Add ssh_tunnel id in our response for the GET endpoint of DB so we can call test_connection endpoint without depending on DB's name

hughhhh · 2023-01-10T23:33:15Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm/index.tsx

@@ -73,6 +73,7 @@ export interface FieldPropTypes {
    onRemoveTableCatalog: (idx: number) => void;


also we need to check the available endpoint to see if a given engine allows ssh tunneling
https://github.com/apache/superset/blob/master/superset/db_engine_specs/base.py#L196

Oh! Ok, yeah, IIRC there a engine_information property we are getting in the UI with the supports_file_upload, so we might need to include this one there and use it. Thanks!

- Use the allow_ssh_tunneling flag from the db_engine along with the feature flag to display the SSH Tunnel form, so, if a DB engine does not support SSH Tunnels, nothing gets displayed. Right now, only Postgres does

- Fix tests with proper flag values

- More fixes to tests

hughhhh

LGTM

- Fix test

hughhhh added 30 commits October 19, 2022 19:27

save

830a283

create migration

2c1e736

created schema and rename

f78df83

linting

d482df4

fix encrpytions

9edb581

remove map tabl

da27d8f

fix linting

773a6c8

add constraint

2f2dda2

add fk to migration

fd0d7f2

init

158da8d

update all the examples

face73f

change remaining bits

95d079e

add id

d5926e3

use factory instead

f7a6a41

Merge branch 'master' of https://github.com/apache/superset into crea…

30e380a

…te-sshtunnelconfig-tbl

Merge branch 'master' into ref-get-sqla-engine-2

87c0d79

fix confict

11b240b

fix conflict

1bfdbda

setup return value for contextmanager

4146d5a

add sshtunnel pip

f8b877d

updates test

54fc147

fix linting

fdc6ca3

renaming function

66c0801

fix test

1f9ec5e

Merge branch 'ref-get-sqla-engine-2' of https://github.com/apache/sup…

c698cf4

…erset into create-sshtunnelconfig-tbl

add schema to test_connection api

41bd19b

fix get engine to return contextmanager

8811a99

why

82d7532

yerp

1f829ac

update typing

d53d116

Antonio-RiveroMartnez requested a review from a team as a code owner January 3, 2023 22:32

pull-request-size bot added size/XXL and removed size/XL labels Jan 3, 2023

Merge master

27587ec

pull-request-size bot added size/XL and removed size/XXL labels Jan 3, 2023

Antonio-RiveroMartnez added 5 commits January 4, 2023 15:49

SSH Tunnel UI:

1cb7384

- Add visual test to confirm the new SSH Tunnel form fields are present if we click the toggle button

Merge branch 'master' into ssh_sqla_form

86db39d

SSH Tunnel UI:

6a03887

- Add extra UI tests for our SSH Tunnel Form

SSH Tunnel UI:

ff5ec7f

- Add new test to consider changing the login method used in SSH Tunnel Form

Merge branch 'master' into ssh_sqla_form

213f57a

hughhhh reviewed Jan 5, 2023

View reviewed changes

hughhhh reviewed Jan 9, 2023

View reviewed changes

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx Outdated Show resolved Hide resolved

Antonio-RiveroMartnez added 4 commits January 10, 2023 10:11

Merge branch 'master' into ssh_sqla_form

1e0161d

SSH Tunnel UI:

e394e7a

- Hide the entire form behind the feature flag

SSH Tunnel UI:

a096122

- Fix TestConnection when editing a DB with SSH Tunnel attached to it - Add new unmask_password_info util method for SSH Tunnels

SSH Tunnel UI:

8df08f9

- Add ssh_tunnel id in our response for the GET endpoint of DB so we can call test_connection endpoint without depending on DB's name

hughhhh reviewed Jan 10, 2023

View reviewed changes

Antonio-RiveroMartnez added 4 commits January 11, 2023 12:47

Merge branch 'master' into ssh_sqla_form

26a668d

SSH Tunnel UI:

97330e1

- Use the allow_ssh_tunneling flag from the db_engine along with the feature flag to display the SSH Tunnel form, so, if a DB engine does not support SSH Tunnels, nothing gets displayed. Right now, only Postgres does

SSH Tunnel UI:

9dd84de

- Fix tests with proper flag values

SSH Tunnel UI:

2ba8bdc

- More fixes to tests

hughhhh approved these changes Jan 11, 2023

View reviewed changes

SSH Tunnel UI:

cae95ae

- Fix test

hughhhh merged commit 5399365 into apache:master Jan 11, 2023

mistercrunch added the 🚢 2.1.3 label Feb 18, 2024

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ssh_tunnel): SQLAlchemy Form UI #22513

feat(ssh_tunnel): SQLAlchemy Form UI #22513

Antonio-RiveroMartnez commented Dec 22, 2022 •

edited

Loading

hughhhh Jan 5, 2023

Antonio-RiveroMartnez Jan 5, 2023 •

edited

Loading

hughhhh Jan 10, 2023

Antonio-RiveroMartnez Jan 11, 2023

hughhhh left a comment

		@@ -73,6 +73,7 @@ export interface FieldPropTypes {
		onRemoveTableCatalog: (idx: number) => void;

feat(ssh_tunnel): SQLAlchemy Form UI #22513

feat(ssh_tunnel): SQLAlchemy Form UI #22513

Conversation

Antonio-RiveroMartnez commented Dec 22, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

hughhhh Jan 5, 2023

Choose a reason for hiding this comment

Antonio-RiveroMartnez Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

hughhhh Jan 10, 2023

Choose a reason for hiding this comment

Antonio-RiveroMartnez Jan 11, 2023

Choose a reason for hiding this comment

hughhhh left a comment

Choose a reason for hiding this comment

Antonio-RiveroMartnez commented Dec 22, 2022 •

edited

Loading

Antonio-RiveroMartnez Jan 5, 2023 •

edited

Loading