[ACR Query Extension] Bug fix: omitting login server endpoint suffix #6606
Conversation
️✔️Azure CLI Extensions Breaking Change Test
|
|
Hi @CarolineNB, |
|
ACR Query Extension |
|
|
||
| with self.argument_context('acr query') as c: | ||
| c.argument('registry_name', options_list=['--name', '-n'], help='The name of the container registry that the query is run against.') | ||
| c.argument('registry_name', options_list=['--name', '-n'], validator=validate_registry_name, help='The name of the container registry that the query is run against.') |
| # Query with basic auth and filter by repository | ||
| token = self.cmd('acr login -n {registry_name} --expose-token').get_output_in_json() | ||
| self.kwargs['username'] = EMPTY_GUID | ||
| self.kwargs['password'] = token["accessToken"] |
There was a problem hiding this comment.
Hi @zhoxing-ms, is there a secure way for me to pass in credentials so that they do not show up in our recording files? This particular service is blocked by a feature flag that we cannot enable during the test--requiring us to log in to a pre-existing registry.
If not, I can remove it temporarily and we can replace with mocks until the feature flag is removed.
There was a problem hiding this comment.
Actually, you can try using the customized Preparer to avoid recording the accessToken for acr login. such as: code link
If not, I can remove it temporarily and we can replace with mocks until the feature flag is removed.
If this is only a one-time temporary need, it is also acceptable
There was a problem hiding this comment.
Preparer generates a new resource within a specified or new resource group correct?
In our case, the resource would need to exist already prior to running the tests (and we would need to login to it). Would it still be possible to use Preparer?
There was a problem hiding this comment.
The test scenario we have is that we have a pre-created resources because the feature is blocked by default we need to enable it in a specific resources. Should we avoid using pre-created resources for testing?
There was a problem hiding this comment.
@CarolineNB Yes, please use the dynamically created resources in the test instead of pre-created resources to avoid the problem of dependent resources being deleted will cause the test to be failed
|
Hey @zhoxing-ms, tests have been added. Let me know if there's anything else needed before merging. Thanks! |
src/acrquery/HISTORY.rst
Outdated
|
|
||
| Release History | ||
| =============== | ||
| 1.1.0 |
There was a problem hiding this comment.
For bug fix, please bump the patch version number. 1.0.0 -> 1.0.1
|
|
||
| with self.argument_context('acr query') as c: | ||
| c.argument('registry_name', options_list=['--name', '-n'], help='The name of the container registry that the query is run against.') | ||
| c.argument('registry_name', options_list=['--name', '-n'], validator=validate_registry_name, help='The name of the container registry that the query is run against.') |
There was a problem hiding this comment.
Question, do we need update history.rst?
|
Please take a look at this comment #6606 (comment). If you want to release a new extension version for this PR, please also modify the |
|
Added, thanks @zhoxing-ms! |
|
Hey @zhoxing-ms, is there anything else that is needed for this PR? |
|
@jaysterp Could you please review this PR again? |
|
[Release] Update index.json for extension [ acrquery ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=84533&view=results |
6 participants
This fix prevents appending .azurecr.io to queries when users specify their ACR.
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az acr query -n <registry_name> -q
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally?About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.