Skip to content

Add properties of Identity. #8647

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 3, 2020
Merged

Add properties of Identity. #8647

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78d0aa7
Add missed properties of Identity.
dandanwang0320 Mar 9, 2020
063a87b
Fix prettier check.
dandanwang0320 Mar 9, 2020
ee67e47
Add read only according to comment.
dandanwang0320 Apr 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion ...resources/resource-manager/Microsoft.Solutions/stable/2019-07-01/managedapplications.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,16 +1606,39 @@
"type": "string",
"description": "The identity type.",
"enum": [
"SystemAssigned"
"SystemAssigned",
Copy link
Member

@lirenhe lirenhe Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this cause breaking changes

Copy link
Contributor Author

@dandanwang0320 dandanwang0320 Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be breaking. It's adding new value to enum and added a not required property. Old schema should still be accepted.

Copy link
Member

@lirenhe lirenhe Mar 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have SDK released for this? As the enum is not modeled as string, new SDK released will be broken because the values in enum could be different

"UserAssigned",
"SystemAssigned, UserAssigned",
"None"
],
"x-ms-enum": {
"name": "ResourceIdentityType",
"modelAsString": false
}
},
"userAssignedIdentities": {
"type": "object",
"additionalProperties": {
"$ref": "#/definitions/userAssignedResourceIdentity"
},
"description": "The list of user identities associated with the resource. The user identity dictionary key references will be resource ids in the form: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName}'."
}
},
"description": "Identity for the resource."
},
"userAssignedResourceIdentity": {
"properties": {
"principalId": {
"type": "string",
"description": "The principal id of user assigned identity."
},
"tenantId": {
Copy link
Contributor

@pilor pilor Mar 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public docs and the MSI spec seem to imply that the contract has changed to "principalId" and "clientId"... However, the contracts in ARM still have these properties. Which is correct? https://microsoft.sharepoint.com/:w:/r/teams/azureresourcemanagerteam/_layouts/15/Doc.aspx?sourcedoc=%7B66505FCE-8057-4524-B84F-4CEFE2D8A77C%7D&file=ARMResourcesWithStrongIdentity.docx&action=default&mobileredirect=true

I've seen more RPs using clientId recently

Copy link
Contributor Author

@dandanwang0320 dandanwang0320 Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inline with current service behavior, we are trying to make them sync for now.
I talked to the code owner of the the backend service, will submit a new api version and swagger when he makes the change.

Copy link
Contributor

@pilor pilor Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, please mark them as readOnly since the user is not providing these values

"type": "string",
"description": "The tenant id of user assigned identity."
}
},
"description": "Represents the user assigned identity that is contained within the UserAssignedIdentities dictionary on ResourceIdentity"
},
"Resource": {
"properties": {
"id": {
Expand Down