All committers have signed the CLA.

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

_{Have you signed the CLA already but the status is still pending? Recheck it.}

I think after reading a TON more code I'm starting to understand something; but I have never written Go before, and I've only just touched Packer in the past week. So please be generous with the constructive criticism! (Cross-posting on issue).

BE WARNED At this point I have not tested this code; however, it lines up with other code I have seen and should work. I just wanted to get my idea down to start getting feedback while I figure out how to set up my development environment proper and perform testing.

Hey @bl1nk1n,

Thanks for the PR!

A few things come to mind:

1. Generating the SSH private key looks OK to me in the current state. I'm not certain type-checking for the communicator is the best way to do so, I see there's a step already defined in the clone builder, I wonder if we cannot reuse it? builder/proxmox/clone/step_ssh_key_pair.go is the one doing that.
2. Looking at the clone builder, it seems that the Run function for it is a good example of how to use that step, I would think we can replicate this on the iso builder as well, so long as the StepSshKeyPair is moved to common.
3. Just to be clear, the generation of the private key is one step, but there needs to be an extra step at some point to have the public key transmitted to the instance so the SSH connections with that key are accepted, I believe clone does that with proxmoxapi.CloudInit, unless I'm mistaken, you will probably need a similar mechanism for the iso builder.

Hope this feedback helps, let me know if something isn't clear!

[...] I'm not certain type-checking for the communicator is the best way to do so [...]

I just did this as the SSH keys should probably not be generated for other types of communicators such as winrm. I based/copied it from the Packer communicator code that checks if it is ssh or winrm and calls the respective step.

[...] I see there's a step already defined in the clone builder, I wonder if we cannot reuse it? [...] Looking at the clone builder, it seems that the Run function for it is a good example of how to use that step, I would think we can replicate this on the iso builder as well, so long as the StepSshKeyPair is moved to common.

Ah! I did not see that, I'll have to go take a look! I haven't gotten to the step in my homelab IaC development to use proxmox-clone yet (literally my next step after building base templates with proxmox-iso; otherwise, I may have seen this.

Just to be clear, the generation of the private key is one step, but there needs to be an extra step at some point to have the public key transmitted to the instance so the SSH connections with that key are accepted, I believe clone does that with proxmoxapi.CloudInit, unless I'm mistaken, you will probably need a similar mechanism for the iso builder.

I was wondering about this. It felt super weird to just generate the SSH key and not add it in through CloudInit anywhere. I was using other template preparation plugin code for other hypervisors as examples and didn't see where they added it into CloudInit. So I assumed it was part of the process somehow (as I really don't know too much about Packer).

Thanks for the tips! I'll try and work on this here in the following week or two. While it is Thanksgiving, there is usually a decent amount of downtime with the family.

@lbajolet-hashicorp

I moved StepSshKeyPair to common and added another step to update Cloud-Init configurations that will be passed via HTTP or CD. The code is rough and there are some parts where I am not sure what to do yet, but I wanted to give you an update in case there is feedback/direction you can give. Again, sorry for the ignorance but this is my first time working with Go and Packer plugins.

The SSH key generation really was no problem, just like you stated. However, what I noticed was that proxmox-clone injects the public key via Cloud-Init on VM creation; which won't necessarily work for proxmox-iso. Usually Cloud-Init is not set up prior to template or even VM creation; it is done as a provisioning step. Instead, the credentials would have to be passed in as part of something like Kickstart or Autoinstall via a configuration file. These configuration files can be passed in via CD or the built-in HTTP server. So for proxmox-iso the public key would more than likely need to be put into the configuration file.

Normally when using Packer I could just use templatefile and, given the SSH key as a variable, inject it into the configuration that way. Instead, I needed to go through every way in which SSH keys can be passed in, pull the data down, replace the template variable ${temporary_ssh_public_key} with the public key, and then write it either to memory or back to the file. In the current state I have it, there are a few cases, when dealing with directories, in which this new feature won't work (yet). If the user specifies a public key/password to be used and a temporary key is not generated, it will cause the template variable to not be filled (which is another issue). In addition, it currently only looks for a user-data file/file contents which might only be for Ubuntu autoinstall.

In the current state, the user would basically specify ${temporary_ssh_public_key} in their automated installation configuration and it will get filled in during build time.

I think this feature is possible to fully implement; however, after spending some time noodling with it, this might add unnecessary complexity and UI weirdness. Trying to inject SSH keys into auto install configurations prior to CD creation/HTTP server deployment while supporting many OSes seems like it will get very messy. This feature might need a more experienced touch. If no one else is able to look at it, I am willing to continue learning Go/Packer plugins and messing with it with guidance!

NOTE: I still haven't set up my development environment for this plugin/go/Packer so this code is not tested. This will probably be my next priority now that I've laid a good foundation.