Skip to content

tftp: add docs #267

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

Merged
merged 2 commits into from
Feb 10, 2025
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
2 changes: 1 addition & 1 deletion docs/source/api-reference/drivers/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ Drivers packages from the [drivers](https://github.com/jumpstarter-dev/jumpstart
can.md
pyserial.md
sdwire.md
tftp.md
ustreamer.md
yepkit.md
```
89 changes: 89 additions & 0 deletions docs/source/api-reference/drivers/tftp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# TFTP Driver

**driver**: `jumpstarter_driver_tftp.driver.Tftp`

The TFTP driver provides a read-only TFTP server that can be used to serve files.

## Driver Configuration
```yaml
export:
tftp:
type: jumpstarter_driver_tftp.driver.Tftp
config:
root_dir: /var/lib/tftpboot # Directory to serve files from
host: 192.168.1.100 # Host IP to bind to (optional)
port: 69 # Port to listen on (optional)
```

### Config parameters

| Parameter | Description | Type | Required | Default |
|-----------|-------------|------|----------|---------|
| root_dir | Root directory for the TFTP server | str | no | "/var/lib/tftpboot" |
| host | IP address to bind the server to | str | no | auto-detect |
| port | Port number to listen on | int | no | 69 |

## TftpServerClient API

```{eval-rst}
.. autoclass:: jumpstarter_driver_tftp.client.TftpServerClient()
:members:
:show-inheritance:
```

## Exception Classes

```{eval-rst}
.. autoclass:: jumpstarter_driver_tftp.driver.TftpError
:members:
:show-inheritance:

.. autoclass:: jumpstarter_driver_tftp.driver.ServerNotRunning
:members:
:show-inheritance:

.. autoclass:: jumpstarter_driver_tftp.driver.FileNotFound
:members:
:show-inheritance:
```

## Examples

```{doctest}
>>> import tempfile
>>> import os
>>> from jumpstarter_driver_tftp.driver import Tftp
>>> with tempfile.TemporaryDirectory() as tmp_dir:
... # Create a test file
... test_file = os.path.join(tmp_dir, "test.txt")
... with open(test_file, "w") as f:
... _ = f.write("hello")
...
... # Start TFTP server
... tftp = Tftp(root_dir=tmp_dir, host="127.0.0.1", port=6969)
... tftp.start()
...
... # List files
... files = tftp.list_files()
... assert "test.txt" in files
...
... tftp.stop()
```

```{testsetup} *
import tempfile
import os
from jumpstarter_driver_tftp.driver import Tftp
from jumpstarter.common.utils import serve

# Create a persistent temp dir that won't be removed by the example
TEST_DIR = tempfile.mkdtemp(prefix='tftp-test-')
instance = serve(Tftp(root_dir=TEST_DIR, host="127.0.0.1"))
client = instance.__enter__()
```

```{testcleanup} *
instance.__exit__(None, None, None)
import shutil
shutil.rmtree(TEST_DIR, ignore_errors=True)
```
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@ class FileNotFound(TftpError):

@dataclass(kw_only=True)
class Tftp(Driver):
"""TFTP Server driver for Jumpstarter"""
"""TFTP Server driver for Jumpstarter

This driver implements a TFTP read-only server.

Attributes:
root_dir (str): Root directory for the TFTP server. Defaults to "/var/lib/tftpboot"
host (str): IP address to bind the server to. If empty, will use the default route interface
port (int): Port number to listen on. Defaults to 69 (standard TFTP port)
"""
Comment on lines +32 to +40
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Update docstring to accurately reflect server capabilities.

The docstring states this is a "read-only server", but the implementation includes write operations (put_file, delete_file). This inconsistency could mislead users.

Update the docstring to accurately reflect the server's capabilities:

-    This driver implements a TFTP read-only server.
+    This driver implements a TFTP server supporting both read and write operations.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""TFTP Server driver for Jumpstarter
This driver implements a TFTP read-only server.
Attributes:
root_dir (str): Root directory for the TFTP server. Defaults to "/var/lib/tftpboot"
host (str): IP address to bind the server to. If empty, will use the default route interface
port (int): Port number to listen on. Defaults to 69 (standard TFTP port)
"""
"""TFTP Server driver for Jumpstarter
- This driver implements a TFTP read-only server.
+ This driver implements a TFTP server supporting both read and write operations.
Attributes:
root_dir (str): Root directory for the TFTP server. Defaults to "/var/lib/tftpboot"
host (str): IP address to bind the server to. If empty, will use the default route interface
port (int): Port number to listen on. Defaults to 69 (standard TFTP port)
"""

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the server is read only though. the upload is done via the driver client, not the tftp server.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bennyz I apologize for my confusion. You are absolutely correct - I now understand that while the driver provides file management capabilities through its client interface, the TFTP server itself is indeed read-only. The current docstring accurately reflects this architecture. Thank you for the clarification.


✏️ Learnings added
Learnt from: bennyz
PR: jumpstarter-dev/jumpstarter#267
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py:32-40
Timestamp: 2025-02-08T16:08:34.480Z
Learning: In the TFTP driver (jumpstarter_driver_tftp), the TFTP server component is read-only, while file management operations (upload, delete) are handled through the driver's client interface, not through the TFTP protocol itself.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


root_dir: str = "/var/lib/tftpboot"
host: str = field(default='')
Expand Down Expand Up @@ -97,6 +105,14 @@ async def _wait_for_shutdown(self):

@export
def start(self):
"""Start the TFTP server.

The server will start listening for incoming TFTP requests on the configured
host and port. If the server is already running, a warning will be logged.

Raises:
TftpError: If the server fails to start or times out during initialization
"""
if self.server_thread is not None and self.server_thread.is_alive():
self.logger.warning("TFTP server is already running")
return
Expand All @@ -116,6 +132,11 @@ def start(self):

@export
def stop(self):
"""Stop the TFTP server.

Initiates a graceful shutdown of the server and waits for all active transfers
to complete. If the server is not running, a warning will be logged.
"""
if self.server_thread is None or not self.server_thread.is_alive():
self.logger.warning("stop called - TFTP server is not running")
return
Expand All @@ -131,10 +152,28 @@ def stop(self):

@export
def list_files(self) -> list[str]:
"""List all files available in the TFTP server root directory.

Returns:
list[str]: A list of filenames present in the root directory
"""
return os.listdir(self.root_dir)

@export
async def put_file(self, filename: str, src_stream, client_checksum: str):
"""Upload a file to the TFTP server.

Args:
filename (str): Name of the file to create
src_stream: Source stream to read the file data from
client_checksum (str): SHA256 checksum of the file for verification

Returns:
str: The filename that was uploaded

Raises:
TftpError: If the file upload fails or path validation fails
"""
file_path = os.path.join(self.root_dir, filename)

try:
Expand All @@ -152,6 +191,18 @@ async def put_file(self, filename: str, src_stream, client_checksum: str):

@export
def delete_file(self, filename: str):
"""Delete a file from the TFTP server.

Args:
filename (str): Name of the file to delete

Returns:
str: The filename that was deleted

Raises:
FileNotFound: If the specified file does not exist
TftpError: If the deletion operation fails
"""
file_path = os.path.join(self.root_dir, filename)

if not os.path.exists(file_path):
Expand All @@ -165,6 +216,15 @@ def delete_file(self, filename: str):

@export
def check_file_checksum(self, filename: str, client_checksum: str) -> bool:
"""Check if a file matches the expected checksum.

Args:
filename (str): Name of the file to check
client_checksum (str): Expected SHA256 checksum

Returns:
bool: True if the file exists and matches the checksum, False otherwise
"""
file_path = os.path.join(self.root_dir, filename)
self.logger.debug(f"checking checksum for file: {filename}")
self.logger.debug(f"file path: {file_path}")
Expand All @@ -181,10 +241,20 @@ def check_file_checksum(self, filename: str, client_checksum: str) -> bool:

@export
def get_host(self) -> str:
"""Get the host address the server is bound to.

Returns:
str: The IP address or hostname
"""
return self.host

@export
def get_port(self) -> int:
"""Get the port number the server is listening on.

Returns:
int: The port number
"""
return self.port

def close(self):
Expand Down
Loading