Skip to content

Automatically create ssh keys in gh cs ssh#5752

Merged
cmbrose merged 20 commits intocli:trunkfrom
jungaretti:jungaretti/no-ssh-keys
Jun 14, 2022
Merged

Automatically create ssh keys in gh cs ssh#5752
cmbrose merged 20 commits intocli:trunkfrom
jungaretti:jungaretti/no-ssh-keys

Conversation

@cmbrose
Copy link
Member

@cmbrose cmbrose commented Jun 3, 2022

Fixes: https://github.com/github/codespaces/issues/8154

Adds automatic ssh key generation to gh cs ssh and gh cs cp commands, effectively removing the need for the user to already have set up ssh keys in GitHub.

The key is named codespaces and is stored under ~/.ssh. If it already exists it is just reused, otherwise it is generated without prompt and without passphrase. When starting the ssh server on the codespace (over the RPC call), an additional argument is sent, which is the public key. The codespace agent will append this public key to the authorized_keys file.

This functionality requires a change in the codespaces agent to support, so it should NOT be merged until that change is released.

ssh

@cmbrose cmbrose marked this pull request as ready for review June 4, 2022 00:37
@cmbrose cmbrose requested review from a team as code owners June 4, 2022 00:37
@cmbrose cmbrose requested review from mislav and removed request for a team June 4, 2022 00:37
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Jun 4, 2022
@cmbrose
Copy link
Member Author

cmbrose commented Jun 9, 2022

@cli/codespaces, @mislav this is still pending a codespaces service prod release, but is up and running in PPE (see the gif in the description 🎉 ). If we can get 👀 on this before the release that would help keep things rolling - thanks!

Copy link
Contributor

@markphelps markphelps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. I would recommend adding a Do Not Merge label or simliar to it so that it doesnt get released by mistake before the backend changes are ready

Copy link
Contributor

@josebalius josebalius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good! I've been away on vacation so apologies for the late review.

Left some initial comments.

Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is shaping up well, but I would generally simplify the APIs.

@cmbrose cmbrose requested review from josebalius and mislav June 13, 2022 20:33
Copy link
Contributor

@josebalius josebalius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is almost there, left some comments mostly about errors. I couldn't think of a better name for ssh.Context so good with me as is 👍

Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Only style nitpicks remain

Copy link
Contributor

@josebalius josebalius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@cmbrose cmbrose removed the blocked label Jun 14, 2022
@cmbrose
Copy link
Member Author

cmbrose commented Jun 14, 2022

The service release needed for this just went out 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external pull request originating outside of the CLI core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants