chore: migrate azure-sdk-for-go/containerregistry to the latest release

[shahramk64]

Description

What this PR does / why we need it:

auth provider currently uses an old preview version of azure sdk for go. With the latest release of the sdk, the necessary API to exchange the AAD access token for an ACR refresh token is exposed and we can migrate to this latest release.

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):

Fixes #959

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Helm Chart Change (any edit/addition/update that is necessary for changes merged to the main branch)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

Here is a link to the successful AKS run:
https://github.com/shahramk64/forked_ratify/actions/runs/11024417178/job/30676580847?pr=2

Checklist:

• Does the affected code have corresponding tests?
• Are the changes documented, not just with inline documentation, but also with conceptual documentation such as an overview of a new feature, or task-based documentation like a tutorial? Consider if this change should be announced on your project blog.
• Does this introduce breaking changes that would require an announcement or bumping the major version?
• Do all new files have appropriate license header?

Post Merge Requirements

• MAINTAINERS: manually trigger the "Publish Package" workflow after merging any PR that indicates Helm Chart Change

[codecov]

Codecov Report

Attention: Patch coverage is 61.05263% with 37 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...kg/common/oras/authprovider/azure/azureidentity.go	61.36%	16 Missing and 1 partial ⚠️
...n/oras/authprovider/azure/azureworkloadidentity.go	56.41%	16 Missing and 1 partial ⚠️
pkg/common/oras/authprovider/azure/helper.go	75.00%	2 Missing and 1 partial ⚠️

Files with missing lines	Coverage Δ
pkg/common/oras/authprovider/azure/helper.go	75.00% <75.00%> (ø)
...kg/common/oras/authprovider/azure/azureidentity.go	60.37% <61.36%> (+35.73%)	⬆️
...n/oras/authprovider/azure/azureworkloadidentity.go	72.22% <56.41%> (+31.89%)	⬆️

... and 129 files with indirect coverage changes

[susanshi]

Hi @shahramk64 , thanks for the PR. please link to the AKS run in your fork once this is ready for review. thanks!

[binbin-li]

nit: we can follow the new pattern to generate a Ratify error: https://github.com/ratify-project/ratify/blob/dev/pkg/controllers/utils/verifier.go#L63

[shahramk64]

@binbin-li I tried to mimic the pattern that's already being used in this file (and in azureworkloadidentity.go) in multiple places. Should I go ahead and change all of them?

[shahramk64]

@akashsinghal @susanshi
I added unit tests to azureworkloadidentity_test.go. I would appreciate it if you could have a look at it. Based on what I understood, to mock the exported functions in azcontainerregistry, I needed to use dependency injection, so I refactored azureworkloadidentity.go for that.
If this is the right way to write the unit tests, I'll follow the same pattern for azureidentity.go and will try to bring the coverage up to 80%

[akashsinghal]

@akashsinghal @susanshi I added unit tests to azureworkloadidentity_test.go. I would appreciate it if you could have a look at it. Based on what I understood, to mock the exported functions in azcontainerregistry, I needed to use dependency injection, so I refactored azureworkloadidentity.go for that. If this is the right way to write the unit tests, I'll follow the same pattern for azureidentity.go and will try to bring the coverage up to 80%

Thanks @shahramk64. I think this mock approach makes sense if the underlying azure client cannot be mocked

[shahramk64]

Here is a link to the latest e2e AKS run on my forked repo:
https://github.com/shahramk64/forked_ratify/actions/runs/11451048602/job/31860970244?pr=3

[susanshi]

should this commented out code be removed @shahramk64 ?

[susanshi]

thanks for updating the SDK and making significant effort to increase code coverage

[binbin-li]

lgtm, thanks for the sdk upgrade work!