Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
benjaminleonard opened this issue May 9, 2023 · 3 comments · Fixed by #4764
Closed

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

benjaminleonard opened this issue May 9, 2023 · 3 comments · Fixed by #4764
Labels
known issue To include in customer documentation and training
Milestone

Comments

@benjaminleonard
Copy link
Contributor

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
Copy link
Contributor

Relevant code:

pub struct InstanceCreate {

let public_keys = self
.db_datastore
.ssh_keys_list(
opctx,
&authz_user,
&PaginatedBy::Name(DataPageParams {
marker: None,
direction: dropshot::PaginationOrder::Ascending,
limit: std::num::NonZeroU32::new(MAX_KEYS_PER_INSTANCE)
.unwrap(),
}),
)
.await?
.into_iter()
.map(|ssh_key| ssh_key.public_key)
.collect::<Vec<String>>();

@david-crespo david-crespo changed the title Non-automatic inclusion of a user's SSH keys Let user specify SSH keys to inject at instance create time Jan 2, 2024
@benjaminleonard
Copy link
Contributor Author

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
known issue To include in customer documentation and training
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants