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

[azurerm_batch_account] - expose required properties when pool_allocation_mode is UserSubscription #3535

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a37bc5a
wip: bug fix for batch account in UserSubscription mode
jcorioland May 28, 2019
788c9bd
fix typo and fmt
jcorioland May 29, 2019
2b31138
add docs for user subscription and keyvault ref.
jcorioland May 29, 2019
c75bcb7
update batch account data source
jcorioland May 30, 2019
bfbe33f
update batch account docs
jcorioland May 30, 2019
4018c0c
Merge remote-tracking branch 'upstream/master' into jcorioland/batch-…
jcorioland May 31, 2019
044a832
Merge branch 'master' into jcorioland/batch-user-subscription
katbyte Jun 6, 2019
067c5b9
fix build
katbyte Jun 6, 2019
60b9f5e
Merge branch 'master' into jcorioland/batch-user-subscription
katbyte Jun 6, 2019
51877aa
fix build
katbyte Jun 6, 2019
6d5169a
update unit tests
jcorioland Jun 7, 2019
03b1fb1
wip: bug fix for batch account in UserSubscription mode
jcorioland May 28, 2019
d4ad324
fix typo and fmt
jcorioland May 29, 2019
de93cb1
add docs for user subscription and keyvault ref.
jcorioland May 29, 2019
d8159f2
update batch account data source
jcorioland May 30, 2019
3dc8fa3
update batch account docs
jcorioland May 30, 2019
5cf9964
fix build
katbyte Jun 6, 2019
154e58d
fix build
katbyte Jun 6, 2019
4c24ff0
update unit tests
jcorioland Jun 7, 2019
a1f783a
fixed requested after review + fix tests with azuread provider
jcorioland Jun 19, 2019
c795d3a
move user subscription example from docs to examples
jcorioland Jun 19, 2019
a0ef07c
Merge branch 'jcorioland/batch-user-subscription' of github.com:jcori…
jcorioland Jun 19, 2019
4f5ce9c
remove example from docs
jcorioland Jun 19, 2019
fcc4ca7
fix typo
jcorioland Jun 20, 2019
317f1c3
remove role assignment from test + udpate docs
jcorioland Jul 12, 2019
82789e5
fix docs conflict for batch account
jcorioland Jul 12, 2019
8affd7c
fix docs conflict for batch account
jcorioland Jul 12, 2019
855d24b
fix typo in batch account docs
jcorioland Jul 12, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 35 additions & 10 deletions azurerm/data_source_batch_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Copy link
Contributor

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

if err := d.Set("key_vault_reference", azure.FlattenBatchAccountKeyvaultReference(keyVaultReference)); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 if statement above?

e.g.

if err := d.Set("key_vault_reference", []interface{}); err != nil {
}

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)
Expand Down
57 changes: 57 additions & 0 deletions azurerm/data_source_batch_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
}
45 changes: 45 additions & 0 deletions azurerm/helpers/azure/batch_account.go
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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
return nil
return []interface{}{}

}

if keyVaultReference.ID != nil {
result["id"] = *keyVaultReference.ID
}

if keyVaultReference.URL != nil {
result["url"] = *keyVaultReference.URL
}

return []interface{}{result}
}
36 changes: 36 additions & 0 deletions azurerm/resource_arm_batch_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"log"
"regexp"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate"

"github.com/Azure/azure-sdk-for-go/services/batch/mgmt/2018-12-01/batch"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
Expand Down Expand Up @@ -52,6 +54,25 @@ func resourceArmBatchAccount() *schema.Resource {
string(batch.UserSubscription),
}, false),
},
"key_vault_reference": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"id": {
Type: schema.TypeString,
Required: true,
ValidateFunc: azure.ValidateResourceID,
},
"url": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validate.URLIsHTTPS,
},
},
},
},
"primary_access_key": {
Type: schema.TypeString,
Sensitive: true,
Expand Down Expand Up @@ -105,6 +126,21 @@ func resourceArmBatchAccountCreate(d *schema.ResourceData, meta interface{}) err
Tags: expandTags(tags),
}

// if pool allocation mode is UserSubscription, a key vault reference needs to be set
if poolAllocationMode == string(batch.UserSubscription) {
keyVaultReferenceSet := d.Get("key_vault_reference").([]interface{})
keyVaultReference, err := azure.ExpandBatchAccountKeyVaultReference(keyVaultReferenceSet)
if err != nil {
return fmt.Errorf("Error creating Batch account %q (Resource Group %q): %+v", name, resourceGroup, err)
}

if keyVaultReference == nil {
return fmt.Errorf("Error creating Batch account %q (Resource Group %q): When setting pool allocation mode to UserSubscription, a Key Vault reference needs to be set", name, resourceGroup)
}

parameters.KeyVaultReference = keyVaultReference
}

if storageAccountId != "" {
parameters.AccountCreateProperties.AutoStorage = &batch.AutoStorageBaseProperties{
StorageAccountID: &storageAccountId,
Expand Down
51 changes: 51 additions & 0 deletions azurerm/resource_arm_batch_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,30 @@ func TestAccAzureRMBatchAccount_complete(t *testing.T) {
})
}

func TestAccAzureRMBatchAccount_userSubscription(t *testing.T) {
resourceName := "azurerm_batch_account.test"
ri := tf.AccRandTimeInt()
rs := acctest.RandString(4)
location := testLocation()

config := testAccAzureRMBatchAccount_userSubscription(ri, rs, location)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMBatchAccountDestroy,
Steps: []resource.TestStep{
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMBatchAccountExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "pool_allocation_mode", "UserSubscription"),
),
},
},
})
}

func testCheckAzureRMBatchAccountExists(resourceName string) resource.TestCheckFunc {
return func(s *terraform.State) error {
// Ensure we have enough information in state to look up in API
Expand Down Expand Up @@ -270,3 +294,30 @@ resource "azurerm_batch_account" "test" {
}
`, rInt, location, rString, rString)
}

func testAccAzureRMBatchAccount_userSubscription(rInt int, batchAccountSuffix string, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "testaccRG-%d-batchaccount"
location = "%s"
}

data "azurerm_key_vault" "test" {
name = "azurebatchkv"
resource_group_name = "batch-custom-img-rg"
}

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}"
}
}
`, rInt, location, batchAccountSuffix)
}
14 changes: 13 additions & 1 deletion website/docs/d/batch_account.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ The following attributes are exported:

* `account_endpoint` - The account endpoint used to interact with the Batch service.

* `key_vault_reference` - The `key_vault_reference` block that describes the Azure KeyVault reference to use when deploying the Azure Batch account using the `UserSubscription` pool allocation mode.

* `tags` - A map of tags assigned to the Batch account.

~> **NOTE:** Primary and secondary access keys are only available when `pool_allocation_mode` is set to `BatchService`. See [documentation](https://docs.microsoft.com/en-us/azure/batch/batch-api-basics) for more information.
~> **NOTE:** Primary and secondary access keys are only available when `pool_allocation_mode` is set to `BatchService`. See [documentation](https://docs.microsoft.com/en-us/azure/batch/batch-api-basics) for more information.

---

A `key_vault_reference` block have the following proporties:

* `id` - The Azure identifier of the Azure KeyVault reference.

* `url` - The HTTPS URL of the Azure KeyVault reference.

---
Loading