-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[azurerm_batch_account] - expose required properties when pool_allocation_mode
is UserSubscription
#3535
[azurerm_batch_account] - expose required properties when pool_allocation_mode
is UserSubscription
#3535
Changes from 6 commits
a37bc5a
788c9bd
2b31138
c75bcb7
bfbe33f
4018c0c
044a832
067c5b9
60b9f5e
51877aa
6d5169a
03b1fb1
d4ad324
de93cb1
d8159f2
3dc8fa3
5cf9964
154e58d
4c24ff0
a1f783a
c795d3a
a0ef07c
4f5ce9c
fcc4ca7
317f1c3
82789e5
8affd7c
855d24b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ package azurerm | |
import ( | ||
"fmt" | ||
|
||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" | ||
|
||
"github.com/Azure/azure-sdk-for-go/services/batch/mgmt/2018-12-01/batch" | ||
"github.com/hashicorp/terraform/helper/schema" | ||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" | ||
|
@@ -29,6 +31,22 @@ func dataSourceArmBatchAccount() *schema.Resource { | |
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"key_vault_reference": { | ||
Type: schema.TypeList, | ||
Computed: true, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"id": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"url": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
}, | ||
}, | ||
}, | ||
"primary_access_key": { | ||
Type: schema.TypeString, | ||
Sensitive: true, | ||
|
@@ -78,17 +96,24 @@ func dataSourceArmBatchAccountRead(d *schema.ResourceData, meta interface{}) err | |
d.Set("storage_account_id", autoStorage.StorageAccountID) | ||
} | ||
d.Set("pool_allocation_mode", props.PoolAllocationMode) | ||
} | ||
|
||
if d.Get("pool_allocation_mode").(string) == string(batch.BatchService) { | ||
keys, err := client.GetKeys(ctx, resourceGroup, name) | ||
|
||
if err != nil { | ||
return fmt.Errorf("Cannot read keys for Batch account %q (resource group %q): %v", name, resourceGroup, err) | ||
poolAllocationMode := d.Get("pool_allocation_mode").(string) | ||
|
||
if poolAllocationMode == string(batch.BatchService) { | ||
keys, err := client.GetKeys(ctx, resourceGroup, name) | ||
|
||
if err != nil { | ||
return fmt.Errorf("Cannot read keys for Batch account %q (resource group %q): %v", name, resourceGroup, err) | ||
} | ||
|
||
d.Set("primary_access_key", keys.Primary) | ||
d.Set("secondary_access_key", keys.Secondary) | ||
} else if poolAllocationMode == string(batch.UserSubscription) { | ||
if keyVaultReference := props.KeyVaultReference; keyVaultReference != nil { | ||
if err := d.Set("key_vault_reference", azure.FlattenBatchAccountKeyvaultReference(keyVaultReference)); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since the pool allocation mode field isn't ForceNew: https://github.com/terraform-providers/terraform-provider-azurerm/pull/3535/files#diff-f0f3a4096c35cff4a638ce971ae85210R48 - we'll need to ensure that this is set to an empty value in all cases (since otherwise the data could be stale) - as such could we pass an empty list into this in the other e.g.
|
||
return fmt.Errorf("Error flattening `key_vault_reference`: %+v", err) | ||
} | ||
} | ||
} | ||
|
||
d.Set("primary_access_key", keys.Primary) | ||
d.Set("secondary_access_key", keys.Secondary) | ||
} | ||
|
||
flattenAndSetTags(d, resp.Tags) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,30 @@ func TestAccDataSourceAzureRMBatchAccount_complete(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestAccDataSourceAzureRMBatchAccount_userSubscription(t *testing.T) { | ||
dataSourceName := "data.azurerm_batch_account.test" | ||
ri := tf.AccRandTimeInt() | ||
rs := acctest.RandString(4) | ||
location := testLocation() | ||
config := testAccDataSourceAzureBatchAccount_userSubscription(ri, rs, location) | ||
|
||
resource.ParallelTest(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: config, | ||
Check: resource.ComposeTestCheckFunc( | ||
resource.TestCheckResourceAttr(dataSourceName, "name", fmt.Sprintf("testaccbatch%s", rs)), | ||
resource.TestCheckResourceAttr(dataSourceName, "location", azureRMNormalizeLocation(location)), | ||
resource.TestCheckResourceAttr(dataSourceName, "pool_allocation_mode", "UserSubscription"), | ||
resource.TestCheckResourceAttr(dataSourceName, "key_vault_reference.#", "1"), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func testAccDataSourceAzureRMBatchAccount_basic(rInt int, rString string, location string) string { | ||
return fmt.Sprintf(` | ||
resource "azurerm_resource_group" "test" { | ||
|
@@ -111,3 +135,36 @@ data "azurerm_batch_account" "test" { | |
} | ||
`, rInt, location, rString, rString) | ||
} | ||
|
||
func testAccDataSourceAzureBatchAccount_userSubscription(rInt int, rString string, location string) string { | ||
return fmt.Sprintf(` | ||
resource "azurerm_resource_group" "test" { | ||
name = "testaccRG-%d-batchaccount" | ||
location = "%s" | ||
} | ||
|
||
# assuming that you have an existing keyvault and configured for Microsoft Azure Batch for this test to pass | ||
data "azurerm_key_vault" "test" { | ||
name = "azurebatchkv" | ||
resource_group_name = "batch-custom-img-rg" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rather than doing this we should be able to spin one up dynamically as a part of the test: https://www.terraform.io/docs/providers/azurerm/r/key_vault.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, that's the idea, but as I needed to have the ad provider to do some service principal/role assignment to the keyvault, I was waiting for the AD provider to be available. I will make sure the tests now create all the infra needed. |
||
} | ||
|
||
resource "azurerm_batch_account" "test" { | ||
name = "testaccbatch%s" | ||
resource_group_name = "${azurerm_resource_group.test.name}" | ||
location = "${azurerm_resource_group.test.location}" | ||
|
||
pool_allocation_mode = "UserSubscription" | ||
|
||
key_vault_reference { | ||
id = "${data.azurerm_key_vault.test.id}" | ||
url = "${data.azurerm_key_vault.test.vault_uri}" | ||
} | ||
} | ||
|
||
data "azurerm_batch_account" "test" { | ||
name = "${azurerm_batch_account.test.name}" | ||
resource_group_name = "${azurerm_resource_group.test.name}" | ||
} | ||
`, rInt, location, rString) | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,45 @@ | ||||||
package azure | ||||||
|
||||||
import ( | ||||||
"fmt" | ||||||
|
||||||
"github.com/Azure/azure-sdk-for-go/services/batch/mgmt/2018-12-01/batch" | ||||||
) | ||||||
|
||||||
// ExpandBatchAccountKeyVaultReference expands Batch account KeyVault reference | ||||||
func ExpandBatchAccountKeyVaultReference(list []interface{}) (*batch.KeyVaultReference, error) { | ||||||
if len(list) == 0 { | ||||||
return nil, fmt.Errorf("Error: key vault reference should be defined") | ||||||
} | ||||||
|
||||||
keyVaultRef := list[0].(map[string]interface{}) | ||||||
|
||||||
keyVaultRefID := keyVaultRef["id"].(string) | ||||||
keyVaultRefURL := keyVaultRef["url"].(string) | ||||||
|
||||||
ref := &batch.KeyVaultReference{ | ||||||
ID: &keyVaultRefID, | ||||||
URL: &keyVaultRefURL, | ||||||
} | ||||||
|
||||||
return ref, nil | ||||||
} | ||||||
|
||||||
// FlattenBatchAccountKeyvaultReference flattens a Batch account keyvault reference | ||||||
func FlattenBatchAccountKeyvaultReference(keyVaultReference *batch.KeyVaultReference) interface{} { | ||||||
result := make(map[string]interface{}) | ||||||
|
||||||
if keyVaultReference == nil { | ||||||
return nil | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rather than returning nil, can we return an empty list here, which should set this to an empty value:
Suggested change
|
||||||
} | ||||||
|
||||||
if keyVaultReference.ID != nil { | ||||||
result["id"] = *keyVaultReference.ID | ||||||
} | ||||||
|
||||||
if keyVaultReference.URL != nil { | ||||||
result["url"] = *keyVaultReference.URL | ||||||
} | ||||||
|
||||||
return []interface{}{result} | ||||||
} |
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.
we can remove this check since this is handled within the Flatten function below