Merge branch 'master' into tdd/issue-24180-ssh-ed25519

- add circular-import justification comment on the deferred get_default_port import
- drop no-op continue in the per-type loop
- add NOTE clarifying last_exc reflects the final (RSAKey) attempt
- clarify docstring re paramiko 3.2+ PKey.from_path() and why it's unsafe here
- strengthen ed25519/RSA tests with isinstance assertions on the parsed key type
- add test for passphrase-protected key without password (PasswordRequiredException)
- add test for an unparseable key (SSHException listing all attempted types)
- de-duplicate test setup via _make_manager / _make_ssh_tunnel helpers
- consistent assertion messages across key-type tests

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

helm/superset/Chart.yaml

@@ -29,7 +29,7 @@ maintainers:
   - name: craig-rueda
     email: craig@craigrueda.com
     url: https://github.com/craig-rueda
-version: 0.17.0 # See [README](https://github.com/apache/superset/blob/master/helm/superset/README.md#versioning) for version details.
+version: 0.16.0 # See [README](https://github.com/apache/superset/blob/master/helm/superset/README.md#versioning) for version details.
 dependencies:
   - name: postgresql
     version: 16.7.27

helm/superset/README.md

@@ -23,7 +23,7 @@ NOTE: This file is generated by helm-docs: https://github.com/norwoodj/helm-docs
 
 # superset
 
-![Version: 0.17.0](https://img.shields.io/badge/Version-0.17.0-informational?style=flat-square)
+![Version: 0.16.0](https://img.shields.io/badge/Version-0.16.0-informational?style=flat-square)
 
 Apache Superset is a modern, enterprise-ready business intelligence web application
 
@@ -111,6 +111,9 @@ On helm this can be set on `extraSecretEnv.SUPERSET_SECRET_KEY` or `configOverri
 | init.resources | object | `{}` |  |
 | init.tolerations | list | `[]` |  |
 | init.topologySpreadConstraints | list | `[]` | TopologySpreadConstrains to be added to init job |
+| initImage.pullPolicy | string | `"IfNotPresent"` |  |
+| initImage.repository | string | `"apache/superset"` |  |
+| initImage.tag | string | `"dockerize"` |  |
 | nameOverride | string | `nil` | Provide a name to override the name of the chart |
 | nodeSelector | object | `{}` |  |
 | postgresql | object | see `values.yaml` | Configuration values for the postgresql dependency. ref: https://github.com/bitnami/charts/tree/main/bitnami/postgresql |

helm/superset/values.yaml

@@ -194,6 +194,11 @@ image:
 
 imagePullSecrets: []
 
+initImage:
+  repository: apache/superset
+  tag: dockerize
+  pullPolicy: IfNotPresent
+
 service:
   type: ClusterIP
   port: 8088
@@ -298,28 +303,15 @@ supersetNode:
   # @default -- a container waiting for postgres
   initContainers:
     - name: wait-for-postgres
-      image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
-      imagePullPolicy: "{{ .Values.image.pullPolicy }}"
+      image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
+      imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
       envFrom:
         - secretRef:
             name: "{{ tpl .Values.envFromSecret . }}"
       command:
-        - /bin/bash
+        - /bin/sh
         - -c
-        - |
-          # bash's /dev/tcp redirect performs a TCP connect; no external
-          # `dockerize`, `nc`, or busybox needed. SECONDS-based deadline
-          # mirrors the prior `dockerize -timeout 120s` behaviour.
-          SECONDS=0
-          until (echo > /dev/tcp/"$DB_HOST"/"$DB_PORT") 2>/dev/null; do
-            if [ "$SECONDS" -ge 120 ]; then
-              echo "timeout waiting for postgres at $DB_HOST:$DB_PORT after 120s" >&2
-              exit 1
-            fi
-            echo "waiting for postgres at $DB_HOST:$DB_PORT (elapsed ${SECONDS}s)"
-            sleep 2
-          done
-          echo "postgres at $DB_HOST:$DB_PORT is up"
+        - dockerize -wait "tcp://$DB_HOST:$DB_PORT" -timeout 120s
       resources:
         limits:
           memory: "256Mi"
@@ -415,31 +407,15 @@ supersetWorker:
   # @default -- a container waiting for postgres and redis
   initContainers:
     - name: wait-for-postgres-redis
-      image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
-      imagePullPolicy: "{{ .Values.image.pullPolicy }}"
+      image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
+      imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
       envFrom:
         - secretRef:
             name: "{{ tpl .Values.envFromSecret . }}"
       command:
-        - /bin/bash
+        - /bin/sh
         - -c
-        - |
-          # See supersetNode.initContainers for the rationale.
-          SECONDS=0
-          wait_for() {
-            local host=$1 port=$2 name=$3
-            until (echo > /dev/tcp/"$host"/"$port") 2>/dev/null; do
-              if [ "$SECONDS" -ge 120 ]; then
-                echo "timeout waiting for $name at $host:$port after 120s" >&2
-                exit 1
-              fi
-              echo "waiting for $name at $host:$port (elapsed ${SECONDS}s)"
-              sleep 2
-            done
-            echo "$name at $host:$port is up"
-          }
-          wait_for "$DB_HOST" "$DB_PORT" postgres
-          wait_for "$REDIS_HOST" "$REDIS_PORT" redis
+        - dockerize -wait "tcp://$DB_HOST:$DB_PORT" -wait "tcp://$REDIS_HOST:$REDIS_PORT" -timeout 120s
       resources:
         limits:
           memory: "256Mi"
@@ -519,31 +495,15 @@ supersetCeleryBeat:
   # @default -- a container waiting for postgres
   initContainers:
     - name: wait-for-postgres-redis
-      image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
-      imagePullPolicy: "{{ .Values.image.pullPolicy }}"
+      image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
+      imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
       envFrom:
         - secretRef:
             name: "{{ tpl .Values.envFromSecret . }}"
       command:
-        - /bin/bash
+        - /bin/sh
         - -c
-        - |
-          # See supersetNode.initContainers for the rationale.
-          SECONDS=0
-          wait_for() {
-            local host=$1 port=$2 name=$3
-            until (echo > /dev/tcp/"$host"/"$port") 2>/dev/null; do
-              if [ "$SECONDS" -ge 120 ]; then
-                echo "timeout waiting for $name at $host:$port after 120s" >&2
-                exit 1
-              fi
-              echo "waiting for $name at $host:$port (elapsed ${SECONDS}s)"
-              sleep 2
-            done
-            echo "$name at $host:$port is up"
-          }
-          wait_for "$DB_HOST" "$DB_PORT" postgres
-          wait_for "$REDIS_HOST" "$REDIS_PORT" redis
+        - dockerize -wait "tcp://$DB_HOST:$DB_PORT" -wait "tcp://$REDIS_HOST:$REDIS_PORT" -timeout 120s
       resources:
         limits:
           memory: "256Mi"
@@ -634,31 +594,15 @@ supersetCeleryFlower:
   # @default -- a container waiting for postgres and redis
   initContainers:
     - name: wait-for-postgres-redis
-      image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
-      imagePullPolicy: "{{ .Values.image.pullPolicy }}"
+      image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
+      imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
       envFrom:
         - secretRef:
             name: "{{ tpl .Values.envFromSecret . }}"
       command:
-        - /bin/bash
+        - /bin/sh
         - -c
-        - |
-          # See supersetNode.initContainers for the rationale.
-          SECONDS=0
-          wait_for() {
-            local host=$1 port=$2 name=$3
-            until (echo > /dev/tcp/"$host"/"$port") 2>/dev/null; do
-              if [ "$SECONDS" -ge 120 ]; then
-                echo "timeout waiting for $name at $host:$port after 120s" >&2
-                exit 1
-              fi
-              echo "waiting for $name at $host:$port (elapsed ${SECONDS}s)"
-              sleep 2
-            done
-            echo "$name at $host:$port is up"
-          }
-          wait_for "$DB_HOST" "$DB_PORT" postgres
-          wait_for "$REDIS_HOST" "$REDIS_PORT" redis
+        - dockerize -wait "tcp://$DB_HOST:$DB_PORT" -wait "tcp://$REDIS_HOST:$REDIS_PORT" -timeout 120s
       resources:
         limits:
           memory: "256Mi"
@@ -820,26 +764,15 @@ init:
   # @default -- a container waiting for postgres
   initContainers:
     - name: wait-for-postgres
-      image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
-      imagePullPolicy: "{{ .Values.image.pullPolicy }}"
+      image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
+      imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
       envFrom:
         - secretRef:
             name: "{{ tpl .Values.envFromSecret . }}"
       command:
-        - /bin/bash
+        - /bin/sh
         - -c
-        - |
-          # See supersetNode.initContainers for the rationale.
-          SECONDS=0
-          until (echo > /dev/tcp/"$DB_HOST"/"$DB_PORT") 2>/dev/null; do
-            if [ "$SECONDS" -ge 120 ]; then
-              echo "timeout waiting for postgres at $DB_HOST:$DB_PORT after 120s" >&2
-              exit 1
-            fi
-            echo "waiting for postgres at $DB_HOST:$DB_PORT (elapsed ${SECONDS}s)"
-            sleep 2
-          done
-          echo "postgres at $DB_HOST:$DB_PORT is up"
+        - dockerize -wait "tcp://$DB_HOST:$DB_PORT" -timeout 120s
       resources:
         limits:
           memory: "256Mi"

superset/extensions/ssh.py

@@ -21,7 +21,14 @@ from typing import TYPE_CHECKING
 
 import sshtunnel
 from flask import Flask
-from paramiko import RSAKey
+from paramiko import (
+    ECDSAKey,
+    Ed25519Key,
+    PasswordRequiredException,
+    PKey,
+    RSAKey,
+    SSHException,
+)
 
 from superset.commands.database.ssh_tunnel.exceptions import SSHTunnelDatabasePortError
 from superset.databases.utils import make_url_safe
@@ -30,6 +37,39 @@ from superset.utils.class_utils import load_class_from_name
 if TYPE_CHECKING:
     from superset.databases.ssh_tunnel.models import SSHTunnel
 
+# Order matters: paramiko's per-class loaders raise SSHException with vague
+# "unpack requires 4 bytes" messages on type mismatches, so we try the more
+# modern key types first (ed25519, ECDSA) and fall back to RSA, which is the
+# most permissive parser and the historical default in this codebase.
+_SSH_KEY_TYPES: tuple[type[PKey], ...] = (Ed25519Key, ECDSAKey, RSAKey)
+
+
+def _load_private_key(pem: str, password: str | None) -> PKey:
+    """Load a private key PEM regardless of algorithm (ed25519, ECDSA, RSA).
+
+    paramiko 3.2+ has ``PKey.from_path()`` for polymorphic loading, but it
+    requires a filesystem path; writing private key material to disk would be a
+    security regression. Each per-class loader only accepts its own format, so
+    iterate over the supported types on the in-memory ``StringIO`` and return
+    the first that parses cleanly.
+    """
+    last_exc: SSHException | None = None
+    for key_class in _SSH_KEY_TYPES:
+        try:
+            return key_class.from_private_key(StringIO(pem), password=password)
+        except PasswordRequiredException:
+            raise
+        except SSHException as exc:
+            last_exc = exc
+    # NOTE: last_exc holds the error from the final attempt (RSAKey), not the
+    # closest-matching type. For a corrupted ed25519 key, the appended message
+    # reflects RSAKey's parse error; the full type list above still identifies
+    # all types attempted.
+    raise SSHException(
+        "Unable to parse SSH private key as any of "
+        f"{', '.join(k.__name__ for k in _SSH_KEY_TYPES)}: {last_exc}"
+    ) from last_exc
+
 
 class SSHManager:
     def __init__(self, app: Flask) -> None:
@@ -53,6 +93,9 @@ class SSHManager:
         ssh_tunnel: "SSHTunnel",
         sqlalchemy_database_uri: str,
     ) -> sshtunnel.SSHTunnelForwarder:
+        # Deferred import to break a circular import:
+        # superset.utils.ssh_tunnel -> superset.databases.ssh_tunnel.models
+        # -> superset.extensions -> superset.extensions.ssh (this module).
         from superset.utils.ssh_tunnel import get_default_port
 
         url = make_url_safe(sqlalchemy_database_uri)
@@ -71,11 +114,9 @@ class SSHManager:
         if ssh_tunnel.password:
             params["ssh_password"] = ssh_tunnel.password
         elif ssh_tunnel.private_key:
-            private_key_file = StringIO(ssh_tunnel.private_key)
-            private_key = RSAKey.from_private_key(
-                private_key_file, ssh_tunnel.private_key_password
+            params["ssh_pkey"] = _load_private_key(
+                ssh_tunnel.private_key, ssh_tunnel.private_key_password
             )
-            params["ssh_pkey"] = private_key
 
         return sshtunnel.open_tunnel(**params)
 

tests/unit_tests/extensions/ssh_test.py

@@ -14,11 +14,45 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
+import pytest
 import sshtunnel
+from cryptography.hazmat.primitives.asymmetric import ec
+from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey
+from cryptography.hazmat.primitives.asymmetric.rsa import (
+    generate_private_key as generate_rsa_key,
+)
+from cryptography.hazmat.primitives.serialization import (
+    BestAvailableEncryption,
+    Encoding,
+    NoEncryption,
+    PrivateFormat,
+)
+from paramiko import Ed25519Key, PasswordRequiredException, RSAKey, SSHException
 
-from superset.extensions.ssh import SSHManagerFactory
+from superset.extensions.ssh import SSHManager, SSHManagerFactory
+
+
+def _make_manager() -> SSHManager:
+    app = Mock()
+    app.config = {
+        "SSH_TUNNEL_LOCAL_BIND_ADDRESS": "127.0.0.1",
+        "SSH_TUNNEL_TIMEOUT_SEC": 10.0,
+        "SSH_TUNNEL_PACKET_TIMEOUT_SEC": 10.0,
+    }
+    return SSHManager(app)
+
+
+def _make_ssh_tunnel(private_key: str, private_key_password: str | None = None) -> Mock:
+    ssh_tunnel = Mock()
+    ssh_tunnel.server_address = "ssh.example.com"
+    ssh_tunnel.server_port = 22
+    ssh_tunnel.username = "tunneluser"
+    ssh_tunnel.password = None
+    ssh_tunnel.private_key = private_key
+    ssh_tunnel.private_key_password = private_key_password
+    return ssh_tunnel
 
 
 def test_ssh_tunnel_timeout_setting() -> None:
@@ -34,3 +68,151 @@ def test_ssh_tunnel_timeout_setting() -> None:
     factory.init_app(app)
     assert sshtunnel.TUNNEL_TIMEOUT == 123.0
     assert sshtunnel.SSH_TIMEOUT == 321.0
+
+
+def _make_ed25519_pem() -> str:
+    """Generate a fresh OpenSSH-format ed25519 private key PEM."""
+    key = Ed25519PrivateKey.generate()
+    return key.private_bytes(
+        encoding=Encoding.PEM,
+        format=PrivateFormat.OpenSSH,
+        encryption_algorithm=NoEncryption(),
+    ).decode()
+
+
+def test_create_tunnel_accepts_ed25519_private_key() -> None:
+    """
+    Regression for #24180: ed25519 SSH keys must be loadable for tunnel
+    setup. The bug surfaces as ``unpack requires 4 bytes`` because the
+    code unconditionally calls ``RSAKey.from_private_key`` regardless of
+    key type, which mis-parses the ed25519 byte stream.
+
+    The Superset UI accepts any key the user pastes, so the fix is to
+    detect the key type (or use ``paramiko.PKey.from_private_key`` once
+    paramiko exposes a polymorphic loader) rather than hard-coding RSA.
+
+    This test exercises ``create_tunnel`` end-to-end with a freshly
+    generated ed25519 key. It mocks ``sshtunnel.open_tunnel`` so the
+    test does not actually open a network connection — only the key
+    parsing path is exercised.
+    """
+    manager = _make_manager()
+    ssh_tunnel = _make_ssh_tunnel(_make_ed25519_pem())
+
+    with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+        manager.create_tunnel(
+            ssh_tunnel, "postgresql://user:pass@db.example.com:5432/x"
+        )
+
+    # Key-type-agnostic loader must produce a paramiko PKey usable as ssh_pkey.
+    assert mock_open.called, "open_tunnel was never invoked — key parsing aborted"
+    forwarded_pkey = mock_open.call_args.kwargs["ssh_pkey"]
+    assert isinstance(forwarded_pkey, Ed25519Key)
+
+
+def test_create_tunnel_accepts_rsa_private_key_unchanged() -> None:
+    """
+    Companion to test_create_tunnel_accepts_ed25519_private_key: pin the
+    backward-compatible RSA path so a fix for ed25519 doesn't regress the
+    historically-supported RSA case. Uses a freshly generated RSA key in
+    OpenSSH format.
+    """
+    rsa_pem = (
+        generate_rsa_key(public_exponent=65537, key_size=2048)
+        .private_bytes(
+            encoding=Encoding.PEM,
+            format=PrivateFormat.OpenSSH,
+            encryption_algorithm=NoEncryption(),
+        )
+        .decode()
+    )
+
+    manager = _make_manager()
+    ssh_tunnel = _make_ssh_tunnel(rsa_pem)
+
+    with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+        manager.create_tunnel(
+            ssh_tunnel, "postgresql://user:pass@db.example.com:5432/x"
+        )
+
+    assert mock_open.called, "open_tunnel was never invoked — RSA key parsing aborted"
+    assert isinstance(mock_open.call_args.kwargs["ssh_pkey"], RSAKey)
+
+
+def test_create_tunnel_accepts_ecdsa_private_key() -> None:
+    """
+    Companion to the ed25519 and RSA tests: verify ECDSAKey (the third type
+    in _SSH_KEY_TYPES) is reachable by _load_private_key. Uses NIST P-256,
+    the most common ECDSA curve in practice.
+    """
+    ecdsa_pem = (
+        ec.generate_private_key(ec.SECP256R1())
+        .private_bytes(
+            encoding=Encoding.PEM,
+            format=PrivateFormat.OpenSSH,
+            encryption_algorithm=NoEncryption(),
+        )
+        .decode()
+    )
+
+    manager = _make_manager()
+    ssh_tunnel = _make_ssh_tunnel(ecdsa_pem)
+
+    with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+        manager.create_tunnel(
+            ssh_tunnel, "postgresql://user:pass@db.example.com:5432/x"
+        )
+
+    assert mock_open.called, "open_tunnel was never invoked — ECDSA key parsing aborted"
+    assert mock_open.call_args.kwargs["ssh_pkey"] is not None
+
+
+def test_create_tunnel_passphrase_protected_key_without_password() -> None:
+    """
+    A passphrase-protected key supplied without a passphrase must surface as
+    ``PasswordRequiredException`` (an actionable "key requires passphrase"
+    signal) rather than being absorbed by the per-type loop and reported as a
+    generic "Unable to parse" error.
+    """
+    encrypted_pem = (
+        Ed25519PrivateKey.generate()
+        .private_bytes(
+            encoding=Encoding.PEM,
+            format=PrivateFormat.OpenSSH,
+            encryption_algorithm=BestAvailableEncryption(b"correct horse"),
+        )
+        .decode()
+    )
+
+    manager = _make_manager()
+    ssh_tunnel = _make_ssh_tunnel(encrypted_pem, private_key_password=None)
+
+    with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+        with pytest.raises(PasswordRequiredException):
+            manager.create_tunnel(
+                ssh_tunnel, "postgresql://user:pass@db.example.com:5432/x"
+            )
+
+    assert not mock_open.called
+
+
+def test_create_tunnel_invalid_key_raises_combined_error() -> None:
+    """
+    When a key parses as none of the supported types, ``_load_private_key``
+    raises ``SSHException`` whose message lists every type that was attempted,
+    so the failure clearly communicates that all loaders were tried.
+    """
+    manager = _make_manager()
+    ssh_tunnel = _make_ssh_tunnel("not a valid private key")
+
+    with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+        with pytest.raises(SSHException) as exc_info:
+            manager.create_tunnel(
+                ssh_tunnel, "postgresql://user:pass@db.example.com:5432/x"
+            )
+
+    message = str(exc_info.value)
+    assert "Ed25519Key" in message
+    assert "ECDSAKey" in message
+    assert "RSAKey" in message
+    assert not mock_open.called