-
Notifications
You must be signed in to change notification settings - Fork 79
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
Enable SecurityProfile (TrustedLaunch) for Virtual Machines #257
Enable SecurityProfile (TrustedLaunch) for Virtual Machines #257
Conversation
Hey @amccullough84 thanks for working on that, This PR I think is actually blocked because of #220 (comment), where we mention that secure launch VMs can't be saved as managed images, only deployed to SIGs, so I believe we need to finish merging #242, another open PR I am currently working on getting merged before we can get this working fully, I could be misunderstanding this feature though. I will come back to this PR once that PR is merged. |
@amccullough84 can you rebase your PR on main please so that I can test that these flags work when publishing directly to SIG without a managed image? |
=== RUN TestTrustedLaunch01 --- PASS: TestTrustedLaunch01 (0.00s) PASS ok github.com/hashicorp/packer-plugin-azure/builder/azure/arm 1.721s
c0e8868
to
b24c4c4
Compare
@JenGoldstrich rebased on main now 👍 |
Updated approved JSON following rebase
I've tried to build an image with this PR's changes am running into some failures. Specifically
With the following build file:
I don't think we support setting a security type, but I've tried with multiple different what I thought were supported OS's have gotten this error, how are you getting around it for your builds? Is there a way to tell which images would be supported, or is there something I need to change on the Azure side? Is my image type just not supported? I used the following list of supported OSs https://learn.microsoft.com/en-us/azure/virtual-machines/trusted-launch but there doesn't seem to be a good way to find a specific supported image SKU that supports trusted-launch |
Does the image definition in the ACG you're distributing the image to have the security type set to trusted launch? You'd set that when creating the image definition and the error suggests there's a mismatch. If its enabled on the source image it must be enabled in the destination image definition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! That makes sense, I fixed that and was able to push to my compute gallery, nice!
When I try to build this while also publishing to a managed image, autorest panics and crashes and I get an Azure error above that about not being able to publish trusted launch images to managed images as expected, but I think it would be good to add a check in the arm config.go in the assertRequiredParametersSet function right after the
if isImageUrl && c.ManagedImageResourceGroupName != ""
error check that is something like
if (c.SecureBootEnabled || c.VTpmEnabled) && isCustomManagedImage) {
errs = packersdk.MultiErrorAppend(errs, fmt.Errorf("A managed image can not set SecureBoot or VTpm, these features are only supported when directly publishing to a Shared Image Gallery"))
}
What do you think @amccullough84?
Hi @JenGoldstrich, sorry for the delay - I've added that check now and seems to catch that conflict now when I run it locally. |
…ommitting directly to your fork, will describe changes in PR comment
builder/azure/arm/config.go
Outdated
@@ -1063,6 +1068,10 @@ func assertRequiredParametersSet(c *Config, errs *packersdk.MultiError) { | |||
errs = packersdk.MultiErrorAppend(errs, fmt.Errorf("A managed image must be created from a managed image, it cannot be created from a VHD.")) | |||
} | |||
|
|||
if (c.SecureBootEnabled || c.VTpmEnabled) && (c.ManagedImageName != "" && c.ManagedImageResourceGroupName != "") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amccullough84 changed this flag, and added tests for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amccullough84 hey no worries friend, I actually messed up and told you the wrong config field, this should block the creation of managed_images, not using custom_managed_image as a base image field, Trusted Launch is only supported when direct publishing to a compute gallery/shared image gallery as mentioned, I've made a commit 7e80049 to your fork directly to make that change, and to add unit tests for that suggested change.
Can you take a look and give me a thumbs up if this still works for your use case and that I haven't broken this for you, and then this should be good to merge
…id when secure boot is set
Hi @JenGoldstrich, looks good to me! |
This PR extends the existing PR #220 but adds the requested change from @JenGoldstrich to add tests for the new parameters.
The previous PR has been idle for several months and as this is a popular request I'm hoping this can be progressed.
As per the previous discussion - TrustedLaunch only applies to Virtual Machines, not images, however an image to be used with for a Trusted Launch enabled VM must be created using a builder VM with trusted launch enabled. It cannot be enabled after initial deployment.
Description of change from existing PR (220):
Sets the SecurityProfile if either Secure Boot or the VTPM option has been set in the packer configuration. This allows the deployment of VMs using Trusted Launch either from marketplace or Shared Image Gallery if the image has been enabled for trusted launch.
Introduced two new parameters:
secure_boot_enabled
vtpm_enabled
Closes #221