[Bug] TypeSpec for CustomerManagedKeyEncryption in Common Type v5 mismatches with corresponding Swagger spec

[abatishchev]

Describe the bug

CustomerManagedKeyEncryption in Common Type v5 defines both Guid properties as non-nullable:

"federatedClientId": {
  "type": "string",
  "format": "uuid",
  "description": "application client identity to use for accessing key encryption key Url in a different tenant. Ex: f83c6b1b-4d34-47e4-bb34-9d83df58b540"
},
"delegatedIdentityClientId": {
  "type": "string",
  "format": "uuid",
  "description": "delegated identity to use for accessing key encryption key Url. Ex: /subscriptions/fa5fc227-a624-475e-b696-cdd604c735bc/resourceGroups/<resource group>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myId. Mutually exclusive with identityType systemAssignedIdentity and userAssignedIdentity - internal use only."
}

https://github.com/Azure/azure-rest-api-specs/blob/1d9bdf047d98fb6844ce9309301ca719c1cc003d/specification/common-types/resource-management/v5/customermanagedkeys.json#L42-L53

So does this repo's replica:

typespec-azure/packages/samples/specs/resource-manager/common-types/v5/customermanagedkeys.json

Lines 42 to 53 in b10f51e

	"description": "User assigned identity to use for accessing key encryption key Url. Ex: /subscriptions/fa5fc227-a624-475e-b696-cdd604c735bc/resourceGroups/<resource group>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myId. Mutually exclusive with identityType systemAssignedIdentity."
	},
	"federatedClientId": {
	"type": "string",
	"format": "uuid",
	"description": "application client identity to use for accessing key encryption key Url in a different tenant. Ex: f83c6b1b-4d34-47e4-bb34-9d83df58b540"
	},
	"delegatedIdentityClientId": {
	"type": "string",
	"format": "uuid",
	"description": "delegated identity to use for accessing key encryption key Url. Ex: /subscriptions/fa5fc227-a624-475e-b696-cdd604c735bc/resourceGroups/<resource group>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myId. Mutually exclusive with identityType systemAssignedIdentity and userAssignedIdentity - internal use only."
	}

But the corresponding TypeSpec spec defines one of the values as nullable:

typespec-azure/packages/typespec-azure-resource-manager/lib/customer-managed-keys.tsp

Lines 39 to 43 in b10f51e

	@doc("application client identity to use for accessing key encryption key Url in a different tenant. Ex: f83c6b1b-4d34-47e4-bb34-9d83df58b540")
	federatedClientId?: uuid;
	@doc("delegated identity to use for accessing key encryption key Url. Ex: /subscriptions/fa5fc227-a624-475e-b696-cdd604c735bc/resourceGroups/<resource group>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/myId. Mutually exclusive with identityType systemAssignedIdentity and userAssignedIdentity - internal use only.")
	delegatedIdentityClientId: uuid;

Expected behavior

federatedClientId must be redefined as just federatedClientId: uuid.

or

The Common Types should be updated, what I don't think the case.

[abatishchev]

+@weshaggard who made the original change: 8b8d7c0

[abatishchev]

However, looking at the discussion on the original PR for adding the model to Common Types v4: From https://github.com/Azure/azure-rest-api-specs/pull/20144/files#r940546434 and then at the ARM RPC (legacy public version, it's the opposite (sort of):

• federatedClientId is present (but has default value "")
• delegatedIdentityClientId is not present

[markcowl]

@abatishchev Neither of these properties is marked as required in Swagger, so they should both be optional .

Also, this type only supports V5, so we don't need to worry about v4

[abatishchev]

I'd be happy to submit a PR to mark the other one nullable too