Skip to content

instance-ssh() tweaks #324

Open
Open
@rawiriblundell

Description

@rawiriblundell

Hi,
As I'm not 100% clear on the expected coding style and/or contribution guidelines here, I'm going to raise this as an issue rather than a PR and see what gets accepted/influenced/rejected.

First, a tiny critique: there's a number of ShellCheck SC2155's. I see that this has been ad-hoc disabled globally in at least one other lib file, which seems to me to be a sub-optimal choice. FWIW my take on this rule is to simply use the "everybody's happy" option of just declaring your local vars once on a single line and moving on.

With respect specifically to instance-ssh(), this:

    local instance_details="$(instance-ssh-details $instance_id)"
    local instance_id=$(echo $instance_details | awk '{print $1}')
    local keyname=$(echo $instance_details | awk '{print $2}')
    local private_ip=$(echo $instance_details | awk '{print $3}')
    local instance_name=$(echo $instance_details | awk '{print $4}')
    local instance_default_user=$(echo $instance_details | awk '{print $5}')

... could potentially be expressed more efficiently like so:

local instance_id keyname private_ip instance_name instance_default_user
read -r instance_id keyname private_ip instance_name instance_default_user < <(instance-ssh-details "$instance_id")

Or perhaps something like this:

local instance_id keyname private_ip instance_name instance_default_user
set -- $(instance-ssh-details "$instance_id")
instance_id="$1"
keyname="$2"
... etc

The other tweak I'd like to suggest is changing this:

-i "${BMA_SSH_DIR:-~/.ssh/}$keyname"

to

-i "${BMA_SSH_DIR:-~/.ssh}/$keyname"

The rationale is that if BMA_SSH_DIR is missing a trailing slash, this will ensure that one is present. If BMA_SSH_DIR does have a trailing slash, then a non-fatal double-slash will be present (i.e. /path/to/BMA_SSH_DIR//keyname) and the connection will still work. Either way, the default behaviour of ~/.ssh/$keyname remains intact.

With the current code, a BMA_SSH_DIR without a trailing slash results in e.g. /path/to/BMA_SSH_DIRkeyname which will obviously fail.

Cheers!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions