external-dns
external-dns copied to clipboard
WIP: Update Azure SDK and remove deprecated autorest dependency
Description
This PR updates the Azure SDK to the latest stable version and replaces the use of the deprecated autorest with azidentity.
Fixes #ISSUE
Checklist
- [ ] Unit tests updated
- [ ] End user documentation updated
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: phillebaba / name: Philip Laine (70eda8b84b57376ab7f1e09c67ce65f29a031634, d42df08c17845ba815db074b3a1c7ec7f1d685a3, 55c93e78237487e648aed3262d74ff8b281fcf2e, 985a53217cfa0fc6ef3876934e0f7f3260084398)
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: phillebaba
Once this PR has been reviewed and has the lgtm label, please assign szuecs for approval by writing /assign @szuecs
in a comment. For more information see:The Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
@phillebaba: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
Welcome @phillebaba!
It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.
You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.
You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.
You may want to refer to our testing guide if you run into trouble with your tests not passing.
If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!
Thank you, and welcome to Kubernetes. :smiley:
I have been able to validate this in Azure Public with MSI authentication. Are there any more e2e tests that can be run against Azure?
@phillebaba do you have any more context on why there is still a reference to the go-autorest
library?
github.com/Azure/go-autorest/autorest v0.11.27 // indirect github.com/Azure/go-autorest/autorest/adal v0.9.20 // indirect
Running go mod graph
shows that kubernetes/client-go
actually still uses autorest and adal in v0.25. This have however been removed in v0.26 so they should disappear. Note that the dependency is indirect so it is not a dependency that is imported by this project but indirectly from another dependency.
Thanks for the confirmation @phillebaba, that was my assumption.
@Raffo could you approve the GH workflows?
/ok-to-test
@phillebaba Seems like 1.3.0 will be released only around April after the Workload Identity extension is promoted to GA (https://github.com/Azure/azure-sdk-for-go/issues/19765#issuecomment-1376512434), though I guess it'll land with the same code as 1.3.0-beta.2
, so might be worth trying to support workload identity.
Releases:
- https://github.com/Azure/azure-sdk-for-go/releases/tag/sdk%2Fazidentity%2Fv1.3.0-beta.1
- https://github.com/Azure/azure-sdk-for-go/releases/tag/sdk%2Fazidentity%2Fv1.3.0-beta.2
@phillebaba can you rebase this? I'd love to get the azure changes in.
@phillebaba once your changes are merged, I'll update my PR targeting Azure Workload Identity (#3111). I guess I can bump version of the azidentity
to v1.3.0-beta.2
there then.
FYI @Raffo
Created this https://github.com/kubernetes-sigs/external-dns/pull/3380, as @phillebaba appears to be busy. I've got some free time to see this through.
@Raffo @siiimooon @weisdd sorry guys I have been busy with other stuff and kind of left this on ice until workload identities would be released. As stated this seems to be delayed once again so I have rebased so others can get on with other solutions.
@Raffo I was wondering if there's any chance to have this PR reviewed in coming days/weeks. Thanks!
Workload identity release candidate just came out - any chance to get this merged?
Workload identity v1.0.0 was released two days ago.
Today, the workload identity graduated to GA: https://azure.microsoft.com/en-ca/updates/ga-azure-active-directory-workload-identity-with-aks/
So, we will probably see stable release of azidentity
in a few days.
@weisdd the announcement link has gone dead and the AKS docs are still referring to it as in preview so I wonder if it's been pulled?
@stevehipwell that's interesting, I still have the cached RSS feed item:
Screenshot
With Azure docs, I've seen some examples before where "Preview" mark was left forgotten for a while after a GA announcement. So, that is not necessarily a concern, whereas the pulled announcement is. Anyway, let's wait then to see if they publish another GA announcement in coming days. :)

https://github.com/Azure/AKS/issues/1480#issuecomment-1489840667
My long-open PR which adds NS support for Azure, would need to completely rebuild after this is merged. How about integrating NS support into this PR directly? https://github.com/kubernetes-sigs/external-dns/pull/2835
/ok-to-test /lgtm /assign Raffo
@phillebaba are you in a position to rebase this?
@Raffo is this something which could be prioritised given the need to support Azure AD Workload Identity (https://github.com/kubernetes-sigs/external-dns/pull/3111) which is dependent on this PR.
Looks like the official 1.3.0 release with workload identity support has just been released, would be great to be able to get this merged in so external-dns can support workload identity
https://github.com/Azure/azure-sdk-for-go/releases/tag/sdk%2Fazidentity%2Fv1.3.0
@phillebaba Are you able to update this PR? The official release of the identity sdk with workload identity support was published last week, so would be really great to get this guy merged in to move towards workload identity support in external-dns
Sorry been busy fixing other stuff with Kubernetes upgrades. Will get this rebased and updated. I have other work with Thanos getting similar work done. My worry is that there is some configuration changes that have to be made that would create issues with AAD Pod ID and WI users. Especially because it's been a while since I created this PR.
@phillebaba Do you think you can rebase this PR ? The CI is now fixed.
@phillebaba I left one small suggestion on error handling, otherwise it LGTM.
/lgtm
Excited to see there is movement on this MR, can't wait for this feature to be available. Do we know when a release might occur with this feature included?
@mloiseleur @johngmyers can any of you put the approved tag ? 😅
Neither of us is an approver