Skip to content

Commit

Permalink
BREAKING CHANGE(sh): only run rbmk & built-in commands (#44)
Browse files Browse the repository at this point in the history
This diff modifies how `rbmk sh` executes commands by only allowing
internal commands (`pwd`, `cd`) and `rbmk` commands, which are also
exported as internal commands.

The main motivation for implementing this change is ensuring that a
script running on, say, Linux, would also work on, say, Windows, without
issues caused by missing commands.

Figuring out whether the decision to implement this change was motivated
by theoretical concerns or hands on experience is left as an exercise
for the reader 😬.

To preserve backward compatibility, we set `$RBMK_EXE` to `rbmk` thus
allowing previous script to work as long as they were using the `rbmk`
command only. Scripts using external commands would stop working, hence
this is still a breaking change.

See the updated documentation in pkg/cli/sh/README.md for more details
about this change and guidance on writing portable scripts.
  • Loading branch information
bassosimone authored Dec 10, 2024
1 parent 699abaf commit 93815ff
Show file tree
Hide file tree
Showing 7 changed files with 270 additions and 66 deletions.
30 changes: 23 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,30 @@ you can observe each operation in isolation.
- `nc`: TCP/TLS endpoint measurements
- `stun`: Resolve the public IP addresses

- Scripting Support:
- Built-in POSIX shell interpreter
- Cross-platform Unix-like subcommands (e.g., `cat`, `tar`, `mv`)
- Script generation tools

The tool is designed to support both general use and measurement-specific
features, with support for scripting concurrent operations and
extensive integration testing capabilities.
features, with support for scripting and extensive integration testing
capabilities through the [internal/qa](internal/qa) package.

### Portable Scripting Support

RBMK provides a POSIX-compliant shell environment through `rbmk sh` that
guarantees script portability:

```bash
$ rbmk sh measurement.sh
```

Key features:

- Scripts only use `rbmk` commands as built-in commands
- Executing external commands is not possible
- Cross-platform Unix-like built-in subcommands (e.g., `rbmk tar`, `rbmk mv`)
- Identical behavior across Unix-like systems and Windows
- Develop locally, deploy anywhere without modification
- No surprises caused by missing or different external tools

This design ensures that measurement scripts work consistently across
different environments, eliminating common portability issues.

## Minimum Go version

Expand Down
File renamed without changes.
77 changes: 77 additions & 0 deletions internal/rootcmd/rootcmd.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// SPDX-License-Identifier: GPL-3.0-or-later

/*
Package rootcmd contains shared code to
implement the `rbmk` command.
This code is shared between the following packages:
1. [github.com/rbmk-project/rbmk/pkg/cli/sh].
2. [github.com/rbmk-project/rbmk/pkg/cli].
The former package implements the `rbmk sh` command,
while the latter implements the `rbmk` command.
Both packages need to have access to the list of internal
commands as well as to the text printed on `--help`.
*/
package rootcmd

import (
_ "embed"

"github.com/rbmk-project/common/cliutils"
"github.com/rbmk-project/rbmk/pkg/cli/cat"
"github.com/rbmk-project/rbmk/pkg/cli/curl"
"github.com/rbmk-project/rbmk/pkg/cli/dig"
"github.com/rbmk-project/rbmk/pkg/cli/intro"
"github.com/rbmk-project/rbmk/pkg/cli/ipuniq"
"github.com/rbmk-project/rbmk/pkg/cli/mkdir"
"github.com/rbmk-project/rbmk/pkg/cli/mv"
"github.com/rbmk-project/rbmk/pkg/cli/nc"
"github.com/rbmk-project/rbmk/pkg/cli/pipe"
"github.com/rbmk-project/rbmk/pkg/cli/rm"
"github.com/rbmk-project/rbmk/pkg/cli/stun"
"github.com/rbmk-project/rbmk/pkg/cli/tar"
"github.com/rbmk-project/rbmk/pkg/cli/timestamp"
"github.com/rbmk-project/rbmk/pkg/cli/tutorial"
"github.com/rbmk-project/rbmk/pkg/cli/version"
)

//go:embed README.md
var readme string

// HelpText returns the text to be printed when the `--help`
// flag is passed to the `rbmk` command. The text may be rendered
// using the markdown package, depending on the build tags.
func HelpText() string {
return readme
}

// CommandsWithoutSh returns a new map containing the mapping
// between the name of a command and the command itself.
//
// The returned map DOES NOT include the `sh` command, which
// is a special case, which the packages using this func could
// choose to implement differently (and how they choose to
// implement it is not this function's concern anyway).
func CommandsWithoutSh() map[string]cliutils.Command {
return map[string]cliutils.Command{
"cat": cat.NewCommand(),
"curl": curl.NewCommand(),
"dig": dig.NewCommand(),
"intro": intro.NewCommand(),
"ipuniq": ipuniq.NewCommand(),
"mkdir": mkdir.NewCommand(),
"mv": mv.NewCommand(),
"nc": nc.NewCommand(),
"pipe": pipe.NewCommand(),
"rm": rm.NewCommand(),
"stun": stun.NewCommand(),
"tar": tar.NewCommand(),
"timestamp": timestamp.NewCommand(),
"tutorial": tutorial.NewCommand(),
"version": version.NewCommand(),
}
}
41 changes: 4 additions & 37 deletions pkg/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,47 +8,14 @@ import (

"github.com/rbmk-project/common/cliutils"
"github.com/rbmk-project/rbmk/internal/markdown"
"github.com/rbmk-project/rbmk/pkg/cli/cat"
"github.com/rbmk-project/rbmk/pkg/cli/curl"
"github.com/rbmk-project/rbmk/pkg/cli/dig"
"github.com/rbmk-project/rbmk/pkg/cli/intro"
"github.com/rbmk-project/rbmk/pkg/cli/ipuniq"
"github.com/rbmk-project/rbmk/pkg/cli/mkdir"
"github.com/rbmk-project/rbmk/pkg/cli/mv"
"github.com/rbmk-project/rbmk/pkg/cli/nc"
"github.com/rbmk-project/rbmk/pkg/cli/pipe"
"github.com/rbmk-project/rbmk/pkg/cli/rm"
"github.com/rbmk-project/rbmk/internal/rootcmd"
"github.com/rbmk-project/rbmk/pkg/cli/sh"
"github.com/rbmk-project/rbmk/pkg/cli/stun"
"github.com/rbmk-project/rbmk/pkg/cli/tar"
"github.com/rbmk-project/rbmk/pkg/cli/timestamp"
"github.com/rbmk-project/rbmk/pkg/cli/tutorial"
"github.com/rbmk-project/rbmk/pkg/cli/version"
)

//go:embed README.md
var readme string

// NewCommand constructs a new [cliutils.Command] for the `rbmk` command.
func NewCommand() cliutils.Command {
directory := rootcmd.CommandsWithoutSh()
directory["sh"] = sh.NewCommand()
return cliutils.NewCommandWithSubCommands(
"rbmk", markdown.LazyMaybeRender(readme),
map[string]cliutils.Command{
"cat": cat.NewCommand(),
"curl": curl.NewCommand(),
"dig": dig.NewCommand(),
"intro": intro.NewCommand(),
"ipuniq": ipuniq.NewCommand(),
"mkdir": mkdir.NewCommand(),
"mv": mv.NewCommand(),
"nc": nc.NewCommand(),
"pipe": pipe.NewCommand(),
"rm": rm.NewCommand(),
"sh": sh.NewCommand(),
"stun": stun.NewCommand(),
"tar": tar.NewCommand(),
"timestamp": timestamp.NewCommand(),
"tutorial": tutorial.NewCommand(),
"version": version.NewCommand(),
})
"rbmk", markdown.LazyMaybeRender(rootcmd.HelpText()), directory)
}
41 changes: 30 additions & 11 deletions pkg/cli/sh/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,28 @@ across operating systems and supports:

- Environment variables


## Available Commands

Apart from built-in commands (e.g., `cd`, `test`), the shell will
only allow running the `rbmk` command, which will behave as when you
normaly execute `rbmk`, except that `rbmk sh` won't be available.

We do this to restrict the set of commands that `rbmk sh` could run
and ensure scripts are portable. If you have more complex measurement
needs, we recommend using GNU bash instead.

## Environment

The `rbmk sh` command inherits the parent environment and includes the
following environment variables:

### `RBMK_EXE`

Automatically set to the absolute path of the `rbmk` executable to
help the script invoke `rbmk` commands.
Automatically set to `rbmk` to allow scripts written before RBMK
v0.7.0 to continue running without modification. Since v0.7.0, `rbmk sh`
cannot execute external commands and is only allowed to run shell
built-in commands and the `rbmk` command.

## Example

Expand All @@ -51,18 +64,15 @@ First, let's see the content of the the `script.bash` file:
```sh
#!/bin/bash
set -x
timestamp=$("${RBMK_EXE}" timestamp)
timestamp=$(rbmk timestamp)
outdir="$timestamp"
"${RBMK_EXE}" mkdir -p "$outdir"
"${RBMK_EXE}" dig +short=ip A "dns.google" > "$outdir/dig1.txt"
"${RBMK_EXE}" dig +short=ip AAAA "dns.google" > "$outdir/dig2.txt"
"${RBMK_EXE}" tar -czf "results_$timestamp.tar.gz" "$outdir"
"${RBMK_EXE}" rm -rf "$outdir"
rbmk mkdir -p "$outdir"
rbmk dig +short=ip A "dns.google" > "$outdir/dig1.txt"
rbmk dig +short=ip AAAA "dns.google" > "$outdir/dig2.txt"
rbmk tar -czf "results_$timestamp.tar.gz" "$outdir"
rbmk rm -rf "$outdir"
```

Note that we use the `${RBMK_EXE}` environment variable to invoke `rbmk`
indirectly, which is useful when `rbmk` is not in the `PATH`.

To execute the script using `rbmk sh` run:

```
Expand All @@ -72,3 +82,12 @@ $ rbmk sh script.bash
## Exit Status

This command exits with `0` on success and `1` on failure.

## History

Before RBMK v0.7.0, `rbmk sh` used to set the `$RBMK_EXE` environment
variable to its path, to allow a script to execute `rbmk` commands.

Since v0.7.0. `rbmk` is an internal shell command, `rbmk sh` is not capable
of executing external commands, and `$RBMK_EXE` is set to `rbmk`, thus
supporting previously existing scripts without modification.
132 changes: 132 additions & 0 deletions pkg/cli/sh/builtin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// SPDX-License-Identifier: GPL-3.0-or-later

package sh

import (
"context"
"errors"
"fmt"
"io"

"github.com/rbmk-project/common/cliutils"
"github.com/rbmk-project/common/fsx"
"github.com/rbmk-project/rbmk/internal/markdown"
"github.com/rbmk-project/rbmk/internal/rootcmd"
"mvdan.cc/sh/v3/interp"
)

// builtInMiddleware is the middleware to execute built-in commands.
type builtInMiddleware func(next interp.ExecHandlerFunc) interp.ExecHandlerFunc

// newBuiltInMiddleware creates a new built-in middleware for
// executing built-in commands with the shell.
func newBuiltInMiddleware() builtInMiddleware {
return func(next interp.ExecHandlerFunc) interp.ExecHandlerFunc {
return func(ctx context.Context, args []string) error {
// 1. ensure we have a command to run and that such a
// command is indeed the "rbmk" internal command.
if len(args) < 1 {
return errors.New("no command to run")
}
if args[0] != "rbmk" {
return fmt.Errorf("%s: command not found", args[0])
}

// 2. construct the subcommand environment.
env := newBuiltInEnvironment(interp.HandlerCtx(ctx))

// 3. construct the root command to switch depending on the
// actual `rbmk` subcommand being invoked.
directory := rootcmd.CommandsWithoutSh()
directory["sh"] = builtInShCommand{}
root := cliutils.NewCommandWithSubCommands(
"rbmk", markdown.LazyMaybeRender(rootcmd.HelpText()), directory)

// 4. execute the root command and return the result
return root.Main(ctx, env, args...)
}
}
}

// builtInEnvironment contains the environment for executing built-in commands.
type builtInEnvironment struct {
// fs is the file system to use.
fs fsx.FS

// stdin is the standard input.
stdin io.Reader

// stdout is the standard output.
stdout io.Writer

// stderr is the standard error.
stderr io.Writer
}

// newBuiltInEnvironment creates a new built-in environment.
//
// Uses:
//
// 1. [fsx.NewChdirFS] to simulate chdir into the current directory.
//
// 2. the shells's current stdin, stdout, and stderr.
//
// We ignore the shell environment since we don't actually use it.
func newBuiltInEnvironment(shCtx interp.HandlerContext) *builtInEnvironment {
return &builtInEnvironment{
fs: fsx.NewChdirFS(fsx.OsFS{}, shCtx.Dir),
stdin: shCtx.Stdin,
stdout: shCtx.Stdout,
stderr: shCtx.Stderr,
}
}

// Ensure that builtInEnvironment implements [cliutils.Environment].
var _ cliutils.Environment = &builtInEnvironment{}

// FS implements [cliutils.Environment].
func (env *builtInEnvironment) FS() fsx.FS {
return env.fs
}

// Stderr implements [cliutils.Environment].
func (env *builtInEnvironment) Stderr() io.Writer {
return env.stderr
}

// Stdin implements [cliutils.Environment].
func (env *builtInEnvironment) Stdin() io.Reader {
return env.stdin
}

// Stdout implements [cliutils.Environment].
func (env *builtInEnvironment) Stdout() io.Writer {
return env.stdout
}

// builtInShCommand is the built-in `sh` command executed
// from inside the `rbmk sh` environment. We do not permit
// executing the shell inside the shell because that has
// not been tested, and it would probably not WAI.
type builtInShCommand struct{}

// Ensure that [builtInShCommand] implements [cliutils.Command].
var _ cliutils.Command = builtInShCommand{}

// Help implements [cliutils.Command].
func (cmd builtInShCommand) Help(env cliutils.Environment, argv ...string) error {
return NewCommand().Help(env, argv...)
}

// Main implements [cliutils.Command].
func (cmd builtInShCommand) Main(ctx context.Context, env cliutils.Environment, argv ...string) error {
// 1. Honour requests for printing the help.
if cliutils.HelpRequested(argv...) {
return cmd.Help(env, argv...)
}

// 2. otherwise prevent re-execution of the shell
err := errors.New("cannot execute `rbmk sh` inside `rbmk sh`")
fmt.Fprintf(env.Stderr(), "rbmk sh: %s\n", err.Error())
return err
}
Loading

0 comments on commit 93815ff

Please sign in to comment.