Enable temporary_key_pair_type option for ed25519 #179

wedge-jarrad · 2022-01-20T07:09:57Z

Allow Packer to create ed25519 keys as the temporary SSH keypair by setting the temporary_key_pair_type option. Defaults to rsa.

This is my first foray into the Packer code so I have no idea what I'm doing :D. Feel free to boss me around a bit if there are better ways to go about this, particularly the way the input is validated/defaulted.

I only spent a little bit of time looking at the tests but it wasn't immediately obvious to me how to incorporate this in the tests. Might have some more time to look a bit harder tomorrow (pointers appreciated!). I did, however, test manually and it seems to work. If you specify temporary_key_pair_type = "ed25519" you get an ed25519 keypair. Any other value or omission of temporary_key_pair_type results in an rsa keypair.

Closes #144
Relates hashicorp/packer#10074

hashicorp-cla · 2022-01-20T07:10:00Z

All committers have signed the CLA.

JenGoldstrich

Looks great so far, as you pointed out there isn't a great place to test this currently,

@nywilken and I are going to make some changes to the key_pair step code to make it easier to test, the refactor to allow us to mock the ec2Connection seems a bit complicated for a first time contributor, we will update you here once we have merged those changes so you can write a test.

I tested your changes locally and they worked on my end for setting the key

We will also need to update the docs here https://github.com/hashicorp/packer/blob/master/website/content/partials/packer-plugin-sdk/communicator/SSHTemporaryKeyPair-not-required.mdx but we can handle that ourselves once we have your PR merged.

Thank you so much for this contribution, we'll get back to you soon!

JenGoldstrich · 2022-01-20T16:44:53Z

builder/common/run_config.go

@@ -601,6 +601,9 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 		c.Comm.SSHPrivateKeyFile == "" && c.Comm.SSHPassword == "" {

 		c.Comm.SSHTemporaryKeyPairName = fmt.Sprintf("packer_%s", uuid.TimeOrderedUUID())
+		if c.Comm.SSHTemporaryKeyPairType != "ed25519" {


I think this defaulting makes sense sense the AWS docs here https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateKeyPair.html specify that that ed25519 is not supported on several instances

JenGoldstrich · 2022-01-20T18:13:04Z

builder/common/step_key_pair.go

@@ -64,6 +64,7 @@ func (s *StepKeyPair) Run(ctx context.Context, state multistep.StateBag) multist
 	ui.Say(fmt.Sprintf("Creating temporary keypair: %s", s.Comm.SSHTemporaryKeyPairName))
 	keypair := &ec2.CreateKeyPairInput{
 		KeyName: &s.Comm.SSHTemporaryKeyPairName,
+		KeyType: &s.Comm.SSHTemporaryKeyPairType,


Yep that was the issue!

wedge-jarrad · 2022-01-21T06:07:26Z

Wasn't really sure what to do with the docs. The option is already documented even though it hadn't been implemented and has fewer actual options than shown. The docs are generic so there is stuff there that doesn't apply to AWS. temporary_key_pair_bits, for example, isn't something that can be implemented for AWS since they don't give you that option when creating key pairs. Can't just remove those things, though, because they might be relevant and implemented for other plugins.

Thanks for working on the unit tests. Will keep an eye out for that when it's ready :)

Introduce a unit test and acceptance test for the key pair step, validating that an ssh key name is respected, and that its type is RSA, adding these tests to help with #179 Co-authored-by: Jenna Goldstrich <[email protected]> Co-authored-by: Wilken Rivera <[email protected]>

JenGoldstrich · 2022-01-22T00:08:17Z

Hey @wedge-jarrad,

I just merged #181 which @nywilken and I worked on to add a unit test and an acceptance test for the keypair step. So you'll need to rebase your PR on top of that.

For unit tests you should be able to un-comment the extra lines in this test

packer-plugin-amazon/builder/common/step_key_pair_test.go

Line 55 in 06fe648

func TestStepKeyPair_withDefault(t *testing.T) {

to test that the step key pair actually passes in the type as we'll expect it to, I would also add a unit test to this file as well https://github.com/hashicorp/packer-plugin-amazon/blob/main/builder/common/run_config_test.go to cover the default logic to rsa.

For acceptance tests if you'd like you can add a duplicate of this test

packer-plugin-amazon/builder/ebs/builder_acc_test.go

Line 495 in 06fe648

func TestAccBuilder_EbsKeyPair_rsa(t *testing.T) {

to check if that if you pass in ed25519, it correctly sets that as the ssh-key type. To run all acceptance tests you can run make testacc, or to run a single test, like the RSA one, you could run PACKER_ACC=1 go test -count=1 -v ./... -run='TestAccBuilder_EbsKeyPair' --timeout=120m

One other small change I'd recommend is to edit the run_config to include a check that the SSH Key type is rsa, or ed25519, and throw an error if its not one of those types, and then add a corresponding unit test. Here is an example of a validator in the run config that checks the value of a field

packer-plugin-amazon/builder/common/run_config.go

Line 631 in 06fe648

    
           if c.Metadata.HttpEndpoint != "enabled" && c.Metadata.HttpEndpoint != "disabled" {

We can handle the docs change if you'd like.

I know this is a bit long so, feel free to let us know if there's any of that you'd like help on/are confused on!

Closes hashicorp#144

Default to "rsa" if not given. Error if a value other than "rsa" or "ed25519" is given.

wedge-jarrad · 2022-01-23T09:49:53Z

Ok, I think that's everything. Let me know if I've missed anything. And thank you for taking the time to guide me through this :)

Acceptance tests:

$ PACKER_ACC=1 go test -count=1 -v ./builder/ebs/... -run='TestAccBuilder_EbsKeyPair' --timeout=120m
=== RUN   TestAccBuilder_EbsKeyPair_rsa
--- PASS: TestAccBuilder_EbsKeyPair_rsa (108.29s)
=== RUN   TestAccBuilder_EbsKeyPair_ed25519
--- PASS: TestAccBuilder_EbsKeyPair_ed25519 (103.57s)
PASS
ok  	github.com/hashicorp/packer-plugin-amazon/builder/ebs	211.876s
?   	github.com/hashicorp/packer-plugin-amazon/builder/ebs/acceptance	[no test files]

Unit tests:

$ gmake test
?   	github.com/hashicorp/packer-plugin-amazon	[no test files]
ok  	github.com/hashicorp/packer-plugin-amazon/builder/chroot	0.224s
ok  	github.com/hashicorp/packer-plugin-amazon/builder/common	0.099s
?   	github.com/hashicorp/packer-plugin-amazon/builder/common/awserrors	[no test files]
?   	github.com/hashicorp/packer-plugin-amazon/builder/common/ssm	[no test files]
ok  	github.com/hashicorp/packer-plugin-amazon/builder/ebs	0.263s
?   	github.com/hashicorp/packer-plugin-amazon/builder/ebs/acceptance	[no test files]
ok  	github.com/hashicorp/packer-plugin-amazon/builder/ebssurrogate	0.086s
ok  	github.com/hashicorp/packer-plugin-amazon/builder/ebsvolume	0.091s
ok  	github.com/hashicorp/packer-plugin-amazon/builder/instance	0.143s
ok  	github.com/hashicorp/packer-plugin-amazon/datasource/ami	0.054s
ok  	github.com/hashicorp/packer-plugin-amazon/datasource/parameterstore	0.047s
ok  	github.com/hashicorp/packer-plugin-amazon/datasource/secretsmanager	0.047s
?   	github.com/hashicorp/packer-plugin-amazon/post-processor/import	[no test files]
?   	github.com/hashicorp/packer-plugin-amazon/version	[no test files]

JenGoldstrich · 2022-01-24T15:59:26Z

Looks fantastic to me, excellent contribution @wedge-jarrad, merging!

azr · 2022-01-24T16:34:08Z

Super nice !

jsf9k · 2022-02-18T20:59:00Z

Thanks for taking care of this @wedge-jarrad!

JRemitz · 2022-03-02T22:56:52Z

Not sure if I'm doing something wrong, but I now see that my local private key is ed25519 however packer fails to connect. If I connect with instance-connect in a debug session, I see the public key is still listed as ssh-rsa instead of ssh-ed25519.

ssh-keygen -l
...
256 SHA256:....  (ED25519)

Default ssh user's authorized_keys

ssh-rsa AAAAB3NzaC1yc2... packer_62....

Feels like there is a mismatch here?

JRemitz · 2022-03-03T04:45:22Z

Okay, to the point of this PR - it works great, thank you! I was finally able to verify.

Building on my comment above, I'm not sure where to go but I'm building an AMI on top of another packer-built AMI. It turns out that the public ssh key from the first Packer run is still in the authorized_keys and for whatever reason, the second Packer key isn't being added. So my first key that was an ssh-rsa from a community image since Packer works with ed25519 but Ansible provisioner fails. My second Packer build on top of that fails with Packer unable to ssh to the temporary ec2 instance because the only Packer key that is being allowed/put into the authorized_keys on the machine was the public ssh key from the first build, despite the new keypair from AWS being the temporary key. I was finally able to get this working by adding ssh_clear_authorized_keys=true so the key was cleared out after the first build. This only fails for Amazon Linux 2 machines, Ubuntu are fine. 😕

sbraz · 2022-04-04T12:51:23Z

Hi, would it please be possible to fix this for the OpenStack builder as well? See hashicorp/packer-plugin-openstack#54.

wedge-jarrad requested a review from a team as a code owner January 20, 2022 07:09

JenGoldstrich self-assigned this Jan 20, 2022

JenGoldstrich reviewed Jan 20, 2022

View reviewed changes

JenGoldstrich mentioned this pull request Jan 21, 2022

Add Unit test for KeyPair step #181

Merged

wedge-jarrad added 3 commits January 22, 2022 14:09

Enable temporary_key_pair_type option for ed25519

87437b8

Closes hashicorp#144

Improve temporary_key_pair_type validation

c090a4a

Default to "rsa" if not given. Error if a value other than "rsa" or "ed25519" is given.

Add tests for temporary_key_pair_type

52a16e8

wedge-jarrad force-pushed the packer-144 branch from 26a2f3b to 52a16e8 Compare January 23, 2022 09:35

go fmt

77dce5a

JenGoldstrich merged commit 47325f9 into hashicorp:main Jan 24, 2022

programmablemike mentioned this pull request Jan 25, 2022

Packer amazon-ebs builder cannot ssh with Amazon Linux 2022 #172

Closed

azr added the enhancement label Jan 26, 2022

jvperrin mentioned this pull request Mar 17, 2022

deprecation/removal of ssh-rsa from PubKeyAcceptedTypes hashicorp/packer#10074

Closed

bdwyertech mentioned this pull request May 4, 2022

Add support for specifying SSH key type to be generated test-kitchen/kitchen-ec2#583

Merged

3 tasks

hbeberman mentioned this pull request Jun 15, 2022

feat: enable 2.0 Mariner images Azure/AgentBaker#1871

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable temporary_key_pair_type option for ed25519 #179

Enable temporary_key_pair_type option for ed25519 #179

wedge-jarrad commented Jan 20, 2022

hashicorp-cla commented Jan 20, 2022 •

edited

Loading

JenGoldstrich left a comment •

edited

Loading

JenGoldstrich Jan 20, 2022

JenGoldstrich Jan 20, 2022

wedge-jarrad commented Jan 21, 2022

JenGoldstrich commented Jan 22, 2022

wedge-jarrad commented Jan 23, 2022 •

edited

Loading

JenGoldstrich commented Jan 24, 2022

azr commented Jan 24, 2022

jsf9k commented Feb 18, 2022

JRemitz commented Mar 2, 2022

JRemitz commented Mar 3, 2022 •

edited

Loading

sbraz commented Apr 4, 2022

Enable temporary_key_pair_type option for ed25519 #179

Enable temporary_key_pair_type option for ed25519 #179

Conversation

wedge-jarrad commented Jan 20, 2022

hashicorp-cla commented Jan 20, 2022 • edited Loading

JenGoldstrich left a comment • edited Loading

Choose a reason for hiding this comment

JenGoldstrich Jan 20, 2022

Choose a reason for hiding this comment

JenGoldstrich Jan 20, 2022

Choose a reason for hiding this comment

wedge-jarrad commented Jan 21, 2022

JenGoldstrich commented Jan 22, 2022

wedge-jarrad commented Jan 23, 2022 • edited Loading

JenGoldstrich commented Jan 24, 2022

azr commented Jan 24, 2022

jsf9k commented Feb 18, 2022

JRemitz commented Mar 2, 2022

JRemitz commented Mar 3, 2022 • edited Loading

sbraz commented Apr 4, 2022

hashicorp-cla commented Jan 20, 2022 •

edited

Loading

JenGoldstrich left a comment •

edited

Loading

wedge-jarrad commented Jan 23, 2022 •

edited

Loading

JRemitz commented Mar 3, 2022 •

edited

Loading