Modify Update and Delete azurerm_role_definition

[shelleybess]

This pull request addresses the issue described in #1540 where the azurerm_role_definition was unable to be updated or deleted.

Changes to Delete:
Changes have been made to get the path of the resource and pull the id from the end.

Changes to CreateOrUpdate:
Now retrieves the id from the end of the path if it exists or generates one if the resource path is empty.

(fixes #1540)

[katbyte]

Hi @shelleybess,

Thank you for opening this PR, I have two comments:

• Is there a reason not to use parseAzureResourceID() here?
• do the test functions testCheckAzureRMRoleDefinitionExists and testCheckAzureRMRoleDefinitionDestroy need to be updated with the new logic?

Look forward to getting this merged once those comments are addressed 🙂

[katbyte]

Is there a reason to not use parseAzureResourceID() here?

[shelleybess]

I took this route because I encountered an issue parsing the role definition Ids. They do not contain a resource group and parseAzureResourceID() makes assumptions about the structure of the URLs.

[ERROR] root: eval: *terraform.EvalApplyPost, err: 1 error(s) occurred:
* azurerm_role_definition.shelley-dev: Error retrieving resource Id for resource "/subscriptions/<subscription_ID>/providers/Microsoft.Authorization/roleDefinitions/<role_def_id>": No resource group name found in: "subscriptions/<subscription_id>/providers/Microsoft.Authorization/roleDefinitions/<role_def_id>"

[katbyte]

Thanks for the explanation, sounds good to me.

[katbyte]

as above

[katbyte]

Running the tests i believe you will need to update the logic as these three are failing:

------- Stdout: -------
=== RUN   TestAccAzureRMRoleDefinition_basic
--- FAIL: TestAccAzureRMRoleDefinition_basic (11.48s)
	testing.go:513: Step 0 error: Check failed: Check 1/1 error: Bad: Role Definition "azurerm_role_definition.test" (Scope: "/subscriptions/*******") does not exist
FAIL

------- Stdout: -------
=== RUN   TestAccAzureRMRoleDefinition_complete
--- FAIL: TestAccAzureRMRoleDefinition_complete (12.05s)
	testing.go:513: Step 0 error: Check failed: Check 1/1 error: Bad: Role Definition "azurerm_role_definition.test" (Scope: "/subscriptions/*******") does not exist
FAIL

------- Stdout: -------
=== RUN   TestAccAzureRMRoleDefinition_update
--- FAIL: TestAccAzureRMRoleDefinition_update (11.43s)
	testing.go:513: Step 0 error: Check failed: Check 1/5 error: Bad: Role Definition "azurerm_role_definition.test" (Scope: "/subscriptions/*******") does not exist
FAIL

[shelleybess]

I'll take a look at the tests since this may be because the tests are using the old logic.

[shelleybess]

@katbyte Thank you for reviewing!

[katbyte]

@shelleybess,

Thank you for the updates! Everything looks good to me except just one test is failing now:

------- Stdout: -------
=== RUN   TestAccAzureRMRoleDefinition_update
--- FAIL: TestAccAzureRMRoleDefinition_update (30.67s)
	testing.go:513: Step 1 error: Check failed: Check 5/6 error: azurerm_role_definition.test: Attribute 'permissions.0.not_actions.#' expected "1", got "0"
FAIL

I updated the other tests to check (i hope you don't mind) and they pass, it seems to only be an issue for the update.

[tombuildsstuff]

hey @shelleybess

Thanks for this PR - apologies for the delayed movement here. I hope you don't mind but we've ended up taking a (similar) but slightly different approach in #1887 which I'm going to close this PR in favour of.

Thanks!

[ghost]

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 👉 [email protected]. Thanks!