-
Notifications
You must be signed in to change notification settings - Fork 16
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
Replaces go-autorest MS graph client with msgraph-sdk-go #121
Conversation
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.
Looks good! question about the error handling: I noticed that a lot of the if err
checks return the error directly. Do we feel that the error returned provides enough context? If not, we can consider wrapping them.
Good point @raymonstah. Added more error context and moved some context around in 3bcea36. |
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.
Looks great, thanks for working through this!
Overview
This PR replaces the deprecated go-autorest module with msgraph-sdk-go. Additionally, it migrates the azure-sdk-for-go to the new package structure (see related issue).
There are no known API changes or compatibility breaks in this PR.
Testing
All tests in the repository are passing. Additionally, I manually tested the secrets engine functionality using our Terraform bootstrap which pulls an IMDS token off of a VM.
I've additionally added an acceptance test that actually uses Azure resources to login in.
Related Issues/Pull Requests
Contributor Checklist