Add ability to use exec on directories
Opened this issue · 8 comments
Use Case
When using the docker plugin, in order to run commands in a particular directory, I have to do this:
wexec docker/containers/container_name sh -c 'cd /var/ && ls'
When it would be, IMO, much more convenient and idiomatic to do this:
wexec docker/containers/container_name/fs/var ls
Describe the Solution You Would Like
Add the "exec" action to directories in the docker fs folder, where exec is equivilant to docker exec -it -w $directory_name $container_name $cmd
.
For example:
docker exec -it -w /shared/httpd/ devilbox_php_1 pwd
would be equivilant to:
wexec docker/containers/devilbox_php_1/fs/shared/httpd pwd
Thanks! I really love the project.
Thanks for opening your first issue here! We will follow up as soon as we can.
That's a great idea, I like it. I'm going to mull it over for a bit, but I don't see any way that contradicts how exec might evolve in the future.
One thing it does break is a habit I have of equating executable things with directly accessible resources, but that's a kludge I use to limit queries anyway. I think a better solution would be to have a separate property (maybe an attribute
) that reflects whether the entry is directly addressable or requires indirection to access.
Thanks for the blazing fast response! 😄 If you need any help implementing this on the docker plugin, just point me in the right direction and I can start a PR.
I definitely won't get to it immediately, so that would be great.
I'd suggest starting by taking a look at https://github.com/puppetlabs/wash/blob/master/CORE_PLUGIN_DEVELOPMENT.md. The Docker plugin includes its view of the filesystem at https://github.com/puppetlabs/wash/blob/master/plugin/docker/container.go#L92-L94; all plugins actually use a shared implementation to view filesystems: https://pkg.go.dev/github.com/puppetlabs/wash/volume?tab=doc via NewFS
.
The volume.FS type implements volume.Interface. Calling List
on volume.FS
lists the "root" of the filesystem as an array of volume.dir
types. I think these are what we want to make executable; when you call wexec docker/containers/container_name/fs/var ls
, you're running ls
in the /var
directory; the same wouldn't make sense for a file.
volume.FS
is used to represent a filesystem that can be explored via Exec
, so it already has something that we can exec against. volume.dir
is also used to represent other things like persistent volumes that don't have a directly relationship to something that can be executed. So I think we'd want to create a new type like execableDir
that's used just with volume.FS
and wraps volume.Dir
. It would look something like
type execableDir struct {
dir
impl execableInterface
}
func (v *execableDir) Exec(ctx context.Context, cmd string, args []string, opts plugin.ExecOptions) (plugin.ExecCommand, error) {
return v.impl.VolumeExec(ctx, v.path, cmd, args, opts)
}
type execableInterface interface {
Interface
VolumeExec(ctx context.Context, path string, cmd string, args []string, opts plugin.ExecOptions) (ExecCommand, error)
}
volume.FS
should be updated to implement VolumeExec
. The purpose of VolumeExec
is to update the command to change to the specified path before executing it via the plugin.Execable
that volume.FS
has a reference to. By implementing the Execable interface, execableDir
communicates to Wash that it can execute a command triggered by wexec
.
You'll also need to override List
in execableDir
to create new execableDir
s instead of normal dir
s, and possibly also change what ChildSchemas
returns for FS
and execableDir
.
My usual process for implementing this would be:
- Create
execableDir
wrappingdir
and use it involume.FS
in place ofdir
. Also updateChildSchemas
calls. Thengo run wash.go
and look atstree
output to confirm the schema change. - Implement a dummy
Exec
onexecableDir
, run Wash again, and look atls
output to confirm that actions now includeexec
. You can also addvar _ = plugin.Execable(&execableDir{})
for a static assertion that you have the correctExec
signature. - Finish implementing
Exec
onexecableDir
andVolumeExec
onFS
. Try it out. Work on writing some unit tests around them.
One extra factor is that FS
has the concept of a login shell. So ideally you'd write your VolumeExec
to handle both POSIX-compatible shell and PowerShell syntax (look at other Volume*
commands for how they select implementation based on login shell).
Thanks for the in-depth information! Sorry to keep bugging you, but how should I handle the VolumeExec
function? I'm assuming that I should return d.executor.Exec
from it, but how do I handle exec'ing in the directories? Ideally I'd think that would be implemented in the plugins themselves, right?
I was thinking it'd be something like prepending cd <dir> && <cmd>
, but maybe that wouldn't be sufficient. It does seem likely that plugins might have different ways to determine the working directory they execute in.
I'd add that as a new option to plugin.ExecOptions
. Sounds like enabling this would also need to be opt-in when calling volume.NewFS
, which the plugin should only do if it handles the new "working directory" option in Exec
.
Adding WorkingDir
to plugin.ExecOptions
does the trick! Got it working with this:
// volume/fs.go
func (d *FS) VolumeExec(ctx context.Context, path string, cmd string, args []string, opts plugin.ExecOptions) (plugin.ExecCommand, error) {
opts.WorkingDir = path
return d.executor.Exec(ctx, cmd, args, opts)
}
//docker/container.go
func (c *container) Exec(ctx context.Context, cmd string, args []string, opts plugin.ExecOptions) (plugin.ExecCommand, error) {
// ...
cfg := types.ExecConfig{Cmd: command, AttachStdout: true, AttachStderr: true, Tty: opts.Tty, WorkingDir: opts.WorkingDir}
// ...
}
And with that, docker is working. Now I just need to implement that on the other plugins, which I'm not terribly familiar with.
It's worth noting that although it is working, I'm not seeing [execDir]
when I call stree.
Nice work! The volume
plugin can also be used by external plugins. That's why I suggest having an option to enable this mode in volume.NewFS
, so we can incrementally add it to other plugins, and external plugins can choose whether or not to support it.
Oh, the name used comes from the entry schema, so you probably need to override https://github.com/puppetlabs/wash/blob/master/volume/dir.go#L32-L34 on execDir
.