-
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
Update azurerm_policy_set_definition
- Support policy_definition_reference_id
#7018
Update azurerm_policy_set_definition
- Support policy_definition_reference_id
#7018
Conversation
…azurerm_policy_set_definition
45842a3
to
43e13fe
Compare
azurerm_policy_assignment
, azurerm_policy_definition
, azurerm_policy_set_definition
, azurerm_policy_remediation
azurerm_policy_assignment
, azurerm_policy_definition
, azurerm_policy_set_definition
, azurerm_policy_remediation
- Update policy
api-version from 2018-05-01 to 2019-09-01
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.
@ArcturusZhang - i don't see any tests for the new properties?
Hi @katbyte there is a new block |
azurerm_policy_assignment
, azurerm_policy_definition
, azurerm_policy_set_definition
, azurerm_policy_remediation
- Update policy
api-version from 2018-05-01 to 2019-09-01azurerm_policy_set_definition
- Support policy_definition_reference_id
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.
@ArcturusZhang good catch on the additional error handling.
I'm getting failures with the ...Deprecated
tests with the policy_definitions
attribute:
policy_definitions: "[{\"parameters\":{\"listOfAllowedLocations\":{\"value\":\"[parameters('allowedLocations')]\"}},\"policyDefinitionId\":\"/providers/Microsoft.Authorization/policyDefinitions/e765b5de-1225-4ba3-bd56-1ac6695af988\",\"policyDefinitionReferenceId\":\"15086293629187003799\"}]" => " [\n {\n \"parameters\": {\n \"listOfAllowedLocations\": {\n \"value\": \"[parameters('allowedLocations')]\"\n }\n },\n \"policyDefinitionId\": \"/providers/Microsoft.Authorization/policyDefinitions/e765b5de-1225-4ba3-bd56-1ac6695af988\"\n }\n ]\n"
Also it looks like a rebase onto master is needed.
Hi @manicminer thanks for your review! I have merged master back to this branch, and on my local machine, all the tests are passing. Please have a look again
|
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.
@ArcturusZhang test coverage looks good and they are now passing
This has been released in version 2.19.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.19.0"
}
# ... other configuration ... |
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! |
Upgrade api-version of policy from2018-05-01
to2019-09-01
, fixed the compile errors and runtime type conversion errors.policy_definitions
in resource and data sourceazurerm_policy_set_definition
and replaced withpolicy_definition_reference
block which introducespolicy_definition_reference_id
(Fixes azurerm_policy_set_definition does not set policyDefinitionReferenceId #6843 )