From 0b36ba7facd44cc281d6deb6029e5903fcddecde Mon Sep 17 00:00:00 2001 From: Liem Phan Date: Thu, 1 Nov 2018 08:34:12 +0000 Subject: [PATCH 1/2] Update role assignment retry * Retry on 400 and PrincipalNotFound error when waiting for service principal to become available. * Add test for service principal creation and subsequent role assignment. --- ...urce_arm_azuread_service_principal_test.go | 47 +++++++++++++++++++ azurerm/resource_arm_role_assignment.go | 12 +++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/azurerm/resource_arm_azuread_service_principal_test.go b/azurerm/resource_arm_azuread_service_principal_test.go index cea7ebf6624b..52feff42f988 100644 --- a/azurerm/resource_arm_azuread_service_principal_test.go +++ b/azurerm/resource_arm_azuread_service_principal_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/google/uuid" + "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -32,6 +33,31 @@ func TestAccAzureRMActiveDirectoryServicePrincipal_basic(t *testing.T) { }) } +func TestAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(t *testing.T) { + resourceName := "azurerm_azuread_service_principal.test" + + ri := acctest.RandInt() + id := uuid.New().String() + config := testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(ri, id) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMActiveDirectoryServicePrincipalDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMActiveDirectoryServicePrincipalExists(resourceName), + testCheckAzureRMRoleAssignmentExists("azurerm_role_assignment.test"), + resource.TestCheckResourceAttrSet(resourceName, "display_name"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), + ), + }, + }, + }) +} + func testCheckAzureRMActiveDirectoryServicePrincipalExists(name string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name] @@ -89,3 +115,24 @@ resource "azurerm_azuread_service_principal" "test" { } `, id) } + +func testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(rInt int, roleAssignmentID string) string { + return fmt.Sprintf(` +data "azurerm_subscription" "current" {} + +resource "azurerm_azuread_application" "test" { + name = "acctestspa-%d" +} + +resource "azurerm_azuread_service_principal" "test" { + application_id = "${azurerm_azuread_application.test.application_id}" +} + +resource "azurerm_role_assignment" "test" { + name = "%s" + scope = "${data.azurerm_subscription.current.id}" + role_definition_name = "Reader" + principal_id = "${azurerm_azuread_service_principal.test.id}" +} +`, rInt, roleAssignmentID) +} diff --git a/azurerm/resource_arm_role_assignment.go b/azurerm/resource_arm_role_assignment.go index aaa77fb127cf..7184e991814f 100644 --- a/azurerm/resource_arm_role_assignment.go +++ b/azurerm/resource_arm_role_assignment.go @@ -186,17 +186,19 @@ func retryRoleAssignmentsClient(scope string, name string, properties authorizat roleAssignmentsClient := meta.(*ArmClient).roleAssignmentsClient ctx := meta.(*ArmClient).StopContext - _, err := roleAssignmentsClient.Create(ctx, scope, name, properties) - + resp, err := roleAssignmentsClient.Create(ctx, scope, name, properties) if err != nil { if utils.ResponseErrorIsRetryable(err) { return resource.RetryableError(err) - } else { - return resource.NonRetryableError(err) + } else if resp.Response.StatusCode == 400 && strings.Contains(err.Error(), "PrincipalNotFound") { + // When waiting for service principal to become available + return resource.RetryableError(err) } + + return resource.NonRetryableError(err) } - return nil + return nil } } From 63fc501ebe80c38282dafd28aca909574bbede2a Mon Sep 17 00:00:00 2001 From: kt Date: Tue, 6 Nov 2018 16:29:27 -0800 Subject: [PATCH 2/2] Moved role def sp assingment test --- ...urce_arm_azuread_service_principal_test.go | 47 ------------------ azurerm/resource_arm_role_assignment_test.go | 49 +++++++++++++++++++ 2 files changed, 49 insertions(+), 47 deletions(-) diff --git a/azurerm/resource_arm_azuread_service_principal_test.go b/azurerm/resource_arm_azuread_service_principal_test.go index 52feff42f988..cea7ebf6624b 100644 --- a/azurerm/resource_arm_azuread_service_principal_test.go +++ b/azurerm/resource_arm_azuread_service_principal_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/google/uuid" - "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -33,31 +32,6 @@ func TestAccAzureRMActiveDirectoryServicePrincipal_basic(t *testing.T) { }) } -func TestAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(t *testing.T) { - resourceName := "azurerm_azuread_service_principal.test" - - ri := acctest.RandInt() - id := uuid.New().String() - config := testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(ri, id) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testCheckAzureRMActiveDirectoryServicePrincipalDestroy, - Steps: []resource.TestStep{ - { - Config: config, - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMActiveDirectoryServicePrincipalExists(resourceName), - testCheckAzureRMRoleAssignmentExists("azurerm_role_assignment.test"), - resource.TestCheckResourceAttrSet(resourceName, "display_name"), - resource.TestCheckResourceAttrSet(resourceName, "application_id"), - ), - }, - }, - }) -} - func testCheckAzureRMActiveDirectoryServicePrincipalExists(name string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[name] @@ -115,24 +89,3 @@ resource "azurerm_azuread_service_principal" "test" { } `, id) } - -func testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(rInt int, roleAssignmentID string) string { - return fmt.Sprintf(` -data "azurerm_subscription" "current" {} - -resource "azurerm_azuread_application" "test" { - name = "acctestspa-%d" -} - -resource "azurerm_azuread_service_principal" "test" { - application_id = "${azurerm_azuread_application.test.application_id}" -} - -resource "azurerm_role_assignment" "test" { - name = "%s" - scope = "${data.azurerm_subscription.current.id}" - role_definition_name = "Reader" - principal_id = "${azurerm_azuread_service_principal.test.id}" -} -`, rInt, roleAssignmentID) -} diff --git a/azurerm/resource_arm_role_assignment_test.go b/azurerm/resource_arm_role_assignment_test.go index 0b8fc4ebbd22..b97eff9a5243 100644 --- a/azurerm/resource_arm_role_assignment_test.go +++ b/azurerm/resource_arm_role_assignment_test.go @@ -22,6 +22,9 @@ func TestAccAzureRMRoleAssignment(t *testing.T) { "builtin": testAccAzureRMRoleAssignment_builtin, "custom": testAccAzureRMRoleAssignment_custom, }, + "assingment": { + "sp": testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment, + }, "import": { "basic": testAccAzureRMRoleAssignment_importBasic, "custom": testAccAzureRMRoleAssignment_importCustom, @@ -195,6 +198,31 @@ func testCheckAzureRMRoleAssignmentDestroy(s *terraform.State) error { return nil } +func testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(t *testing.T) { + resourceName := "azurerm_azuread_service_principal.test" + + ri := acctest.RandInt() + id := uuid.New().String() + config := testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(ri, id) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMActiveDirectoryServicePrincipalDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMActiveDirectoryServicePrincipalExists(resourceName), + testCheckAzureRMRoleAssignmentExists("azurerm_role_assignment.test"), + resource.TestCheckResourceAttrSet(resourceName, "display_name"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), + ), + }, + }, + }) +} + func testAccAzureRMRoleAssignment_emptyNameConfig() string { return ` data "azurerm_subscription" "primary" {} @@ -292,3 +320,24 @@ resource "azurerm_role_assignment" "test" { } `, roleDefinitionId, rInt, roleAssignmentId) } + +func testAccAzureRMActiveDirectoryServicePrincipal_roleAssignment(rInt int, roleAssignmentID string) string { + return fmt.Sprintf(` +data "azurerm_subscription" "current" {} + +resource "azurerm_azuread_application" "test" { + name = "acctestspa-%d" +} + +resource "azurerm_azuread_service_principal" "test" { + application_id = "${azurerm_azuread_application.test.application_id}" +} + +resource "azurerm_role_assignment" "test" { + name = "%s" + scope = "${data.azurerm_subscription.current.id}" + role_definition_name = "Reader" + principal_id = "${azurerm_azuread_service_principal.test.id}" +} +`, rInt, roleAssignmentID) +}