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

Permit re-using AWS keypair #1837

Merged
merged 4 commits into from
Jun 15, 2015
Merged

Permit re-using AWS keypair #1837

merged 4 commits into from
Jun 15, 2015

Conversation

nyetsche
Copy link

For various reasons not everyone wants to create and delete a keypair each time an AWS provisioner runs. I see an issue here: #1635 and a slightly different one here: #1697.

This PR adds a ssh_keypair_name option that is used by AWS "instance" and "ebs" builders, the previous ssh_private_key_file option remains as-is.

If ssh_keypair_name is unset (internally seen as to "") AND ssh_private_key_file is unset; the AWS builders default to creating and deleting a temporary keypair.

This is my first attempt Go code, so apologies for any amateur mistakes. It took me a while to realize that I needed to clone the main repo and add my fork as a remote, so that the import ( "github.com/mitchellh/packer/[...]" ) etc. statements worked.

Lesko, Matthew (NIH/NLM/NCBI) [C] added 3 commits January 13, 2015 13:12
Adds a 'ssh_keypair_name' option to the configuration
for AWS, along with some munging to create the
temporarily keypair if one isn't specific.

NOT YET WORKING.

From a 'make' I get the following errors:

builder/amazon/ebs/builder.go:94: b.config.SSHKeyPairName undefined
(type config has no field or method SSHKeyPairName)

builder/amazon/instance/builder.go:199: b.config.SSHKeyPairName
undefined (type Config has no field or method SSHKeyPairName)
These changes permit the use of pre-created SSH keypairs with AWS. If
so, the configuration for the builder needs to include an
ssh_keypair_name option and a ssh_private_key_file.

If ssh_private_key_file is *not* defined, it'll go through the
rigamarole of creating a temporary keypair. The ssh_keypair_name option
by itself won't make that change, because it doesn't make sense to
specify a keypair but not tell packer where the private key is, but it
does happen that you could have a private key and the public-key is
"baked in", and not part of your EC2 account.
@renat-sabitov
Copy link

The problem I see with pre-created keypair is that the fingerprint stays in authorized_keys after baking, therefore instancess will be reachable with the baking key.

Other than that, I can see indentation problems in code.

@nyetsche
Copy link
Author

I wondered about the formatting, turns out that go fmt doesn't recursively do the whole tree. I just pushed a new commit with the formatting changes.

@nyetsche
Copy link
Author

As for your first comment - the ssh_keypair_name is optional and need not be used in all environments.

For me specifically, it is useful, since I'm changing the authorized_keys file by other means. I want our "packerbot" script to use a specific Keypair so I can restrict it via IAM policies; without a named keypair, I have to give packer the IAM policies of creating and deleting arbitrary keypairs.

@renat-sabitov
Copy link

@nyetsche The side-effect of leaving baking keys in authorized_key isn't that clear for everyone and I guess default behaviour should be to delete it after baking.

@nyetsche
Copy link
Author

I concur, the default behavior remains the same. The line https://github.com/nyetsche/packer/blob/9d097f9d4ef331adac5f322d14ca185e879331a0/builder/amazon/common/step_key_pair.go#L24 remains the same as in mitchellh/master ; if PrivateKeyFile is not set (equal to "") then a new keypair is created and later removed by Packer. https://github.com/nyetsche/packer/blob/9d097f9d4ef331adac5f322d14ca185e879331a0/builder/amazon/common/step_key_pair.go#L45

@renat-sabitov
Copy link

@nyetsche Yes, it is the same and authorized_keys will have the baking key in it. Normally, packer doesn't save the secret key during baking, making authorized_keys entry useless.

For permanent keys you posses to the PrivateKeyFile and therefore you will have access to all instances started from the AMI, which is not a desirable behaviour.

@myoung34
Copy link

I think it's against the main point that making assumptions about how infrastructure should be left out. Not all infrastructures are the same, so tools like packer should make no assumptions.

@mitchellh
Copy link
Contributor

LGTM!

@mitchellh mitchellh merged commit 43f08b2 into hashicorp:master Jun 15, 2015
@ghost ghost locked and limited conversation to collaborators Apr 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants