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 2022/11/30 17:03:07 UTC

[superset] branch test-ssh-tunnel-1 created (now 027844f177)

This is an automated email from the ASF dual-hosted git repository.

hugh pushed a change to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 027844f177 fix merge conflicts

This branch includes the following new commits:

     new f00bd69dc9 SSH Tunnel:
     new b2cc56471a SSH Tunnel:
     new dc42aee5e4 SSH Tunnel:
     new 6a210562d4 SSH Tunnel:
     new 28425eb2b1 SSH Tunnel:
     new 0136ba3d1b SSH Tunnel:
     new cead0e67ce SSH Tunnel:
     new f4ab29d5b5 SSH Tunnel:
     new 70bafd38b5 fix merge conflicts
     new 027844f177 fix merge conflicts

The 10 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[superset] 01/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit f00bd69dc9d6d5d99af79d8fb6b809f4067e77e2
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Tue Nov 22 15:54:21 2022 -0300

    SSH Tunnel:
    
    - Initial DELETE and CREATE ssh tunnel APIs
    - Add happy path tests for each API
    
    NOTE:
    - Updating a DB might also result in creating a Tunnel, that flow will get tested in the UPDATE API changes.
---
 superset/constants.py                          |  1 +
 superset/databases/api.py                      | 62 ++++++++++++++++++++++
 superset/databases/commands/create.py          | 13 +++++
 superset/databases/schemas.py                  | 41 ++++++++-------
 tests/integration_tests/databases/api_tests.py | 31 +++++++++++
 tests/unit_tests/databases/api_test.py         | 72 ++++++++++++++++++++++++++
 6 files changed, 200 insertions(+), 20 deletions(-)

diff --git a/superset/constants.py b/superset/constants.py
index 7d759acf67..b775926b21 100644
--- a/superset/constants.py
+++ b/superset/constants.py
@@ -136,6 +136,7 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = {
     "validate_sql": "read",
     "get_data": "read",
     "samples": "read",
+    "delete_ssh_tunnel": "write",
 }
 
 EXTRA_FORM_DATA_APPEND_KEYS = {
diff --git a/superset/databases/api.py b/superset/databases/api.py
index aced8e7c6f..9cab40ede4 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -72,6 +72,12 @@ from superset.databases.schemas import (
     ValidateSQLRequest,
     ValidateSQLResponse,
 )
+from superset.databases.ssh_tunnel.commands.create import CreateSSHTunnelCommand
+from superset.databases.ssh_tunnel.commands.delete import DeleteSSHTunnelCommand
+from superset.databases.ssh_tunnel.commands.exceptions import (
+    SSHTunnelDeleteFailedError,
+    SSHTunnelNotFoundError,
+)
 from superset.databases.utils import get_table_metadata
 from superset.db_engine_specs import get_available_engine_specs
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
@@ -107,6 +113,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         "available",
         "validate_parameters",
         "validate_sql",
+        "delete_ssh_tunnel",
     }
     resource_name = "database"
     class_permission_name = "Database"
@@ -1204,3 +1211,58 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         command = ValidateDatabaseParametersCommand(payload)
         command.run()
         return self.response(200, message="OK")
+
+    @expose("/<int:pk>/ssh_tunnel/", methods=["DELETE"])
+    @protect()
+    @safe
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+        f".delete_ssh_tunnel",
+        log_to_statsd=False,
+    )
+    def delete_ssh_tunnel(self, pk: int) -> Response:
+        """Deletes a SSH Tunnel
+        ---
+        delete:
+          description: >-
+            Deletes a SSH Tunnel.
+          parameters:
+          - in: path
+            schema:
+              type: integer
+            name: pk
+          responses:
+            200:
+              description: SSH Tunnel deleted
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      message:
+                        type: string
+            401:
+              $ref: '#/components/responses/401'
+            403:
+              $ref: '#/components/responses/403'
+            404:
+              $ref: '#/components/responses/404'
+            422:
+              $ref: '#/components/responses/422'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        try:
+            DeleteSSHTunnelCommand(pk).run()
+            return self.response(200, message="OK")
+        except SSHTunnelNotFoundError:
+            return self.response_404()
+        except SSHTunnelDeleteFailedError as ex:
+            logger.error(
+                "Error deleting SSH Tunnel %s: %s",
+                self.__class__.__name__,
+                str(ex),
+                exc_info=True,
+            )
+            return self.response_422(message=str(ex))
diff --git a/superset/databases/commands/create.py b/superset/databases/commands/create.py
index 4dc8e8eda4..421e060a4b 100644
--- a/superset/databases/commands/create.py
+++ b/superset/databases/commands/create.py
@@ -31,6 +31,9 @@ from superset.databases.commands.exceptions import (
 )
 from superset.databases.commands.test_connection import TestConnectionDatabaseCommand
 from superset.databases.dao import DatabaseDAO
+from superset.databases.ssh_tunnel.commands.exceptions import SSHTunnelInvalidError
+from superset.databases.ssh_tunnel.dao import SSHTunnelDAO
+from superset.databases.ssh_tunnel.models import SSHTunnel
 from superset.exceptions import SupersetErrorsException
 from superset.extensions import db, event_logger, security_manager
 
@@ -77,6 +80,16 @@ class CreateDatabaseCommand(BaseCommand):
                 security_manager.add_permission_view_menu(
                     "schema_access", security_manager.get_schema_perm(database, schema)
                 )
+
+            if self._properties.get("ssh_tunnel"):
+                SSHTunnelDAO.create(
+                    {
+                        **self._properties.get("ssh_tunnel"),
+                        "database_id": database.id,
+                    },
+                    commit=False,
+                )
+
             db.session.commit()
         except DAOCreateFailedError as ex:
             db.session.rollback()
diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index b874d147a2..9c2de0b18e 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -365,6 +365,26 @@ class DatabaseValidateParametersSchema(Schema):
     )
 
 
+class DatabaseSSHTunnel(Schema):
+    id = fields.Integer()
+    database_id = fields.Integer()
+
+    server_address = fields.String()
+    server_port = fields.Integer()
+    username = fields.String()
+
+    # Basic Authentication
+    password = fields.String(required=False)
+
+    # password protected private key authentication
+    private_key = fields.String(required=False)
+    private_key_password = fields.String(required=False)
+
+    # remote binding port
+    bind_host = fields.String()
+    bind_port = fields.Integer()
+
+
 class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin):
     class Meta:  # pylint: disable=too-few-public-methods
         unknown = EXCLUDE
@@ -409,6 +429,7 @@ class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin):
     is_managed_externally = fields.Boolean(allow_none=True, default=False)
     external_url = fields.String(allow_none=True)
     uuid = fields.String(required=False)
+    ssh_tunnel = fields.Nested(DatabaseSSHTunnel, allow_none=True)
 
 
 class DatabasePutSchema(Schema, DatabaseParametersSchemaMixin):
@@ -456,26 +477,6 @@ class DatabasePutSchema(Schema, DatabaseParametersSchemaMixin):
     external_url = fields.String(allow_none=True)
 
 
-class DatabaseSSHTunnel(Schema):
-    id = fields.Integer()
-    database_id = fields.Integer()
-
-    server_address = fields.String()
-    server_port = fields.Integer()
-    username = fields.String()
-
-    # Basic Authentication
-    password = fields.String(required=False)
-
-    # password protected private key authentication
-    private_key = fields.String(required=False)
-    private_key_password = fields.String(required=False)
-
-    # remote binding port
-    bind_host = fields.String()
-    bind_port = fields.Integer()
-
-
 class DatabaseTestConnectionSchema(Schema, DatabaseParametersSchemaMixin):
 
     rename_encrypted_extra = pre_load(rename_encrypted_extra)
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index 9698c7e42a..7821900c87 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -35,6 +35,7 @@ from sqlalchemy.sql import func
 
 from superset import db, security_manager
 from superset.connectors.sqla.models import SqlaTable
+from superset.databases.utils import make_url_safe
 from superset.db_engine_specs.mysql import MySQLEngineSpec
 from superset.db_engine_specs.postgres import PostgresEngineSpec
 from superset.db_engine_specs.redshift import RedshiftEngineSpec
@@ -279,6 +280,36 @@ class TestDatabaseApi(SupersetTestCase):
         db.session.delete(model)
         db.session.commit()
 
+    def test_create_database_with_ssh_tunnel(self):
+        """
+        Database API: Test create with SSH Tunnel
+        """
+        self.login(username="admin")
+        example_db = get_example_database()
+        if example_db.backend == "sqlite":
+            return
+        ssh_tunnel_properties = {
+            "server_address": "123.132.123.1",
+            "bind_host": "localhost",
+            "bind_port": "5432",
+            "username": "foo",
+            "password": "bar",
+        }
+        database_data = {
+            "database_name": "test-create-db-with-ssh-tunnel",
+            "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+            "ssh_tunnel": ssh_tunnel_properties,
+        }
+
+        uri = "api/v1/database/"
+        rv = self.client.post(uri, json=database_data)
+        response = json.loads(rv.data.decode("utf-8"))
+        self.assertEqual(rv.status_code, 201)
+        # Cleanup
+        model = db.session.query(Database).get(response.get("id"))
+        db.session.delete(model)
+        db.session.commit()
+
     def test_create_database_invalid_configuration_method(self):
         """
         Database API: Test create with an invalid configuration method.
diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py
index d6f8897c4a..73e79f3a82 100644
--- a/tests/unit_tests/databases/api_test.py
+++ b/tests/unit_tests/databases/api_test.py
@@ -191,3 +191,75 @@ def test_non_zip_import(client: Any, full_api_access: None) -> None:
             }
         ]
     }
+
+
+def test_delete_ssh_tunnel(
+    mocker: MockFixture,
+    app: Any,
+    session: Session,
+    client: Any,
+    full_api_access: None,
+) -> None:
+    """
+    Test that we can delete SSH Tunnel
+    """
+    with app.app_context():
+        from superset.databases.api import DatabaseRestApi
+        from superset.databases.dao import DatabaseDAO
+        from superset.databases.ssh_tunnel.models import SSHTunnel
+        from superset.models.core import Database
+
+        DatabaseRestApi.datamodel.session = session
+
+        # create table for databases
+        Database.metadata.create_all(session.get_bind())  # pylint: disable=no-member
+
+        # Create our Database
+        database = Database(
+            database_name="my_database",
+            sqlalchemy_uri="gsheets://",
+            encrypted_extra=json.dumps(
+                {
+                    "service_account_info": {
+                        "type": "service_account",
+                        "project_id": "black-sanctum-314419",
+                        "private_key_id": "259b0d419a8f840056158763ff54d8b08f7b8173",
+                        "private_key": "SECRET",
+                        "client_email": "google-spreadsheets-demo-servi@black-sanctum-314419.iam.gserviceaccount.com",
+                        "client_id": "SSH_TUNNEL_CREDENTIALS_CLIENT",
+                        "auth_uri": "https://accounts.google.com/o/oauth2/auth",
+                        "token_uri": "https://oauth2.googleapis.com/token",
+                        "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
+                        "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/google-spreadsheets-demo-servi%40black-sanctum-314419.iam.gserviceaccount.com",
+                    },
+                }
+            ),
+        )
+        session.add(database)
+        session.commit()
+
+        # mock the lookup so that we don't need to include the driver
+        mocker.patch("sqlalchemy.engine.URL.get_driver_name", return_value="gsheets")
+        mocker.patch("superset.utils.log.DBEventLogger.log")
+
+        # Create our SSHTunnel
+        tunnel = SSHTunnel(
+            database_id=1,
+            database=database,
+        )
+
+        session.add(tunnel)
+        session.commit()
+
+        # Get our recently created SSHTunnel
+        response_tunnel = DatabaseDAO.get_ssh_tunnel(1)
+        assert response_tunnel
+        assert isinstance(response_tunnel, SSHTunnel)
+        assert 1 == response_tunnel.database_id
+
+        # Delete the recently created SSHTunnel
+        response_delete_tunnel = client.delete("/api/v1/database/1/ssh_tunnel/")
+        assert response_delete_tunnel.json["message"] == "OK"
+
+        response_tunnel = DatabaseDAO.get_ssh_tunnel(1)
+        assert response_tunnel is None


[superset] 06/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 0136ba3d1b801d0aaa09facce41d25c73ccd61ab
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Mon Nov 28 10:36:20 2022 -0300

    SSH Tunnel:
    
    - Fix pre-commit errors
---
 superset/databases/commands/create.py | 4 ++--
 superset/databases/commands/update.py | 6 +++---
 superset/databases/dao.py             | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/superset/databases/commands/create.py b/superset/databases/commands/create.py
index ad8fa3a450..ef2f555d61 100644
--- a/superset/databases/commands/create.py
+++ b/superset/databases/commands/create.py
@@ -79,10 +79,10 @@ class CreateDatabaseCommand(BaseCommand):
                     "schema_access", security_manager.get_schema_perm(database, schema)
                 )
 
-            if self._properties.get("ssh_tunnel"):
+            if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
                 SSHTunnelDAO.create(
                     {
-                        **self._properties.get("ssh_tunnel"),
+                        **ssh_tunnel_properties,
                         "database_id": database.id,
                     },
                     commit=False,
diff --git a/superset/databases/commands/update.py b/superset/databases/commands/update.py
index b662ab3546..ba353b8960 100644
--- a/superset/databases/commands/update.py
+++ b/superset/databases/commands/update.py
@@ -96,13 +96,13 @@ class UpdateDatabaseCommand(BaseCommand):
                     "schema_access", security_manager.get_schema_perm(database, schema)
                 )
 
-            if self._properties.get("ssh_tunnel"):
+            if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
                 existing_ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
                 if existing_ssh_tunnel is None:
                     # We couldn't found an existing tunnel so we need to create one
                     SSHTunnelDAO.create(
                         {
-                            **self._properties.get("ssh_tunnel"),
+                            **ssh_tunnel_properties,
                             "database_id": database.id,
                         },
                         commit=False,
@@ -112,7 +112,7 @@ class UpdateDatabaseCommand(BaseCommand):
                     ssh_tunnel_model = SSHTunnelDAO.find_by_id(existing_ssh_tunnel.id)
                     SSHTunnelDAO.update(
                         ssh_tunnel_model,
-                        self._properties.get("ssh_tunnel"),
+                        ssh_tunnel_properties,
                         commit=False,
                     )
 
diff --git a/superset/databases/dao.py b/superset/databases/dao.py
index 4e9ad1ee8c..d5a58245d1 100644
--- a/superset/databases/dao.py
+++ b/superset/databases/dao.py
@@ -127,7 +127,7 @@ class DatabaseDAO(BaseDAO):
         )
 
     @classmethod
-    def get_ssh_tunnel(cls, database_id: int) -> Dict[str, Any]:
+    def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel:
         ssh_tunnel = (
             db.session.query(SSHTunnel)
             .filter(SSHTunnel.database_id == database_id)


[superset] 02/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit b2cc56471a35383aa9d5e4247417bf67d1e0b412
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Fri Nov 25 08:32:21 2022 -0300

    SSH Tunnel:
    
    - Uncomment ssh tunnel creation for TestConnection command
    - Update our tests
    - Add update changes in API
---
 superset/databases/commands/test_connection.py |  10 +-
 superset/databases/commands/update.py          |  27 ++++-
 superset/databases/schemas.py                  |   1 +
 tests/integration_tests/databases/api_tests.py | 140 ++++++++++++++++++++++++-
 4 files changed, 170 insertions(+), 8 deletions(-)

diff --git a/superset/databases/commands/test_connection.py b/superset/databases/commands/test_connection.py
index 7b913ed202..098c24ff11 100644
--- a/superset/databases/commands/test_connection.py
+++ b/superset/databases/commands/test_connection.py
@@ -32,6 +32,7 @@ from superset.databases.commands.exceptions import (
     DatabaseTestConnectionUnexpectedError,
 )
 from superset.databases.dao import DatabaseDAO
+from superset.databases.ssh_tunnel.models import SSHTunnel
 from superset.databases.utils import make_url_safe
 from superset.errors import ErrorLevel, SupersetErrorType
 from superset.exceptions import (
@@ -90,10 +91,13 @@ class TestConnectionDatabaseCommand(BaseCommand):
             database.set_sqlalchemy_uri(uri)
             database.db_engine_spec.mutate_db_for_connection_test(database)
 
-            # TODO: (hughhh) uncomment in API enablement PR
+            # Generate tunnel if present in the properties
             ssh_tunnel = None
-            # if self._properties.get("ssh_tunnel"):
-            #     ssh_tunnel = SSHTunnel(**self._properties["ssh_tunnel"])
+            if ssh_tunnel := self._properties.get("ssh_tunnel"):
+                url = make_url_safe(database.sqlalchemy_uri_decrypted)
+                ssh_tunnel["bind_host"] = url.host
+                ssh_tunnel["bind_port"] = url.port
+                ssh_tunnel = SSHTunnel(**ssh_tunnel)
 
             event_logger.log_with_context(
                 action="test_connection_attempt",
diff --git a/superset/databases/commands/update.py b/superset/databases/commands/update.py
index 80e3a9b54e..b662ab3546 100644
--- a/superset/databases/commands/update.py
+++ b/superset/databases/commands/update.py
@@ -21,7 +21,7 @@ from flask_appbuilder.models.sqla import Model
 from marshmallow import ValidationError
 
 from superset.commands.base import BaseCommand
-from superset.dao.exceptions import DAOUpdateFailedError
+from superset.dao.exceptions import DAOCreateFailedError, DAOUpdateFailedError
 from superset.databases.commands.exceptions import (
     DatabaseConnectionFailedError,
     DatabaseExistsValidationError,
@@ -30,6 +30,7 @@ from superset.databases.commands.exceptions import (
     DatabaseUpdateFailedError,
 )
 from superset.databases.dao import DatabaseDAO
+from superset.databases.ssh_tunnel.dao import SSHTunnelDAO
 from superset.extensions import db, security_manager
 from superset.models.core import Database
 from superset.utils.core import DatasourceType
@@ -94,11 +95,35 @@ class UpdateDatabaseCommand(BaseCommand):
                 security_manager.add_permission_view_menu(
                     "schema_access", security_manager.get_schema_perm(database, schema)
                 )
+
+            if self._properties.get("ssh_tunnel"):
+                existing_ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
+                if existing_ssh_tunnel is None:
+                    # We couldn't found an existing tunnel so we need to create one
+                    SSHTunnelDAO.create(
+                        {
+                            **self._properties.get("ssh_tunnel"),
+                            "database_id": database.id,
+                        },
+                        commit=False,
+                    )
+                else:
+                    # We found an existing tunnel so we need to update it
+                    ssh_tunnel_model = SSHTunnelDAO.find_by_id(existing_ssh_tunnel.id)
+                    SSHTunnelDAO.update(
+                        ssh_tunnel_model,
+                        self._properties.get("ssh_tunnel"),
+                        commit=False,
+                    )
+
             db.session.commit()
 
         except DAOUpdateFailedError as ex:
             logger.exception(ex.exception)
             raise DatabaseUpdateFailedError() from ex
+        except DAOCreateFailedError as ex:
+            logger.exception(ex.exception)
+            raise DatabaseUpdateFailedError() from ex
         return database
 
     @staticmethod
diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index 9c2de0b18e..ff134eb0af 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -475,6 +475,7 @@ class DatabasePutSchema(Schema, DatabaseParametersSchemaMixin):
     )
     is_managed_externally = fields.Boolean(allow_none=True, default=False)
     external_url = fields.String(allow_none=True)
+    ssh_tunnel = fields.Nested(DatabaseSSHTunnel, allow_none=True)
 
 
 class DatabaseTestConnectionSchema(Schema, DatabaseParametersSchemaMixin):
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index 7821900c87..2dd5559219 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -35,6 +35,7 @@ from sqlalchemy.sql import func
 
 from superset import db, security_manager
 from superset.connectors.sqla.models import SqlaTable
+from superset.databases.ssh_tunnel.models import SSHTunnel
 from superset.databases.utils import make_url_safe
 from superset.db_engine_specs.mysql import MySQLEngineSpec
 from superset.db_engine_specs.postgres import PostgresEngineSpec
@@ -280,7 +281,12 @@ class TestDatabaseApi(SupersetTestCase):
         db.session.delete(model)
         db.session.commit()
 
-    def test_create_database_with_ssh_tunnel(self):
+    @mock.patch(
+        "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+    )
+    def test_create_database_with_ssh_tunnel(
+        self, mock_test_connection_database_command_run
+    ):
         """
         Database API: Test create with SSH Tunnel
         """
@@ -290,13 +296,12 @@ class TestDatabaseApi(SupersetTestCase):
             return
         ssh_tunnel_properties = {
             "server_address": "123.132.123.1",
-            "bind_host": "localhost",
-            "bind_port": "5432",
+            "server_port": 8080,
             "username": "foo",
             "password": "bar",
         }
         database_data = {
-            "database_name": "test-create-db-with-ssh-tunnel",
+            "database_name": "test-db-with-ssh-tunnel",
             "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
             "ssh_tunnel": ssh_tunnel_properties,
         }
@@ -305,11 +310,138 @@ class TestDatabaseApi(SupersetTestCase):
         rv = self.client.post(uri, json=database_data)
         response = json.loads(rv.data.decode("utf-8"))
         self.assertEqual(rv.status_code, 201)
+        model_ssh_tunnel = (
+            db.session.query(SSHTunnel)
+            .filter(SSHTunnel.database_id == response.get("id"))
+            .one()
+        )
+        self.assertEqual(model_ssh_tunnel.database_id, response.get("id"))
         # Cleanup
         model = db.session.query(Database).get(response.get("id"))
         db.session.delete(model)
         db.session.commit()
 
+    @mock.patch(
+        "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+    )
+    @mock.patch(
+        "superset.models.core.Database.get_all_schema_names",
+    )
+    def test_update_database_with_ssh_tunnel(
+        self, mock_test_connection_database_command_run, mock_get_all_schema_names
+    ):
+        """
+        Database API: Test update with SSH Tunnel
+        """
+        self.login(username="admin")
+        example_db = get_example_database()
+        if example_db.backend == "sqlite":
+            return
+        ssh_tunnel_properties = {
+            "server_address": "123.132.123.1",
+            "server_port": 8080,
+            "username": "foo",
+            "password": "bar",
+        }
+        database_data = {
+            "database_name": "test-db-with-ssh-tunnel",
+            "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+        }
+        database_data_with_ssh_tunnel = {
+            "database_name": "test-db-with-ssh-tunnel",
+            "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+            "ssh_tunnel": ssh_tunnel_properties,
+        }
+
+        uri = "api/v1/database/"
+        rv = self.client.post(uri, json=database_data)
+        response = json.loads(rv.data.decode("utf-8"))
+        self.assertEqual(rv.status_code, 201)
+
+        uri = "api/v1/database/{}".format(response.get("id"))
+        rv = self.client.put(uri, json=database_data_with_ssh_tunnel)
+        response_update = json.loads(rv.data.decode("utf-8"))
+        self.assertEqual(rv.status_code, 200)
+
+        model_ssh_tunnel = (
+            db.session.query(SSHTunnel)
+            .filter(SSHTunnel.database_id == response_update.get("id"))
+            .one()
+        )
+        self.assertEqual(model_ssh_tunnel.database_id, response_update.get("id"))
+        # Cleanup
+        model = db.session.query(Database).get(response.get("id"))
+        db.session.delete(model)
+        db.session.commit()
+
+    @mock.patch(
+        "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+    )
+    def test_update_ssh_tunnel_via_database_api(
+        self, mock_test_connection_database_command_run
+    ):
+        """
+        Database API: Test update with SSH Tunnel
+        """
+        self.login(username="admin")
+        example_db = get_example_database()
+
+        if example_db.backend == "sqlite":
+            return
+        initial_ssh_tunnel_properties = {
+            "server_address": "123.132.123.1",
+            "server_port": 8080,
+            "username": "foo",
+            "password": "bar",
+        }
+        updated_ssh_tunnel_properties = {
+            "server_address": "123.132.123.2",
+            "server_port": 8081,
+            "username": "Test",
+            "password": "bar",
+        }
+        database_data_with_ssh_tunnel = {
+            "database_name": "test-db-with-ssh-tunnel",
+            "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+            "ssh_tunnel": initial_ssh_tunnel_properties,
+        }
+        database_data_with_ssh_tunnel_update = {
+            "database_name": "test-db-with-ssh-tunnel",
+            "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+            "ssh_tunnel": updated_ssh_tunnel_properties,
+        }
+
+        uri = "api/v1/database/"
+        rv = self.client.post(uri, json=database_data_with_ssh_tunnel)
+        response = json.loads(rv.data.decode("utf-8"))
+        self.assertEqual(rv.status_code, 201)
+        model_ssh_tunnel = (
+            db.session.query(SSHTunnel)
+            .filter(SSHTunnel.database_id == response.get("id"))
+            .one()
+        )
+        self.assertEqual(model_ssh_tunnel.database_id, response.get("id"))
+        self.assertEqual(model_ssh_tunnel.username, "foo")
+        with mock.patch(
+            "superset.models.core.Database.get_all_schema_names",
+            return_value=["information_schema", "public"],
+        ):
+            uri = "api/v1/database/{}".format(response.get("id"))
+            rv = self.client.put(uri, json=database_data_with_ssh_tunnel_update)
+            response_update = json.loads(rv.data.decode("utf-8"))
+            self.assertEqual(rv.status_code, 200)
+            model_ssh_tunnel = (
+                db.session.query(SSHTunnel)
+                .filter(SSHTunnel.database_id == response_update.get("id"))
+                .one()
+            )
+            self.assertEqual(model_ssh_tunnel.database_id, response_update.get("id"))
+            self.assertEqual(model_ssh_tunnel.username, "Test")
+            # Cleanup
+            model = db.session.query(Database).get(response.get("id"))
+            db.session.delete(model)
+            db.session.commit()
+
     def test_create_database_invalid_configuration_method(self):
         """
         Database API: Test create with an invalid configuration method.


[superset] 09/10: fix merge conflicts

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 70bafd38b50091122585749e64a9f3fa51c0d562
Merge: bb7805592d f4ab29d5b5
Author: hughhhh <hu...@gmail.com>
AuthorDate: Wed Nov 30 11:50:24 2022 -0500

    fix merge conflicts

 superset/constants.py                          |   1 +
 superset/databases/api.py                      |  61 ++++++
 superset/databases/commands/create.py          |  11 ++
 superset/databases/commands/update.py          |  27 ++-
 superset/databases/schemas.py                  |  17 ++
 tests/integration_tests/databases/api_tests.py | 258 +++++++++++++++++++++++++
 tests/unit_tests/databases/api_test.py         | 144 ++++++++++++++
 7 files changed, 518 insertions(+), 1 deletion(-)



[superset] 07/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit cead0e67ce4e171fc53d724837410666f5ae773b
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Tue Nov 29 16:01:58 2022 -0300

    SSH Tunnel:
    
    - Remove bind properties from schema used in TestConnection
    - Add test for SSH Tunnel failure and no DB creation
---
 superset/databases/schemas.py                  |  4 ---
 tests/integration_tests/databases/api_tests.py | 45 ++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index ff134eb0af..8cf6f01597 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -380,10 +380,6 @@ class DatabaseSSHTunnel(Schema):
     private_key = fields.String(required=False)
     private_key_password = fields.String(required=False)
 
-    # remote binding port
-    bind_host = fields.String()
-    bind_port = fields.Integer()
-
 
 class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin):
     class Meta:  # pylint: disable=too-few-public-methods
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index 24a503ae91..4eb1f9ebd4 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -485,6 +485,51 @@ class TestDatabaseApi(SupersetTestCase):
         )
         assert model_ssh_tunnel is None
 
+    @mock.patch(
+        "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+    )
+    def test_do_not_create_database_if_ssh_tunnel_creation_fails(
+        self, mock_test_connection_database_command_run
+    ):
+        """
+        Database API: Test create with SSH Tunnel
+        """
+        self.login(username="admin")
+        example_db = get_example_database()
+        if example_db.backend == "sqlite":
+            return
+        ssh_tunnel_properties = {
+            "server_address_failure": "123.132.123.1",
+        }
+        database_data = {
+            "database_name": "test-db-failure-ssh-tunnel",
+            "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+            "ssh_tunnel": ssh_tunnel_properties,
+        }
+        fail_message = {
+            "message": {"ssh_tunnel": {"server_address_failure": ["Unknown field."]}}
+        }
+
+        uri = "api/v1/database/"
+        rv = self.client.post(uri, json=database_data)
+        response = json.loads(rv.data.decode("utf-8"))
+        self.assertEqual(rv.status_code, 400)
+        model_ssh_tunnel = (
+            db.session.query(SSHTunnel)
+            .filter(SSHTunnel.database_id == response.get("id"))
+            .one_or_none()
+        )
+        assert model_ssh_tunnel is None
+        self.assertEqual(response, fail_message)
+        # Cleanup
+        model = (
+            db.session.query(Database)
+            .filter(Database.database_name == "test-db-failure-ssh-tunnel")
+            .one_or_none()
+        )
+        # the DB should not be created
+        assert model is None
+
     def test_create_database_invalid_configuration_method(self):
         """
         Database API: Test create with an invalid configuration method.


[superset] 04/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 6a210562d49033be95e0951613d8e5aa6c96d084
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Sun Nov 27 12:12:22 2022 -0300

    SSH Tunnel:
    
    - Add not found test when deleting the tunnel
---
 tests/unit_tests/databases/api_test.py | 72 ++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py
index 73e79f3a82..fe4211289c 100644
--- a/tests/unit_tests/databases/api_test.py
+++ b/tests/unit_tests/databases/api_test.py
@@ -263,3 +263,75 @@ def test_delete_ssh_tunnel(
 
         response_tunnel = DatabaseDAO.get_ssh_tunnel(1)
         assert response_tunnel is None
+
+
+def test_delete_ssh_tunnel_not_found(
+    mocker: MockFixture,
+    app: Any,
+    session: Session,
+    client: Any,
+    full_api_access: None,
+) -> None:
+    """
+    Test that we cannot delete a tunnel that does not exist
+    """
+    with app.app_context():
+        from superset.databases.api import DatabaseRestApi
+        from superset.databases.dao import DatabaseDAO
+        from superset.databases.ssh_tunnel.models import SSHTunnel
+        from superset.models.core import Database
+
+        DatabaseRestApi.datamodel.session = session
+
+        # create table for databases
+        Database.metadata.create_all(session.get_bind())  # pylint: disable=no-member
+
+        # Create our Database
+        database = Database(
+            database_name="my_database",
+            sqlalchemy_uri="gsheets://",
+            encrypted_extra=json.dumps(
+                {
+                    "service_account_info": {
+                        "type": "service_account",
+                        "project_id": "black-sanctum-314419",
+                        "private_key_id": "259b0d419a8f840056158763ff54d8b08f7b8173",
+                        "private_key": "SECRET",
+                        "client_email": "google-spreadsheets-demo-servi@black-sanctum-314419.iam.gserviceaccount.com",
+                        "client_id": "SSH_TUNNEL_CREDENTIALS_CLIENT",
+                        "auth_uri": "https://accounts.google.com/o/oauth2/auth",
+                        "token_uri": "https://oauth2.googleapis.com/token",
+                        "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
+                        "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/google-spreadsheets-demo-servi%40black-sanctum-314419.iam.gserviceaccount.com",
+                    },
+                }
+            ),
+        )
+        session.add(database)
+        session.commit()
+
+        # mock the lookup so that we don't need to include the driver
+        mocker.patch("sqlalchemy.engine.URL.get_driver_name", return_value="gsheets")
+        mocker.patch("superset.utils.log.DBEventLogger.log")
+
+        # Create our SSHTunnel
+        tunnel = SSHTunnel(
+            database_id=1,
+            database=database,
+        )
+
+        session.add(tunnel)
+        session.commit()
+
+        # Delete the recently created SSHTunnel
+        response_delete_tunnel = client.delete("/api/v1/database/2/ssh_tunnel/")
+        assert response_delete_tunnel.json["message"] == "Not found"
+
+        # Get our recently created SSHTunnel
+        response_tunnel = DatabaseDAO.get_ssh_tunnel(1)
+        assert response_tunnel
+        assert isinstance(response_tunnel, SSHTunnel)
+        assert 1 == response_tunnel.database_id
+
+        response_tunnel = DatabaseDAO.get_ssh_tunnel(2)
+        assert response_tunnel is None


[superset] 05/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 28425eb2b14abdf2bf8869b21ba56be945dd4776
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Mon Nov 28 10:19:02 2022 -0300

    SSH Tunnel:
    
    - FIx linting errors
---
 superset/databases/api.py             | 1 -
 superset/databases/commands/create.py | 2 --
 2 files changed, 3 deletions(-)

diff --git a/superset/databases/api.py b/superset/databases/api.py
index 9cab40ede4..92b9635fd8 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -72,7 +72,6 @@ from superset.databases.schemas import (
     ValidateSQLRequest,
     ValidateSQLResponse,
 )
-from superset.databases.ssh_tunnel.commands.create import CreateSSHTunnelCommand
 from superset.databases.ssh_tunnel.commands.delete import DeleteSSHTunnelCommand
 from superset.databases.ssh_tunnel.commands.exceptions import (
     SSHTunnelDeleteFailedError,
diff --git a/superset/databases/commands/create.py b/superset/databases/commands/create.py
index 421e060a4b..ad8fa3a450 100644
--- a/superset/databases/commands/create.py
+++ b/superset/databases/commands/create.py
@@ -31,9 +31,7 @@ from superset.databases.commands.exceptions import (
 )
 from superset.databases.commands.test_connection import TestConnectionDatabaseCommand
 from superset.databases.dao import DatabaseDAO
-from superset.databases.ssh_tunnel.commands.exceptions import SSHTunnelInvalidError
 from superset.databases.ssh_tunnel.dao import SSHTunnelDAO
-from superset.databases.ssh_tunnel.models import SSHTunnel
 from superset.exceptions import SupersetErrorsException
 from superset.extensions import db, event_logger, security_manager
 


[superset] 10/10: fix merge conflicts

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 027844f1778ebadcf8e365de5d5f7b9eedc89322
Merge: 70bafd38b5 8c6d6e46a6
Author: hughhhh <hu...@gmail.com>
AuthorDate: Wed Nov 30 11:51:50 2022 -0500

    fix merge conflicts

 superset/config.py                               |  38 +++++++
 superset/databases/ssh_tunnel/commands/create.py |  14 ++-
 superset/databases/ssh_tunnel/commands/update.py |   7 ++
 superset/models/core.py                          |   5 +-
 tests/unit_tests/config_test.py                  | 125 +++++++++++++++++++++++
 5 files changed, 184 insertions(+), 5 deletions(-)

diff --cc superset/models/core.py
index d94372577c,48c0649c28..d81a6801cc
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@@ -394,8 -393,8 +396,7 @@@ class Database
                      source=source,
                      ssh_tunnel_server=server,
                  )
- 
          else:
 -            # do look up in table for using database_id
              yield self._get_sqla_engine(schema=schema, nullpool=nullpool, source=source)
  
      def _get_sqla_engine(


[superset] 03/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit dc42aee5e4834e414341621f53fba5171e8f156f
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Sun Nov 27 12:01:24 2022 -0300

    SSH Tunnel:
    
    - Adjust our tests so it's clear our endpoint uses PATCH and not PUT
    - Add delete cascading test: When deleting a database, the tunnel should be deleted
---
 tests/integration_tests/databases/api_tests.py | 49 ++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index 2dd5559219..24a503ae91 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -395,10 +395,7 @@ class TestDatabaseApi(SupersetTestCase):
             "password": "bar",
         }
         updated_ssh_tunnel_properties = {
-            "server_address": "123.132.123.2",
-            "server_port": 8081,
             "username": "Test",
-            "password": "bar",
         }
         database_data_with_ssh_tunnel = {
             "database_name": "test-db-with-ssh-tunnel",
@@ -437,11 +434,57 @@ class TestDatabaseApi(SupersetTestCase):
             )
             self.assertEqual(model_ssh_tunnel.database_id, response_update.get("id"))
             self.assertEqual(model_ssh_tunnel.username, "Test")
+            self.assertEqual(model_ssh_tunnel.server_address, "123.132.123.1")
+            self.assertEqual(model_ssh_tunnel.server_port, 8080)
             # Cleanup
             model = db.session.query(Database).get(response.get("id"))
             db.session.delete(model)
             db.session.commit()
 
+    @mock.patch(
+        "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+    )
+    def test_cascade_delete_ssh_tunnel(self, mock_test_connection_database_command_run):
+        """
+        Database API: Test create with SSH Tunnel
+        """
+        self.login(username="admin")
+        example_db = get_example_database()
+        if example_db.backend == "sqlite":
+            return
+        ssh_tunnel_properties = {
+            "server_address": "123.132.123.1",
+            "server_port": 8080,
+            "username": "foo",
+            "password": "bar",
+        }
+        database_data = {
+            "database_name": "test-db-with-ssh-tunnel",
+            "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+            "ssh_tunnel": ssh_tunnel_properties,
+        }
+
+        uri = "api/v1/database/"
+        rv = self.client.post(uri, json=database_data)
+        response = json.loads(rv.data.decode("utf-8"))
+        self.assertEqual(rv.status_code, 201)
+        model_ssh_tunnel = (
+            db.session.query(SSHTunnel)
+            .filter(SSHTunnel.database_id == response.get("id"))
+            .one()
+        )
+        self.assertEqual(model_ssh_tunnel.database_id, response.get("id"))
+        # Cleanup
+        model = db.session.query(Database).get(response.get("id"))
+        db.session.delete(model)
+        db.session.commit()
+        model_ssh_tunnel = (
+            db.session.query(SSHTunnel)
+            .filter(SSHTunnel.database_id == response.get("id"))
+            .one_or_none()
+        )
+        assert model_ssh_tunnel is None
+
     def test_create_database_invalid_configuration_method(self):
         """
         Database API: Test create with an invalid configuration method.


[superset] 08/10: SSH Tunnel:

Posted by hu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

hugh pushed a commit to branch test-ssh-tunnel-1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit f4ab29d5b5b8d35cd7f8e4eb9f6390ce7c94d783
Author: Antonio Rivero <an...@gmail.com>
AuthorDate: Tue Nov 29 17:00:33 2022 -0300

    SSH Tunnel:
    
    - Make server_address, server_port and username required fields for our SSHTunnel schema
    - Update the tests so we can consider new message with required fields
---
 superset/databases/schemas.py                  | 6 +++---
 tests/integration_tests/databases/api_tests.py | 9 ++++++++-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index 8cf6f01597..b6dfb5553d 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -369,9 +369,9 @@ class DatabaseSSHTunnel(Schema):
     id = fields.Integer()
     database_id = fields.Integer()
 
-    server_address = fields.String()
-    server_port = fields.Integer()
-    username = fields.String()
+    server_address = fields.String(required=True)
+    server_port = fields.Integer(required=True)
+    username = fields.String(required=True)
 
     # Basic Authentication
     password = fields.String(required=False)
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index 4eb1f9ebd4..588de0554a 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -507,7 +507,14 @@ class TestDatabaseApi(SupersetTestCase):
             "ssh_tunnel": ssh_tunnel_properties,
         }
         fail_message = {
-            "message": {"ssh_tunnel": {"server_address_failure": ["Unknown field."]}}
+            "message": {
+                "ssh_tunnel": {
+                    "server_address_failure": ["Unknown field."],
+                    "server_address": ["Missing data for required field."],
+                    "server_port": ["Missing data for required field."],
+                    "username": ["Missing data for required field."],
+                }
+            }
         }
 
         uri = "api/v1/database/"