-
Notifications
You must be signed in to change notification settings - Fork 72
Extend generate-sas-token to take a device connection string #319
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.
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
Conversation
New feature: generate a SAS token based on only connection string
|
Thank you for this contribution and bringing up the gap in functionality as we aim for the IoT CLI to be a comprehensive tool. Will get back to you on this PR...we may want to pivot in a different implementation (that covers the same functionality). |
| cmd, | ||
| hub_name=None, | ||
| device_id=None, | ||
| device_connection_string=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of supporting an offline mode just for device connection strings, I'm thinking it would be valuable to support either hub, device or module offline SAS token generation via generic connection_string corresponding to a new --connection-string argument on this command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@digimaun absolutely agree. This just happened to cover my use case for IoT Central but agree that there's no reason to restrict to only device connection strings
| elif 'dps_name' in args: | ||
| iot_cmd_type = 'DPS' | ||
| entity_value = args['dps_name'] | ||
| entity_value = args['dps_name'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove the extra spaces.
| policy = None | ||
| key = None | ||
|
|
||
| if device_connection_string: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets add tests to exercise the scenario
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed it creates a bunch of regression issues in my az iot cli which I need to look at before I go further with it
|
@nanowireUK let us know if you want to continue working on the PR, or we can cover this functionality in a different PR. Right now my mental model for the feature is we should cover more than device connection strings. |
|
@digimaun happy to keep working on the PR and absolutely support the idea of making the implementation more generic. I'll have a go at that and update the PR. |
|
Superseded by #375 |
…lated mgmt commands (Azure#319) * Minimizes extraneous changes.
New feature: generate a SAS token based on only device connection string.
This extends the az iot cli to provide the functionality we already have in VS Code and as basic scripts in samples. We can directly generate a SAS token based only on the supplied device connection string without a requirement for any IoT Hub authentication. This is particularly useful for customers wanting to connect devices directly over MQTT to IoT Central since in this case you are not able to get an IoT Hub connection string.
This project has adopted the Microsoft Open Source Code of Conduct. For more information see the Code of Conduct FAQ or contact [email protected] with any additional questions or comments.
Thank you for contributing to the IoT extension!
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
pytest <project root> -vv