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

Add sshfs exporter class #1130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JoshuaWatt
Copy link
Contributor

Adds a class that allows tests to export a local directory to an exporter via sshfs.

In order to prevent the exporter from being able to access any file that the user has permission for on the file system (which would allow it to read SSH keys for example), the SFTP server is run in an isolated container by default using podman, although this can be overridden to use docker or directly execute the SFTP server if the exporters are trusted.

Description

Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • Add a section on how to use the feature to doc/usage.rst
  • Add a section on how to use the feature to doc/development.rst
  • PR has been tested
  • Man pages have been regenerated

Adds a class that allows tests to export a local directory to an
exporter via sshfs.

In order to prevent the exporter from being able to access any file that
the user has permission for on the file system (which would allow it to
read SSH keys for example), the SFTP server is run in an isolated
container by default using podman, although this can be overridden to
use docker or directly execute the SFTP server if the exporters are
trusted.

Signed-off-by: Joshua Watt <Joshua.Watt@garmin.com>
Copy link
Member

@jluebbe jluebbe left a comment

Choose a reason for hiding this comment

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

Looks good otherwise.

Comment on lines +124 to +130

#
# SFTP server
#
FROM alpine:3.17 as labgrid-sftp-server

RUN apk add openssh-sftp-server
Copy link
Member

Choose a reason for hiding this comment

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

It seems that the podman in Ubuntu 22.04 tries to build the other targets as well, so this should be a different Dockerfile.

from ..resource.common import Resource, NetworkResource


DEFAULT_SFTP_SERVER = "podman run --rm -i --mount type=bind,source={path},target={path} labgrid/sftp-server:latest usr/lib/ssh/sftp-server -e {readonly}"
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, how do we want to tell the user that we expect this image to exist locally? Do we want to have a runtime check for the image or is documentation enough?

Comment on lines +32 to +34
resource = attr.ib(
validator=attr.validators.instance_of(Resource),
)
Copy link
Member

Choose a reason for hiding this comment

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

We might want to connect to an exporter for a specific resource (similar to ManagedFile) or to the target as well. So I'd prefer to pass host/port instead of a resource. That would mean passing a config separately as well.

@JoshuaWatt Do you see a downside?

@Emantor Emantor added the needs rebase Needs a rebase onto the master branch, maintainter could probably not push to submitter branch. label Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs rebase Needs a rebase onto the master branch, maintainter could probably not push to submitter branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants