Bug: RoleAssignment needs a GUID as azureName

[Sbou]

trafficstars

Version of Azure Service Operator mcr.microsoft.com/k8s/azureserviceoperator:v2.0.0-beta.0

Describe the bug When I try to add a RoleAssignment to a ServiceBus have got an error about the azureName. This field is used to put the role assignment to Azure as ID, but for a RoleAssignment it should be a GUID.

ASO use the resource name as azureName by default, so it doesn' work.

To Reproduce Steps to reproduce the behavior:

• Apply this resource definition

apiVersion: authorization.azure.com/v1alpha1api20200801preview
kind: RoleAssignment
metadata:
  name: test-role-ass
  namespace: monitoring
spec:
  location: "East US"
  owner: 
    group: servicebus.azure.com
    name: cdm-device-monitoring-sb
    kind: Namespace
  # This is the Principal ID of the AAD identity to which the role will be assigned
  principalId: c9ad1210-fbe8-49ad-8d92-XXXXXXXXX
  roleDefinitionReference:
    # This ARM ID represents "Contributor" - you can read about other built in roles here: https://docs.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
    armId: /subscriptions/9e7dbf8b-6a81-4beb-9434-8113e1c7a0a4/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c

• Received this error:

 The role assignment ID 'test-role-ass' is not valid. The role assignment ID must be a GUID.: PUT https://management.azure.com/subscriptions/XXXXX-XXXX-XXX-XXXXXX/resourceGroups/cdm-device-monitoring-rg/providers/Microsoft.ServiceBus/namespaces/cdm-device-monitoring-sb/providers/Microsoft.Authorization/roleAssignments/test-role-ass
      --------------------------------------------------------------------------------
      RESPONSE 400: 400 Bad Request
      ERROR CODE: InvalidRoleAssignmentId
      --------------------------------------------------------------------------------
      {
        "error": {
          "code": "InvalidRoleAssignmentId",
          "message": "The role assignment ID 'test-role-ass' is not valid. The role assignment ID must be a GUID."
        }
      }
      --------------------------------------------------------------------------------

Expected behavior The RoleAssignment is working. Either, the azureName field is mandatory with a correct description, either this field is autogenerated with a correct description in the documentation.

[super-harsh]

Thanks for raising this, In the above example the name for RoleAssingment is expected to be a GUID. We have corrected the RoleAssignment sample here to fix above and make it clearer.

[matthchr]

Thinking on this more, I'm wondering if we should handcraft a webhook that generates a GUID AzureName for this resource if one isn't specified.

That way users don't have to worry about it if they don't want to. We probably don't need a general solution as (in general) Azure resources don't require specific names like this. RoleAssignment is a bit of an outlier in that regard.

[mehighlow]

@matthchr , The name for RoleAssingment is expected to be a GUID which really confuses a lot as GUID is not a human-friendly name that you need to set under metadata.name section.

In case it is an Azure ARM requirement it'd be nice if ASOv2 can create some random GUID under the hood and match it with the name a user defines.

[matthchr]

In case it is an Azure ARM requirement it'd be nice if ASOv2 can create some random GUID under the hood and match it with the name a user defines.

It is an Azure requirement.

We previously haven't done GUID/name generation because we want things to be repeatable/transferable between clusters as well, and if we just blindly generate a new GUID when we deploy a RoleAssignment with name MyRoleAssignment, then if you copy that into another cluster you'll get another RoleAssignment.

What I think might work well and both improve usability as well as maintain the consistency guarantee is to generate the GUID from a seed based on the name you supply. That means the same name RoleAssignment even in two clusters would point to the same RoleAssignment GUID (as long as they were in the same sub). There may be flaws to that idea I haven't thought through yet but I think it's worth exploring.

[mehighlow]

@matthchr , that is exactly what I did in my helm chart to deploy different ENVs with repeatable(predictable) results. I take sha1sum from a release name and slice it with trunc and subst build-in helm functions. looks a bit ugly, but works.

e.g.

{{- define "project.kv-roleassignement.fullname" -}}
{{- $part1 := .Release.Name | sha1sum | trunc 8 -}}
{{- $part2 := .Release.Name | sha1sum | trunc -4 -}}
{{- $part3 := .Release.Name | sha1sum | trunc -4 -}}
{{- $part4 := .Release.Name | sha1sum | trunc -4 -}}
{{- $part5 := .Release.Name | sha1sum | trunc -12 -}}
{{ printf "%s-%s-%s-%s-%s" $part1 $part2 $part3 $part4 $part5 }}
{{- end -}}

That is an ugly workaround and I believe you can do this much better in the ASO backend.

[matthchr]

Good validation that the semantics (same name == same GUID) make sense for at least 1 user though. Thanks. Definitely sounds worth doing in our backend.

[matthchr]

We're still interested in doing this, but probably not for v2.1.0