cert-manager / website

Source code for the cert-manager.io website, including project documentation
https://cert-manager.io
Apache License 2.0
53 stars 335 forks source link

Azure DNS pod identity incorrectly documents principal_id #693

Open darkn3rd opened 2 years ago

darkn3rd commented 2 years ago

When documenting steps to setup a managed identity, the documentation incorrectly instructs the reader to essentially the use a principalId, which is different than principal_id. The principal's object id aka clientId should be used instead.

The current incorrect documentation is:

az role assignment create --role "DNS Zone Contributor" --assignee $PRINCIPAL_ID --scope $ZONE_ID

The correct documentation should be:

az role assignment create --role "DNS Zone Contributor" --assignee $CLIENT_ID --scope $ZONE_ID

Similarly the Terraform code looks incorrect, and it should be:

resource "azurerm_role_assignment" "dns_contributor" {
  scope                = var.dns_zone_id
  role_definition_name = "DNS Zone Contributor"
  principal_id         = azurerm_user_assigned_identity.dns_identity.client_id
}

The azure_rm provider is quite confusing because the return value (or rather attribute) from azurerm_user_assigned_identity shares the same name as argument for azurerm_role_assignment, but they in fact represent different things.

References:

For the differences between these two in Azure:

The principalId is a unique identifier for the identity that's used for Azure AD administration. The clientId is a unique identifier for the application's new identity that's used for specifying which identity to use during runtime calls. (ref: https://docs.microsoft.com/en-us/azure/app-service/overview-managed-identity?tabs=dotnet)

The Terraform documentation is confusing given the name:

principal_id - (Required) The ID of the Principal (User, Group or Service Principal) to assign the Role Definition to. Changing this forces a new resource to be created. (ref: https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/role_assignment)

Given the description, the principal is the managed identity that was created, and the ID of that principal is the client_id, so in some way, principal_id could be a shortened version of principal_client_id.

maelvls commented 2 years ago

Hi @darkn3rd, thank you for raising this issue, your issue description is stellar!

Would you like to open a PR with the fix?

Thanks!

darkn3rd commented 7 months ago

Thanks for the feedback. I used to work in QA back in a time when customers were not the testers as per industry norms these days. In those days, we'd get lectured for bad issue writeups. Is anyone able to update and make changes, or do you need help with this?

Segue, is there a way to run CI over example snippets to more rapidly test, as such examples require some effort to test on the perspective platforms? If not, I could make another enhancement request for doing an examples directory, we can dump reusable snippets in there, that could later be forklifted back into the docs. This way they could be tested downstream. Currently, I do ad-hoc tests using the free tier/evals on perspective cloud (Azure, GCP, AWS).

darkn3rd commented 7 months ago

You can assign this to me, I will work on this when I do Azure experiments again.