add additional checks for instance ssh key conditional

[kral2]

From discussion Originally posted by @calorbeer in https://github.com/oracle-terraform-modules/terraform-oci-compute-instance/issues/67#issuecomment-916263890

There is an issue with ssh_public_key variables having default value set to null.

If var.ssh_public_key value is set by the user, ssh_authorized_keys argument is set correctly however if it's null the condition is also evaluated to true and as a result it is set to null. The file statements are never reached. To avoid this the variables either need to default to "" or the conditions have to test for != null and "".

To do:

• [x] test should be done uppon null and we also initialize relavant variables to null
• [x] rename ssh_public_key_path to ssh_public_keys_file (plural form for keys and mention file explicitely)

[kral2]

@calorbeer Would you be volunteer to fix that? :-)

For the implementation details, I suggest that the test should be done uppon null and we also initialize relavant variables to null. "" is an empty string: that would make the connection impossible, but it becomes a deliberate choice from the module user.

Let's keep the conditional as simple as possible.

[calorbeer]

Yes to check for null should be sufficient.

[kral2]

Based on a review comment that appeared in PR #71, we should also adapt the variable names to be semantically correct: using plural form.

rename ssh_public_key_path to ssh_public_keys_path

For ssh_public_key, we need to figure out if there is possibility to pass multiple ssh keys as string or array of strings.

[kral2]

Sometimes, less is more :-)

All the use cases can be handled by only one variable: ssh_public_keys (Changing to plural form regarding fc662062bb7890e6782096005ee109e9696d04d0).

This greatly simplify the code in the module, and will be probably less confusing for the user.

resource "oci_core_compute" "my_instance" {
...
  metadata = {
    ssh_authorized_keys = var.ssh_public_keys != null ? var.ssh_public_keys : file(var.ssh_authorized_keys)
    user_data           = var.user_data
  }
...
}

The conditional is here only for backward compatibility with var.ssh_authorized_keys. As soon as we move to the next major release, we can drop the conditional all together and adopt this simpler form:

resource "oci_core_compute" "my_instance" {
...
metadata = {
    ssh_authorized_keys = var.ssh_public_keys
    user_data           = var.user_data
  }
...
}

The module user will assign value to this argument like this:

module "my_instance" {
...
  ssh_public_keys = var.my_public_ssh_key
...
}

To provide multiple keys at once, just use Heredoc strings:

module "my_instance" {
...
  ssh_public_keys = <<EOF
<public ssh key 1>
<public ssh key 2>
<public ssh key n>
EOF
...
}

If the module user prefer to provide keys from a file, that's also possible:

module "my_instance" {
...
  ssh_public_keys = file("/path/to/file")
...
}