-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Support for Managed Identity on azurerm_policy_assignment #2549
Support for Managed Identity on azurerm_policy_assignment #2549
Conversation
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.
hey @olohmann
Thanks for this PR - I've taken a look and left some comments inline, but this is mostly looking good. The main question I've got is around the design of the schema, which IMO would be better to match the approach used by the Virtual Machine (and other) resources - but most of the other comments are fairly minor 👍
Thanks!
Again, thanks a lot for the great feedback! Applied requested changes. Test results: > $ make testacc TESTARGS='-run=TestAccAzureRMPolicyAssignment'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAzureRMPolicyAssignment -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 TestAccAzureRMPolicyAssignment_basic
=== PAUSE TestAccAzureRMPolicyAssignment_basic
=== RUN TestAccAzureRMPolicyAssignment_deployIfNotExists_policy
=== PAUSE TestAccAzureRMPolicyAssignment_deployIfNotExists_policy
=== RUN TestAccAzureRMPolicyAssignment_complete
=== PAUSE TestAccAzureRMPolicyAssignment_complete
=== CONT TestAccAzureRMPolicyAssignment_basic
=== CONT TestAccAzureRMPolicyAssignment_complete
=== CONT TestAccAzureRMPolicyAssignment_deployIfNotExists_policy
--- PASS: TestAccAzureRMPolicyAssignment_basic (247.32s)
--- PASS: TestAccAzureRMPolicyAssignment_complete (247.38s)
--- PASS: TestAccAzureRMPolicyAssignment_deployIfNotExists_policy (250.14s) |
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.
Hi @olohmann,
I've taken a quick look at it seems you mention UserAssinged
in the docs but its not supported in the schema?
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.
hey @olohmann
Thanks for pushing those changes - this now LGTM 👍 - I'll kick off the tests shortly
Thanks!
dismissing since changes have been pushed
Tests pass - thanks for this @olohmann :) |
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! |
This PR adds support for Managed Identity on azurerm_policy_assignment.
Addresses issue #2194
Test Results:
(fixes #2194)