Skip to content
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

Fixed issue when registry creds are provided for ACRs using containerapp up command #7198

Merged

Conversation

harryli0108
Copy link
Member

Description

This PR aims to fix an issue with current container app up command in the scenario where user provides --registry-server, --registry-password, and --registry-username for ACRs. In the past, up command will treat it as ACRs in the current subscription and apply them. However, this results in issues when user starts to use ACRs cross subscriptions.

Before
image

After
Screenshot 2024-01-18 155035

Related command

az containerapp up

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)
  • My extension version conforms to the Extension version schema

For new extensions:

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.json automatically.
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.

Copy link

azure-client-tools-bot-prd bot commented Jan 18, 2024

️✔️Azure CLI Extensions Breaking Change Test
️✔️Non Breaking Changes

Copy link

Hi @harryli0108,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented Jan 18, 2024

Thank you for your contribution! We will review the pull request and get back to you soon.

@daniv-msft
Copy link
Contributor

The change looks good to me overall. However, could you please clarify: was this a regression? If yes, do you know when/how it was introduced?

@daniv-msft
Copy link
Contributor

Another question: could you please clarify if this fix should be in the extension (this repo), or in the base Azure CLI?
https://github.com/Azure/azure-cli/blob/main/src/azure-cli/azure/cli/command_modules/containerapp/_up_utils.py

At this point some of our changes are checked in in the Extensions repo (mostly for preview features), while all the base features are in the base Azure CLI repo so that customers can get these features without installing an extra extension.

@Greedygre
Copy link
Contributor

Greedygre commented Jan 31, 2024

Hi @harryli0108

The following test is failed because of the api-version of containerapp has been update from 2023-08-01-preview to 2023-11-02-preview, please rerun the test or checkout this file to main as you have rerun it before. Thanks.

FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_up_on_connected_env.py::ContainerAppUpConnectedEnvImageTest::test_containerapp_up_on_arc_e2e

 No match for the request (<Request (GET) [https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.App/containerApps?api-version=2023-11-02-preview>](https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.App/containerApps?api-version=2023-11-02-preview%3E)) was found.
E           Found 9 similar requests with 1 different matcher(s) :
E           
E           1 - (<Request (GET) [https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.App/containerApps?api-version=2023-08-01-preview>).](https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.App/containerApps?api-version=2023-08-01-preview%3E).)
E           Matchers succeeded : ['method', 'scheme', 'host', 'port', 'path']
E           Matchers failed :
E           _custom_request_query_matcher - assertion failure :
E           None

@Greedygre
Copy link
Contributor

Hi @harryli0108

Please merge the latest code from main branch.

FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_customized_keys_e2e
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_customized_keys_yaml_e2e
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_e2e
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_none_client_type
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_up_on_connected_env.py::ContainerAppUpConnectedEnvImageTest::test_containerapp_up_on_arc_e2e

@Greedygre
Copy link
Contributor

Greedygre commented Feb 1, 2024

These tests failed:

FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_customized_keys_e2e
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_customized_keys_yaml_e2e
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_e2e
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py::ContainerappServiceBindingTests::test_containerapp_dev_service_binding_none_client_type
FAILED src/containerapp/azext_containerapp/tests/latest/test_containerapp_up_on_connected_env.py::ContainerAppUpConnectedEnvImageTest::test_containerapp_up_on_arc_e2e
============ 5 failed, 107 passed, 36 skipped in 496.07s (0:08:16) ============

I will handle the others but do not include test_containerapp_up_on_arc_e2e.

test_containerapp_dev_service failed because command az containerapp service is deprecated.

Fixed: #7236

@@ -11,7 +11,8 @@ upcoming
0.3.47
++++++
* 'az containerapp add-on' : support for add-on milvus create and delete commands
* [Breaking Change] 'az containerapp service': deprecate command from Azure CLI version 2.59.0
* 'az containerapp up' : Fix registry not found error in subscription when registry server parameters are provided for ACR from another subscription
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.3.47 has been released, please move this description to upcoming.

@Greedygre
Copy link
Contributor

Where is test test_containerapp_up_artifact_with_bullseye_buildpack_e2e?

Copy link

gitguardian bot commented Feb 27, 2024

⚠️ GitGuardian has uncovered 139 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_python310_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_get_customdomainverificationid_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_get_customdomainverificationid_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerappjob_create_with_environment_id.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerappjob_create_with_environment_id.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_storage.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_node18_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_identity_system.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_identity_system.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_preview_create_with_environment_id.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_preview_create_with_environment_id.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerappjob_create_with_yaml.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_source_with_bookworm_buildpack_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_source_with_bookworm_buildpack_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_mtls.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerappjob_create_with_yaml.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerappjob_create_with_yaml.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_create_with_vnet_yaml.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_create_with_vnet_yaml.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_image_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_image_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_node18_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_node18_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_certificate_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_java_component.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_java_component.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_show_all_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_show_all_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_resiliency.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_resiliency.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_update_custom_domains.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_update_custom_domains.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_custom_domains_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_dev_add_on_binding_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_resiliency.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_container_app_mount_secret_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_container_app_mount_secret_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_on_arc_auto_install_extension_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_dapr_component_resiliency.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_dapr_component_resiliency.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_environment_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_environment_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_python310_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_python310_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_managed_service_binding_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_managed_service_binding_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_source_with_multiple_environments_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_source_with_multiple_environments_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_custom_domains_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_custom_domains_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_without_arguments_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_on_arc_auto_install_extension_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_usages.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerappjob_create_with_environment_id.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_dapr_component_resiliency.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_java_component.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_environment_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_custom_domains.yaml View secret
- Basic Auth String 1dd747e src/spring/azext_spring/tests/latest/recordings/test_app_deploy_container.yaml View secret
- Basic Auth String 1dd747e src/spring/azext_spring/tests/latest/recordings/test_app_deploy_container_command.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_update_custom_domains.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_resource_group_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_resource_group_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_custom_domains.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_custom_domains.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_custom_domains.yaml View secret
- Basic Auth String 1dd747e src/spring/azext_spring/tests/latest/recordings/test_app_deploy_container.yaml View secret
- Basic Auth String 1dd747e src/spring/azext_spring/tests/latest/recordings/test_app_deploy_container_command.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_container_app_mount_secret_update_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_container_app_mount_secret_update_e2e.yaml View secret
- Microsoft Azure Storage Account Key 1d8b8e7 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_up_source_with_bookworm_buildpack_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_show_all_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_dev_service_binding_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_dev_service_binding_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_patch_list_and_apply_with_resource_group_e2e.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_env_la_dynamic_json.yaml View secret
- Microsoft Azure Storage Account Key 0c4f9d6 src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_create_with_vnet_yaml.yaml View secret

and 59 others.

🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@Greedygre
Copy link
Contributor

Hi @wangzelin007

Can you help to merge this PR? Thanks.

@wangzelin007 wangzelin007 merged commit 372a3f3 into Azure:main Feb 29, 2024
11 of 15 checks passed
Copy link

@Moazzem-Hossain-pixel Moazzem-Hossain-pixel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help to resolve

Copy link

@MoazzemHossain-bot MoazzemHossain-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot ContainerApp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants