opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-09-01 15:37:32 +08:00
Code Issues Packages Projects Releases Wiki Activity

simplify ssh key management for ascp plugin

Browse Source
This commit is contained in:
Danielle Callan
2025-11-25 09:33:30 -05:00
committed by Nate Coraor
parent 0a2af6a287
commit eff1103c4c
5 changed files with 35 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files
doc/source/admin/data.md
+5 -7
View File
@@ -448,13 +448,11 @@ Example configuration for EBI SRA downloads:
retry_base_delay: 2.0
retry_max_delay: 60.0
enable_resume: true
# SSH key - use either ssh_key_file OR ssh_key_content
ssh_key_file: "/path/to/ssh_key_file" # path to key file
# OR
# ssh_key_content: | # embed key content
# -----BEGIN RSA PRIVATE KEY-----
# ...
# -----END RSA PRIVATE KEY-----
# SSH key content (required) - embed the key directly in the configuration
ssh_key_content: |
-----BEGIN RSA PRIVATE KEY-----
<YOUR ACTUAL SSH PRIVATE KEY CONTENT>
-----END RSA PRIVATE KEY-----
```
The plugin is **download-only** and supports automatic retry with exponential backoff for transient
lib/galaxy/config/sample/file_sources_conf.yml.sample
+5 -7
View File
@@ -83,13 +83,11 @@
retry_max_delay: 60.0 # Maximum delay between retries
enable_resume: true # Enable resume for interrupted transfers
# SSH private key for authentication (download ENA key from: https://www.ebi.ac.uk/ena/browser/about/ascp)
# Option 1: Provide path to key file (recommended)
ssh_key_file: "/path/to/your/ssh_key_file"
# Option 2: Embed key content directly (use ssh_key_content instead of ssh_key_file)
# ssh_key_content: |
# -----BEGIN RSA PRIVATE KEY-----
# <YOUR ACTUAL SSH PRIVATE KEY CONTENT>
# -----END RSA PRIVATE KEY-----
# Embed the key content directly in the configuration
ssh_key_content: |
-----BEGIN RSA PRIVATE KEY-----
<YOUR ACTUAL SSH PRIVATE KEY CONTENT>
-----END RSA PRIVATE KEY-----
# Note: This plugin is download-only (writable: false, browsable: false)
# Example: Custom Aspera endpoint with encryption enabled
lib/galaxy/files/sources/ascp.py
+6 -20
View File
@@ -11,10 +11,7 @@ The implementation is extensible to support future enhancements such as:
"""
import logging
from typing import (
Optional,
Union,
)
from typing import Union
from galaxy.files.models import (
FilesSourceRuntimeContext,
@@ -45,12 +42,11 @@ class AscpFilesSourceTemplateConfiguration(FsspecBaseFileSourceTemplateConfigura
This configuration supports template expansion for all fields, allowing
dynamic configuration based on user context or other variables.
Note: Exactly one of ssh_key_file or ssh_key_content must be provided.
Note: ssh_key_content is required for SSH authentication.
"""
ascp_path: Union[str, TemplateExpansion] = "ascp"
ssh_key_file: Union[str, TemplateExpansion, None] = None # Path to SSH key file
ssh_key_content: Union[str, TemplateExpansion, None] = None # SSH key content as string
ssh_key_content: Union[str, TemplateExpansion] # SSH key content as string (required)
user: Union[str, TemplateExpansion] # Required field
host: Union[str, TemplateExpansion] # Required field
rate_limit: Union[str, TemplateExpansion] = "300m"
@@ -67,12 +63,11 @@ class AscpFilesSourceConfiguration(FsspecBaseFileSourceConfiguration):
This configuration contains the actual values after template expansion.
Note: Exactly one of ssh_key_file or ssh_key_content must be provided.
Note: ssh_key_content is required for SSH authentication.
"""
ascp_path: str = "ascp"
ssh_key_file: Optional[str] = None # Path to SSH key file
ssh_key_content: Optional[str] = None # SSH key content as string
ssh_key_content: str # SSH key content as string (required)
user: str # Required field
host: str # Required field
rate_limit: str = "300m"
@@ -138,18 +133,9 @@ class AscpFilesSource(FsspecFilesSource[AscpFilesSourceTemplateConfiguration, As
config = context.config
# Validate that exactly one of ssh_key_file or ssh_key_content is provided
if config.ssh_key_file and config.ssh_key_content:
raise ValueError("Cannot specify both ssh_key_file and ssh_key_content. Please provide only one.")
if not config.ssh_key_file and not config.ssh_key_content:
raise ValueError("Must specify either ssh_key_file or ssh_key_content for SSH authentication.")
# Determine which key parameter to use
ssh_key = config.ssh_key_file if config.ssh_key_file else config.ssh_key_content
return AscpFileSystem(
ascp_path=config.ascp_path,
ssh_key=ssh_key,
ssh_key=config.ssh_key_content,
user=config.user,
host=config.host,
rate_limit=config.rate_limit,
lib/galaxy/files/sources/ascp_fsspec.py
+19 -36
View File
@@ -178,35 +178,19 @@ class AscpFileSystem(AbstractFileSystem):
if not self.ssh_key:
raise MessageException("SSH key is required for ascp authentication.")
# Determine if ssh_key is a file path or key content
# If it looks like a path and the file exists, use it directly
# Otherwise, treat it as key content and create a temporary file
key_is_file = False
# Create temporary file for key content
key_fd = None
key_path = None
# Check if ssh_key is a file path
if not self.ssh_key.startswith("-----BEGIN") and os.path.isfile(self.ssh_key):
# It's a file path - use it directly
key_path = self.ssh_key
key_is_file = True
# Verify permissions (should be 0600 or 0400)
stat_info = os.stat(key_path)
mode = stat_info.st_mode & 0o777
if mode not in (0o600, 0o400):
log.warning(f"SSH key file {key_path} has permissions {oct(mode)}, ascp may require 0600 or 0400")
try:
# If not using an existing file, create temporary file for key content
if not key_is_file:
key_fd, key_path = tempfile.mkstemp(suffix=".key", text=True)
# Create temporary file for key content
key_fd, key_path = tempfile.mkstemp(suffix=".key", text=True)
# Write key with proper permissions (required by ascp)
os.chmod(key_path, 0o600)
with os.fdopen(key_fd, "w") as key_file:
key_file.write(self.ssh_key)
key_fd = None # Prevent double-close
# Write key with proper permissions (required by ascp)
os.chmod(key_path, 0o600)
with os.fdopen(key_fd, "w") as key_file:
key_file.write(self.ssh_key)
key_fd = None # Prevent double-close
# Type narrowing: key_path is guaranteed to be set by this point
assert key_path is not None
@@ -255,18 +239,17 @@ class AscpFileSystem(AbstractFileSystem):
except OSError as e:
raise MessageException(f"File system error during ascp transfer: {e}") from e
finally:
# Only cleanup temporary key file (not user-provided files)
if not key_is_file:
if key_fd is not None:
try:
os.close(key_fd)
except Exception:
pass # Best effort
if key_path is not None:
try:
os.unlink(key_path)
except Exception:
pass # Best effort
# Cleanup temporary key file
if key_fd is not None:
try:
os.close(key_fd)
except Exception:
pass # Best effort
if key_path is not None:
try:
os.unlink(key_path)
except Exception:
pass # Best effort
def _is_retryable_error(self, exception: MessageException) -> bool:
"""Determine if an error is worth retrying.
test/unit/files/test_ascp.py
-76
View File
@@ -380,46 +380,6 @@ class TestAscpFilesSource:
finally:
os.unlink(config_file)
def test_ssh_key_file_usage(self):
"""Test that ssh_key_file is properly used when provided."""
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".key") as key_file:
key_file.write(TEST_SSH_KEY)
key_file_path = key_file.name
try:
os.chmod(key_file_path, 0o600)
config = {
"type": "ascp",
"id": "test_ascp",
"label": "Test Aspera",
"ssh_key_file": key_file_path,
"user": "test-user",
"host": "test.example.com",
}
with patch("shutil.which", return_value="/usr/bin/ascp"):
from ._util import configured_file_sources
with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f:
import yaml
yaml.dump([config], f)
config_file = f.name
try:
file_sources = configured_file_sources(config_file)
# Should load successfully
assert (
file_sources.get_file_source_path("ascp://test.example.com/path/to/file").file_source.id
== "test_ascp"
)
finally:
os.unlink(config_file)
finally:
if os.path.exists(key_file_path):
os.unlink(key_file_path)
class TestAscpRetryLogic:
"""Tests for retry and resume functionality."""
@@ -683,42 +643,6 @@ class TestAscpRetryLogic:
assert fs.retry_max_delay == 60.0
assert fs.enable_resume is True
def test_ssh_key_as_file_path(self):
"""Test that ssh_key can be provided as a file path."""
with patch("shutil.which", return_value="/usr/bin/ascp"):
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".key") as key_file:
key_file.write(TEST_SSH_KEY)
key_file_path = key_file.name
try:
# Set proper permissions
os.chmod(key_file_path, 0o600)
fs = AscpFileSystem(
ssh_key=key_file_path, # Pass file path instead of content
user="test-user",
host="test.example.com",
)
# Mock subprocess to verify the key file path is used directly
with patch("subprocess.run") as mock_run:
mock_run.return_value = Mock(returncode=0, stderr="", stdout="")
with patch("os.unlink") as mock_unlink:
fs._get_file("/remote/file.txt", "/local/file.txt")
# Verify the original key file was NOT deleted
# (only temporary files should be deleted)
mock_unlink.assert_not_called()
# Verify ascp was called with the original key file path
call_args = mock_run.call_args[0][0]
assert key_file_path in call_args
finally:
if os.path.exists(key_file_path):
os.unlink(key_file_path)
def test_ssh_key_as_content(self):
"""Test that ssh_key can be provided as key content (original behavior)."""
with patch("shutil.which", return_value="/usr/bin/ascp"):