Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ssh_tunnel): Rename allow_ssh_tunneling and change the default value to False #22723

Merged
merged 2 commits into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
'postgresql://user:password@host:port/dbname[?key=value&key=value...]',
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: true,
disable_ssh_tunneling: false,
},
},
{
Expand All @@ -141,7 +141,7 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
preferred: true,
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: false,
disable_ssh_tunneling: false,
},
},
{
Expand Down Expand Up @@ -194,7 +194,7 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
'mysql://user:password@host:port/dbname[?key=value&key=value...]',
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: false,
disable_ssh_tunneling: false,
},
},
{
Expand All @@ -204,7 +204,7 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
preferred: true,
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: true,
disable_ssh_tunneling: false,
},
},
{
Expand All @@ -214,7 +214,7 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
preferred: false,
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: false,
disable_ssh_tunneling: false,
},
},
{
Expand All @@ -239,7 +239,7 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
sqlalchemy_uri_placeholder: 'bigquery://{project_id}',
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: false,
disable_ssh_tunneling: true,
},
},
{
Expand All @@ -250,7 +250,7 @@ fetchMock.mock(AVAILABLE_DB_ENDPOINT, {
preferred: false,
engine_information: {
supports_file_upload: false,
allow_ssh_tunneling: false,
disable_ssh_tunneling: true,
},
},
{
Expand Down Expand Up @@ -1917,7 +1917,7 @@ describe('dbReducer', () => {
payload: {
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: true,
disable_ssh_tunneling: false,
},
...db,
driver: db.driver,
Expand All @@ -1932,7 +1932,7 @@ describe('dbReducer', () => {
configuration_method: db.configuration_method,
engine_information: {
supports_file_upload: true,
allow_ssh_tunneling: true,
disable_ssh_tunneling: false,
},
driver: db.driver,
expose_in_sqllab: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,12 +543,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const sslForced = isFeatureEnabled(
FeatureFlag.FORCE_DATABASE_CONNECTIONS_SSL,
);
const engineAllowsSSHTunneling = (
const disableSSHTunnelingForEngine = (
availableDbs?.databases?.find(
(DB: DatabaseObject) =>
DB.backend === db?.engine || DB.engine === db?.engine,
) as DatabaseObject
)?.engine_information?.allow_ssh_tunneling;
)?.engine_information?.disable_ssh_tunneling;
const sshTunneling = isFeatureEnabled(FeatureFlag.SSH_TUNNELING);
const hasAlert =
connectionAlert || !!(db?.engine && engineSpecificAlertMapping[db.engine]);
Expand Down Expand Up @@ -1493,7 +1493,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
testConnection={testConnection}
testInProgress={testInProgress}
>
{sshTunneling && engineAllowsSSHTunneling && (
{sshTunneling && !disableSSHTunnelingForEngine && (
<SSHTunnelForm
isEditMode={isEditMode}
sshTunneling={sshTunneling}
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/data/database/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export type DatabaseObject = {
// DB Engine Spec information
engine_information?: {
supports_file_upload?: boolean;
allow_ssh_tunneling?: boolean;
disable_ssh_tunneling?: boolean;
};

// SSH Tunnel information
Expand Down
2 changes: 1 addition & 1 deletion superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,7 @@ def available(self) -> Response:
supports_file_upload:
description: Whether the engine supports file uploads
type: boolean
allow_ssh_tunneling:
disable_ssh_tunneling:
description: Whether the engine supports SSH Tunnels
type: boolean
400:
Expand Down
1 change: 1 addition & 0 deletions superset/db_engine_specs/athena.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class AthenaEngineSpec(BaseEngineSpec):
engine = "awsathena"
engine_name = "Amazon Athena"
allows_escaped_colons = False
disable_ssh_tunneling = True

_time_grain_expressions = {
None: "{col}",
Expand Down
6 changes: 3 additions & 3 deletions superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
engine_aliases: Set[str] = set()
drivers: Dict[str, str] = {}
default_driver: Optional[str] = None
allow_ssh_tunneling = False
disable_ssh_tunneling = False

_date_trunc_functions: Dict[str, str] = {}
_time_grain_expressions: Dict[Optional[str], str] = {}
Expand Down Expand Up @@ -1697,11 +1697,11 @@ def get_public_information(cls) -> Dict[str, Any]:
Construct a Dict with properties we want to expose.

:returns: Dict with properties of our class like supports_file_upload
and allow_ssh_tunneling
and disable_ssh_tunneling
"""
return {
"supports_file_upload": cls.supports_file_upload,
"allow_ssh_tunneling": cls.allow_ssh_tunneling,
"disable_ssh_tunneling": cls.disable_ssh_tunneling,
}


Expand Down
1 change: 1 addition & 0 deletions superset/db_engine_specs/bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class BigQueryEngineSpec(BaseEngineSpec):
engine = "bigquery"
engine_name = "Google BigQuery"
max_column_name_length = 128
disable_ssh_tunneling = True

parameters_schema = BigQueryParametersSchema()
default_driver = "bigquery"
Expand Down
1 change: 1 addition & 0 deletions superset/db_engine_specs/gsheets.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class GSheetsEngineSpec(SqliteEngineSpec):
engine_name = "Google Sheets"
allows_joins = True
allows_subqueries = True
disable_ssh_tunneling = True

parameters_schema = GSheetsParametersSchema()
default_driver = "apsw"
Expand Down
1 change: 0 additions & 1 deletion superset/db_engine_specs/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ def epoch_to_dttm(cls) -> str:
class PostgresEngineSpec(PostgresBaseEngineSpec, BasicParametersMixin):
engine = "postgresql"
engine_aliases = {"postgres"}
allow_ssh_tunneling = True

default_driver = "psycopg2"
sqlalchemy_uri_placeholder = (
Expand Down
16 changes: 8 additions & 8 deletions tests/integration_tests/databases/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2304,7 +2304,7 @@ def test_available(self, app, get_available_engine_specs):
"sqlalchemy_uri_placeholder": "postgresql://user:password@host:port/dbname[?key=value&key=value...]",
"engine_information": {
"supports_file_upload": True,
"allow_ssh_tunneling": True,
"disable_ssh_tunneling": False,
},
},
{
Expand All @@ -2327,7 +2327,7 @@ def test_available(self, app, get_available_engine_specs):
"sqlalchemy_uri_placeholder": "bigquery://{project_id}",
"engine_information": {
"supports_file_upload": True,
"allow_ssh_tunneling": False,
"disable_ssh_tunneling": True,
},
},
{
Expand Down Expand Up @@ -2379,7 +2379,7 @@ def test_available(self, app, get_available_engine_specs):
"sqlalchemy_uri_placeholder": "redshift+psycopg2://user:password@host:port/dbname[?key=value&key=value...]",
"engine_information": {
"supports_file_upload": True,
"allow_ssh_tunneling": False,
"disable_ssh_tunneling": False,
},
},
{
Expand All @@ -2402,7 +2402,7 @@ def test_available(self, app, get_available_engine_specs):
"sqlalchemy_uri_placeholder": "gsheets://",
"engine_information": {
"supports_file_upload": False,
"allow_ssh_tunneling": False,
"disable_ssh_tunneling": True,
},
},
{
Expand Down Expand Up @@ -2454,7 +2454,7 @@ def test_available(self, app, get_available_engine_specs):
"sqlalchemy_uri_placeholder": "mysql://user:password@host:port/dbname[?key=value&key=value...]",
"engine_information": {
"supports_file_upload": True,
"allow_ssh_tunneling": False,
"disable_ssh_tunneling": False,
},
},
{
Expand All @@ -2464,7 +2464,7 @@ def test_available(self, app, get_available_engine_specs):
"preferred": False,
"engine_information": {
"supports_file_upload": True,
"allow_ssh_tunneling": False,
"disable_ssh_tunneling": False,
},
},
]
Expand Down Expand Up @@ -2495,7 +2495,7 @@ def test_available_no_default(self, app, get_available_engine_specs):
"preferred": True,
"engine_information": {
"supports_file_upload": True,
"allow_ssh_tunneling": False,
"disable_ssh_tunneling": False,
},
},
{
Expand All @@ -2505,7 +2505,7 @@ def test_available_no_default(self, app, get_available_engine_specs):
"preferred": False,
"engine_information": {
"supports_file_upload": True,
"allow_ssh_tunneling": False,
"disable_ssh_tunneling": False,
},
},
]
Expand Down