-
Notifications
You must be signed in to change notification settings - Fork 99
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
Upgrade to non-legacy Azure SDK #459
Conversation
There is a PR here doing this already @ramondeklein |
Didn't see that one... Will check next time, before implementation. I will take the best parts of both PRs and merge it into a single PR. This PR has some advantages:
I do like the integration tests of the other PR, so I'll bring these over to this PR. |
I started off with https://github.com/jiuker/kes/tree/upgrade_azserect_client and made some changes:
During testing, I also found an issue that deleting secrets wasn't always handled properly. Recreating a secret with the same name after deleting failed sometimes. That has been fixed. |
7767c58
to
e59983b
Compare
@harshavardhana I have rebased @jiuker changes to our current |
@aead Why does it explicitly try to load the first version? If no version is supplied, then the default Azure KeyVault behavior is to fetch the most recent version. Because we don't update secrets, there is always only one version, so the actual behaviour wouldn't change. It would make implementation a bit simpler though. I could add that in this PR. |
@ramondeklein fix lint plz. |
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.
Do you have access to Azure Vault to test this?
I tested this in my private Azure account, but I could create instructions on how to test it in your own Azure account. As an alternative, I can sent you the credentials (via Slack) on how to test this in my account. |
@harshavardhana I have added a script in |
7ebad7b
to
6a63873
Compare
6a63873
to
5150a35
Compare
Processed review comment (only for reporting error for JSON deserialization issue) and rebased on the current |
PTAL @aead |
This PR upgrades from the legacy Azure SDK to the current Azure SDK for Go.