Skip to content
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

Jump to bottom

[IoT Hub] Track 2 updates and managed identity implementation #18098

Merged
merged 12 commits into from
May 21, 2021
Merged

[IoT Hub] Track 2 updates and managed identity implementation #18098

merged 12 commits into from
May 21, 2021

Conversation

c-ryan-k
Copy link
Member

@c-ryan-k c-ryan-k commented May 14, 2021 •
edited
Loading

Description

  • Updates to use track 2 Hub GA SDK

  • Added user-assigned identity functionality

  • Added routing endpoint identity

  • Add DeviceConnectionStateEvents as a routing source type

  • RoutingSource test updates

Testing Guide

Create a hub with a system identity and grant access to a storage account.
az iot hub create -n Hub -g ResourceGroup --mi-system-assigned --role "Storage Blob Data Contributor" --scopes "/storage/account/id"

Update a hub and add a fileupload storage identity.
az iot hub update -n Hub -g ResourceGroup --file-upload-storage-identity [system]

Show identity of a hub
az iot hub identity show -n Hub -g ResourceGroup

Assign new identities to a hub.
az iot hub identity assign -n Hub -g ResourceGroup --user-assigned "user/assigned/managed/identity/id"

Remove identities from a hub.
az iot hub identity remove -n Hub -g ResourceGroup --system --user-assigned "user/assigned/managed/identity/id"

Update hub identity type.
az iot hub identity update -n Hub -g ResourceGroup --type system_assigned

Create a routing endpoint with system-assigned identity
az iot hub routing-endpoint create ... --identity [system]

Create a route using DeviceConnectionStateEvents source type.
az iot hub route create ... --source-type DeviceConnectionStateEvents

History Notes

[IoT] az iot hub create: now supports assigning identities and assigning roles to system-managed identity.

[IoT] az iot hub update: New parameter --file-upload-storage-identity to allow for managed-identity authenticated file upload.

[IoT] az iot hub identity assign: New command to assign user/system-assigned managed identities to an IoT Hub.

[IoT] az iot hub identity show: New command to show identity property of an IoT Hub.

[IoT] az iot hub identity show: New command to update identity type of an IoT Hub.

[IoT] az iot hub identity remove: New command to remove user/system-assigned managed identities from an IoT Hub.

[IoT] az iot hub routing-endpoint create: New --identity parameter allows choosing a user/system-assigned identity for routing endpoints.

[IoT] az iot hub route create: New routing source-type DeviceConnectionStateEvents

This checklist is used to make sure that common guidelines for a pull request are followed.

@c-ryan-k
IoT Hub track 2 updates and managed identity implementation (#2)
c75e60e 
* Updates to use track 2 Hub GA SDK

* Added user-assigned identity functionality

* Added routing endpoint identity

* Add DeviceConnectionStateEvents as a routing source type

* RoutingSource test updates

* SDK version update to 2.0.0

Co-authored-by: Ryan Kelly <rykelly@microsoft.com>
@yonzhan
Copy link
Collaborator

yonzhan commented May 14, 2021

IoT Hub

@yonzhan yonzhan requested review from zhoxing-ms and jsntcy May 14, 2021 23:45
@yonzhan yonzhan added this to the S187 milestone May 14, 2021
@c-ryan-k c-ryan-k marked this pull request as ready for review May 17, 2021 08:11
@c-ryan-k c-ryan-k mentioned this pull request May 18, 2021
Merged
5 tasks
@c-ryan-k
Copy link
Member Author

@zhoxing-ms / @jsntcy is it possible to get this looked at today? We're targeting the 5/25 release and just want to make sure we don't miss it.

c-ryan-k
c-ryan-k commented May 19, 2021
View reviewed changes
src/azure-cli/azure/cli/command_modules/iot/_help.py Outdated
@@ -402,6 +402,9 @@
- name: Create an IoT Hub with the standard pricing tier S1 and 4 partitions, in the 'westus' region, with tags.
text: >
az iot hub create --resource-group MyResourceGroup --name MyIotHub --location westus --tags a=b c=d
- name: Create an IoT Hub with a system-assigned managed identity, and assign a role and scope to a storage account for the created identity.
text: >
az iot hub create --resource-group MyResourceGroup --name MyIotHub --location westus --assign-identity [system] --role "Storage Blob Data Contributor" --scopes {resourceId}
Copy link
Member Author

Choose a reason for hiding this comment

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

[system] is a string used to refer to the system-assigned managed identity, we wanted to keep in line with other services using the same parameter - just a few other examples:

azure-cli/src/azure-cli/azure/cli/command_modules/appconfig/custom.py

Line 28 in f30478c

SYSTEM_ASSIGNED_IDENTITY = '[system]'

image

@c-ryan-k @zhoxing-ms
String fixes from code review
5c394ee 
Co-authored-by: Xing Zhou <Zhou.Xing@microsoft.com>
@c-ryan-k
Copy link
Member Author

Have fixed merge conflict for test_identity_hub.yaml - but need to re-record the test, will likely have that update tomorrow AM to fix the tests.

@c-ryan-k c-ryan-k requested a review from zhoxing-ms May 19, 2021 15:16
@c-ryan-k
New format for managed identity parameters
d60a1a7 
Updated params and help

Test recording updates
zhoxing-ms
zhoxing-ms reviewed May 20, 2021
View reviewed changes
src/azure-cli/azure/cli/command_modules/iot/custom.py Outdated
Comment on lines 736 to 744
if identity_type == IdentityUpdateType.none.value:
hub_identity = _build_identity(system=False, identities=None)
elif identity_type == IdentityUpdateType.system_assigned.value:
hub_identity = _build_identity(system=True, identities=None)
elif identity_type == IdentityUpdateType.user_assigned.value:
if not hub.identity.user_assigned_identities:
raise ArgumentUsageError('Hub {0} is not currently using any user-assigned identities.'.format(hub_name))
hub_identity = _build_identity(system=False, identities=hub.identity.user_assigned_identities)

Copy link
Contributor

Choose a reason for hiding this comment

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

When both user_assigned_identities and system_assigned_identities are turned on, should they also support building in this situation?

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify, this was based on the guideline here:
#17473
Specifically here: https://github.com/Azure/azure-cli/pull/17473/files#diff-8ba75e105324557e00d490758e775b654b82a478a1b936379504cc8adbd6223cR35

It mentions that this is essentially setting the type for the identity, so if the user passes system_assigned it will set the identity type to system assigned, and remove user-assigned identities if it has to.

For user_assigned, the opposite, keep the user-assigned and remove the system identity.

If the hub has user-assigned identities already, the command would just be iot hub identity assign --system then, right?

Copy link
Contributor

@zhoxing-ms zhoxing-ms May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I have left a comment to discuss this question comment link . Let's wait for the reply from @fengzhou-msft

Copy link
Contributor

Choose a reason for hiding this comment

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

@c-ryan-k Hi, I have discussed with @fengzhou-msft , and now we have made some changes to the design of managed identity:

  1. Delete identity update command and modify identity remove to make it support removing all the user assigned identities.
    For details: {Doc} Add managed identity command guideline #17473 (comment)

  2. When the value of user_assigned parameter is [], identity remove will remove all the user assigned identities.
    For details: {Doc} Add managed identity command guideline #17473 (comment)

Copy link
Contributor

@zhoxing-ms zhoxing-ms May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Could you follow the new guideline to modify the code and fix the CI issue (it needs to pull the code from the remote dev branch)?

@c-ryan-k
Fix wrong parameter in error description
6fc1bfc
@c-ryan-k
Removed identity update command
f8ec73d 
Added support for 
emove --user-assigned to remove all user-assigned identities
@yonzhan yonzhan merged commit 4ecd48e into Azure:dev May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhoxing-ms zhoxing-ms zhoxing-ms approved these changes

@fengzhou-msft fengzhou-msft fengzhou-msft approved these changes

@kairu-ms kairu-ms kairu-ms approved these changes

@jsntcy jsntcy Awaiting requested review from jsntcy jsntcy is a code owner

@digimaun digimaun Awaiting requested review from digimaun digimaun is a code owner

@evelyn-ys evelyn-ys Awaiting requested review from evelyn-ys evelyn-ys is a code owner

@houk-ms houk-ms Awaiting requested review from houk-ms

@jiasli jiasli Awaiting requested review from jiasli jiasli is a code owner

@Juliehzl Juliehzl Awaiting requested review from Juliehzl

Assignees

@zhoxing-ms zhoxing-ms

Labels
None yet
Projects
None yet
Milestone
S187
Development

Successfully merging this pull request may close these issues.
5 participants
@c-ryan-k @yonzhan @zhoxing-ms @fengzhou-msft @kairu-ms