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

feat: set implicit condition version on azurerm_role_assignment #24630

Conversation

logan-bobo
Copy link
Contributor

@logan-bobo logan-bobo commented Jan 25, 2024

I noticed when using condition that condition_version is required for resource creation. However when checking out the Microsoft documentation I noticed the following.

conditionVersion: A condition version number. Defaults to 2.0 and is the only publicly supported version.

This change it to propose that on resource creation of azurerm_role_assignment, if condition is set we implicitly set condition_version to be "2.0" as this is the default and only supported version.

Happy to discuss and test, feedback would be appreciated.

@logan-bobo logan-bobo force-pushed the feat/set-implicit-condition-version branch from 89b8077 to b1afea7 Compare January 25, 2024 12:05
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.

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",
Copy link
Contributor

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?

Copy link
Contributor Author

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

@logan-bobo logan-bobo requested a review from katbyte April 16, 2024 12:38
@manicminer
Copy link
Contributor

Getting 2 failing tests with the same error:

  • TestAccRoleAssignment_condition
  • TestAccRoleAssignment_implicitCondition
Screenshot 2024-04-17 at 13 29 17

Copy link

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!

@github-actions github-actions bot added the stale label May 20, 2024
@stephybun
Copy link
Member

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!

@stephybun stephybun closed this Jun 20, 2024
@github-actions github-actions bot removed the stale label Jun 20, 2024
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 21, 2024
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.

5 participants