Add encryption_settings to VMs and managed disks #16
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
StephenWeatherford
changed the title
StephenWeatherford
changed the title
StephenWeatherford
changed the title
whiskeyjay
reviewed
May 4, 2017
|
|
||
| // Azure can change enabled from false to true, but not the other way around, so | ||
| // to keep idempotency, we'll conservatively set this to ForceNew=true | ||
| ForceNew: true, |
There was a problem hiding this comment.
This means the existing disk will be deleted and a new one created. Data loss?
| name = "standard" | ||
| } | ||
|
|
||
| tenant_id = "d6e396d0-5584-41dc-9fc0-268df99bc610" |
There was a problem hiding this comment.
How is this going to work for others running the test?
There was a problem hiding this comment.
Whoops, started but never finished.
| version = "latest" | ||
| } | ||
|
|
||
| storage_os_disk { |
There was a problem hiding this comment.
Where is encryption_settings?
|
Can’t find this comment on github:
* This means the existing disk will be deleted and a new one created. Data loss?
Not if using managed disks, or delete_os_disk_on_termination is left at default false.
From: Jay Wang [mailto:[email protected]]
Sent: Thursday, May 4, 2017 2:50 PM
To: harijayms/terraform <[email protected]>
Cc: Stephen Weatherford <[email protected]>; Author <[email protected]>
Subject: Re: [harijayms/terraform] Add encryption_settings to VMs (will add to managed disks afterwards) (#16)
@whiskeyjay commented on this pull request.
________________________________
In builtin/providers/azurerm/encryption_settings.go<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fharijayms%2Fterraform%2Fpull%2F16%23discussion_r114687635&data=02%7C01%7CStephen.Weatherford%40microsoft.com%7C2ee8d42ecfc64fbbcdd908d493377776%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636295313808152457&sdata=8nB6vSRbDoedgiiEIoSsetvvGT%2FZa%2Bw9e1MVJIcoSlQ%3D&reserved=0>:
+ "github.com/hashicorp/terraform/helper/schema"
+)
+
+var encryptionSettingsSchema = schema.Schema{
+ Type: schema.TypeList,
+ Optional: true,
+ MaxItems: 1,
+ Elem: &schema.Resource{
+ Schema: map[string]*schema.Schema{
+ "enabled": {
+ Type: schema.TypeBool,
+ Required: true,
+
+ // Azure can change enabled from false to true, but not the other way around, so
+ // to keep idempotency, we'll conservatively set this to ForceNew=true
+ ForceNew: true,
This means the existing disk will be deleted and a new one created. Data loss?
________________________________
In builtin/providers/azurerm/resource_arm_virtual_machine_test.go<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fharijayms%2Fterraform%2Fpull%2F16%23discussion_r114845474&data=02%7C01%7CStephen.Weatherford%40microsoft.com%7C2ee8d42ecfc64fbbcdd908d493377776%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636295313808152457&sdata=hXZaRnQl3ri145Wr5Cf91jCG8BymB9TNdjuzWutc1XE%3D&reserved=0>:
+
+resource "azurerm_resource_group" "test" {
+ name = "acctestRG-${var.suffix}"
+ location = "West US 2"
+}
+
+resource "azurerm_key_vault" "test" {
+ name = "acctestRG-${var.suffix}"
+ location = "West US 2"
+ resource_group_name = "${azurerm_resource_group.test.name}"
+
+ sku {
+ name = "standard"
+ }
+
+ tenant_id = "d6e396d0-5584-41dc-9fc0-268df99bc610"
How is this going to work for others running the test?
________________________________
In builtin/providers/azurerm/resource_arm_virtual_machine_test.go<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fharijayms%2Fterraform%2Fpull%2F16%23discussion_r114845642&data=02%7C01%7CStephen.Weatherford%40microsoft.com%7C2ee8d42ecfc64fbbcdd908d493377776%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636295313808152457&sdata=34aMIhv9Y87Fj%2Fubj1P2t1DpUmMHoXmE%2BlYXbIg9JvE%3D&reserved=0>:
+
+resource "azurerm_virtual_machine" "test" {
+ name = "acctvm-${var.suffix}"
+ location = "West US 2"
+ resource_group_name = "${azurerm_resource_group.test.name}"
+ network_interface_ids = ["${azurerm_network_interface.test.id}"]
+ vm_size = "Standard_D1_v2"
+
+ storage_image_reference {
+ publisher = "MicrosoftWindowsServer"
+ offer = "WindowsServer"
+ sku = "2012-Datacenter"
+ version = "latest"
+ }
+
+ storage_os_disk {
Where is encryption_settings?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fharijayms%2Fterraform%2Fpull%2F16%23pullrequestreview-36175054&data=02%7C01%7CStephen.Weatherford%40microsoft.com%7C2ee8d42ecfc64fbbcdd908d493377776%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636295313808152457&sdata=hmeuZCXOxwlZTvwU6T%2F1lARYD9ltIT4nODU9RgoRmPw%3D&reserved=0>, or mute the thread<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAGl9Su1EGLiY4Xdl-d8J8V_5D73snmGKks5r2kfxgaJpZM4NO1iy&data=02%7C01%7CStephen.Weatherford%40microsoft.com%7C2ee8d42ecfc64fbbcdd908d493377776%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636295313808152457&sdata=WmzYJtPprRuQIPeFIkawDH%2FtOm0mU206OBQjU5lqT%2Bs%3D&reserved=0>.
|
StephenWeatherford
force-pushed
the
encryption-settings
branch
from
767581f to
da35f5a
Compare
StephenWeatherford
force-pushed
the
encryption-settings
branch
from
da35f5a to
df87732
Compare
StephenWeatherford
changed the title
|
Creating new PR for hashicorp |
|
@StephenWeatherford - Was this merged in as part of 0.10.0? I can't seem to see anything as part of the provider split |
|
I'm no longer working on terraform. @echuvyrov you took over this work, right? Is there a timeline? |
|
hey @tomasquith, the cadence for Terraform Core and providers is different now - AzureRM provider 0.1.5 was just released today while TF Core is at 0.10.0. I am going to finish up autoscale settings and then jump on this one, 2-3 weeks perhaps. |
|
@StephenWeatherford @echuvyrov - Thanks for the updates. We're about to start a major client deployment into Azure via Terraform, SSE for managed disks is a must - I really appreciate the work on this. @StephenWeatherford's original PR has been moved to the provider repo - |
|
Saw the same pull request (hashicorp#157). Wondering where it's held up. |
|
@gudlyf - just a FYI, your linked PR is to an unrelated one, the correct PR is in the provider repo (since the split), with the same id. @echuvyrov stated a rough speculative timeline above :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add encryption_settings to virtual machines. Part 1 of fixing hashicorp#12259.
Working on tests.