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

Role assignment retry for service principal #2204

Merged
merged 3 commits into from
Nov 7, 2018

Conversation

liemnotliam
Copy link
Contributor

This PR looks to address issue https://github.com/terraform-providers/terraform-provider-azurerm/issues/1635, where a newly created service principal was not available for a role assignment.

There were a couple of merged PRs - PR #1647 and PR #1934 - that addressed issue https://github.com/terraform-providers/terraform-provider-azurerm/issues/1635. The role assignment create would retry, but it wouldn't retry on the error that was being returned when the created service principal wasn't available yet (400 PrincipalNotFound).

Test results for service principals:

~/go/src/github.com/terraform-providers/terraform-provider-azurerm on role-assignment-retry*
λ TESTARGS="-run TestAccAzureRMActiveDirectoryServicePrincipal_ -count=1" make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccAzureRMActiveDirectoryServicePrincipal_ -count=1 -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
?       github.com/terraform-providers/terraform-provider-azurerm       [no test files]
=== RUN   TestAccAzureRMActiveDirectoryServicePrincipal_importBasic
--- PASS: TestAccAzureRMActiveDirectoryServicePrincipal_importBasic (35.57s)
=== RUN   TestAccAzureRMActiveDirectoryServicePrincipal_basic
--- PASS: TestAccAzureRMActiveDirectoryServicePrincipal_basic (27.57s)
=== RUN   TestAccAzureRMActiveDirectoryServicePrincipal_roleAssignment
--- PASS: TestAccAzureRMActiveDirectoryServicePrincipal_roleAssignment (46.88s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm       110.056s
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/authentication        0.021s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure 0.041s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/kubernetes    0.009s [no tests to run]
?       github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/resourceproviders     [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/response      0.038s [no tests to run]
?       github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/set   [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress      0.028s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate      0.017s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils 0.020s [no tests to run]
?       github.com/terraform-providers/terraform-provider-azurerm/version       [no test files]

Test results for role assignments:

~/go/src/github.com/terraform-providers/terraform-provider-azurerm on role-assignment-retry*
λ TESTARGS="-run TestAccAzureRMRoleAssignment -count=1" make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccAzureRMRoleAssignment -count=1 -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
?       github.com/terraform-providers/terraform-provider-azurerm       [no test files]
=== RUN   TestAccAzureRMRoleAssignment
=== RUN   TestAccAzureRMRoleAssignment/basic
=== RUN   TestAccAzureRMRoleAssignment/basic/emptyName
=== RUN   TestAccAzureRMRoleAssignment/basic/roleName
=== RUN   TestAccAzureRMRoleAssignment/basic/dataActions
=== RUN   TestAccAzureRMRoleAssignment/basic/builtin
=== RUN   TestAccAzureRMRoleAssignment/basic/custom
=== RUN   TestAccAzureRMRoleAssignment/import
=== RUN   TestAccAzureRMRoleAssignment/import/basic
=== RUN   TestAccAzureRMRoleAssignment/import/custom
--- PASS: TestAccAzureRMRoleAssignment (269.25s)
    --- PASS: TestAccAzureRMRoleAssignment/basic (201.36s)
        --- PASS: TestAccAzureRMRoleAssignment/basic/emptyName (39.28s)
        --- PASS: TestAccAzureRMRoleAssignment/basic/roleName (35.18s)
        --- PASS: TestAccAzureRMRoleAssignment/basic/dataActions (31.04s)
        --- PASS: TestAccAzureRMRoleAssignment/basic/builtin (33.00s)
        --- PASS: TestAccAzureRMRoleAssignment/basic/custom (62.88s)
    --- PASS: TestAccAzureRMRoleAssignment/import (67.89s)
        --- PASS: TestAccAzureRMRoleAssignment/import/basic (33.18s)
        --- PASS: TestAccAzureRMRoleAssignment/import/custom (34.70s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm       269.282s
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/authentication        0.022s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure 0.024s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/kubernetes    0.010s [no tests to run]
?       github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/resourceproviders     [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/response      0.020s [no tests to run]
?       github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/set   [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress      0.023s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate      0.024s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils 0.021s [no tests to run]
?       github.com/terraform-providers/terraform-provider-azurerm/version       [no test files]

I'm open to suggestions and ideas towards this possible fix.

* Retry on 400 and PrincipalNotFound error when waiting for service
principal to become available.
* Add test for service principal creation and subsequent role
assignment.
@ghost ghost added size/M and removed size/S labels Nov 7, 2018
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @liemnotliam,

Thanks for this change, it doesn't solve the root problem but its a good work around.

I hope you don't mind but i've moved the test into the role def's file, now LGTM 👍

@katbyte katbyte added this to the 1.19.0 milestone Nov 7, 2018
@ghost
Copy link

ghost commented Mar 6, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants