-
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
feat: set implicit condition version on azurerm_role_assignment #24630
feat: set implicit condition version on azurerm_role_assignment #24630
Conversation
89b8077
to
b1afea7
Compare
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.
test failure
------- Stdout: -------
=== RUN TestAccRoleAssignment_implicitCondition
=== PAUSE TestAccRoleAssignment_implicitCondition
=== CONT TestAccRoleAssignment_implicitCondition
testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
Error: Missing required argument
with azurerm_role_assignment.test,
on terraform_plugin_test.tf line 37, in resource "azurerm_role_assignment" "test":
37: condition = "@Resource[Microsoft.Storage/storageAccounts/blobServices/containers:ContainerName] StringEqualsIgnoreCase 'foo_storage_container'"
"condition": all of `condition,condition_version` must be specified
--- FAIL: TestAccRoleAssignment_implicitCondition (3.10s)
FAIL
ValidateFunc: validation.StringInSlice([]string{ | ||
"1.0", |
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.
Whilst 2.0
is defaulted/supported for new resources (although it appears that's been removed from the referenced document at this point) any existing Role Assignments provisioned using 1.0
that's managed by Terraform will fail if we remove this - as such can we add 1.0
back in here?
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.
Yeah I will add this back in now
This PR is being labeled as "stale" because it has not been updated for 30 or more days. If this PR is still valid, please remove the "stale" label. If this PR is blocked, please add it to the "Blocked" milestone. If you need some help completing this PR, please leave a comment letting us know. Thank you! |
Closing since we've not heard back. @logan-bobo when you get a chance to pick this back up again let us know and we can re-open this! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
I noticed when using
condition
thatcondition_version
is required for resource creation. However when checking out the Microsoft documentation I noticed the following.This change it to propose that on resource creation of
azurerm_role_assignment
, if condition is set we implicitly setcondition_version
to be "2.0" as this is the default and only supported version.Happy to discuss and test, feedback would be appreciated.