nickthecook/ops

Add support for argument-checking in Actions

nickthecook opened this issue · 1 comments

When ops runs an action, it appends all command-line arguments to the command specified in the action config. Thus, it's easy to write actions that take arguments. For example:

actions:
  hello:
    command: echo Hello,
$ ops hello these are arguments.
Running 'echo Hello, these are arguments' from ops.yml in environment 'dev'...
Hello, these are arguments.

Also, if you're just running another executable or a script from the action, that executable or script can check its own arguments.

However, sometimes the action uses a specific subset of another executable's functionality. E.g., a script to run a command inside each of a set of containers shouldn't care what the command is or how many containers you tell it to run the command in, but if you have an ops action like console that needs to run a command in exactly one container, that action cares that exactly one argument is supplied.

It's also not obvious to a user that the command needs exactly one argument unless the developer writes it into the action description. And if the description contains the info, it could become stale and inaccurate if the command is updated and the description isn't (as unlikely as it is that a developer would do such a thing).

To help with this case, ops could take specifications like this:

actions:
  console:
    args:
      container_name:
        description: the name of one of the ops test containers
        mandatory: yes
    extra_args_allowed: no
    command: bin/run.sh bash "$container_name"

ops would assign its first command-line argument to the variable $container_name.

This approach:

  • documents the command for the user
  • enforces the documented usage, to prevent drift
  • might be flexible enough to allow most basic actions to get command argument checking

The default for extra_args_allowed should be true (thanks to YAML, a value of yes evaulates to true), and it should work on commands that do not define args.

Alternative, shorter format

An alternative format could also be supported, to make it less onerous to add argument-checking, and leave ops.yml a little smaller:

actions:
  console:
    args:
      - container_name
    extra_args_allowed: no
    command: bin/run.sh bash "$container_name"

Solution for argument interpolation

This approach may also partially solve #10. It would still be nice to have unnamed arguments interpolated into the command, but anyone wanting that could get something like it in most cases by using this functionality. Naming an argument is a bit more work to add to an action that just putting $1 in a command string, but it would work.

Implementation

There are a couple of ways to implement this, at a high level:

  1. update the string before executing the command

This method would have ops gsub all occurrences of $container_name in the command string before executing the command.

Pros: ops could warn if an argument was specified but not used in the command string
Cons: would evaluate all variable references regardless of quoting, which may lead to unexpected results

  1. set environment variables for each arg

This method would have ops set the environment variable "$container_name` to the value of its first argument before executing the command, and execute the command unmodified.

Pros: this would respect shell quoting; e.g. variable references inside single quotes would not be evaluated by the shell
Cons: ops could still warn about unused args, but it wouldn't be accurate if an arg reference occurred inside single-quotes


I feel that the second option is better via the Principle of Least Surprise, since someone writing a shell command wouldn't expect a variable reference inside single-quotes to be evaluated. The ability to warn about unused argument references is of questionable value, since perhaps the arg could be an environment variable that is not used in the command string, but is used by the script or command that it calls.

Maybe required instead of mandatory.