oxidecomputer / omicron

Omicron: Oxide control plane
Mozilla Public License 2.0
244 stars 36 forks source link

Let user specify SSH keys to inject at instance create time #3056

Closed benjaminleonard closed 7 months ago

benjaminleonard commented 1 year ago

The current implementation automatically retrieves and inserts a user's SSH keys into the instances they create. However, it would be beneficial for users to have control over which keys are added, as well as the ability to pass an SSH key not associated with their account.

I propose adding an option to the instance_create API that accepts an array of attached SSH keys identified by name or ID, or a custom SSH key provided as a string (which will not be linked to the user's account).

For an enhanced user experience, we could also include an option that automatically incorporates a user's SSH keys. This would be particularly useful for the CLI, where retrieving SSH keys and adding them as an option is more cumbersome.

It might be reasonable to defer implementation until after FCS. In the meantime, I could work on adding a console message to inform users that all their SSH keys will be injected into the instance (where cloud-init is supported).

Proposed console designs:

image
david-crespo commented 9 months ago

Relevant code:

https://github.com/oxidecomputer/omicron/blob/343835c1b2099e50205fb41be1341c2f58dd43ec/nexus/types/src/external_api/params.rs#L873

https://github.com/oxidecomputer/omicron/blob/343835c1b2099e50205fb41be1341c2f58dd43ec/nexus/src/app/instance.rs#L986-L1001

benjaminleonard commented 8 months ago

We’re looking at the changes required to allow a user to specify the SSH keys to be injected into the instance via cloud-init.

Presently all keys for the current user are fetched from the DB and added to cloud init at instance create and migrate.

Do we:

a) Need to store the keys (or name/IDs) to be retrieved later so that we similarly use them during the subsequent events? or b) Can we dispose of the keys after the instance has been created?

Similarly user_data is persisted in the database – is this needed after an instance has been created?

gjcolombo commented 8 months ago

I think Nexus should expose the same user/instance metadata (through the cloud-init drive) every time it starts an instance, unless a user takes some action (in our API) to change the instance's metadata. There are a couple reasons for this:

  1. We don't really have a way to know for sure that a guest has already "seen" and dealt with the metadata we've shown it. Just having run the instance before isn't sufficient: if an instance reports as Running, but the user turns off the instance (or the guest kernel panics, or the host sled crashes, or...) before the guest cloud-init daemon actually reads and acts on the instance metadata, then the SSH keys won't have been copied, the hostname won't have been set, etc.
  2. Users can ask the cloud-init daemon to rerun its initialization steps and can reasonably expect them to produce the same effects they produced when cloud-init first ran.

So I think we need to do (a) here. How exactly to do that (e.g. do we copy the keys into a column on the instance? do we have the instance refer to the keys by ID and take care not to hard-delete them from the database until all references to them are gone?) is an implementation detail, albeit an important one.