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

Adding cmds & tests for Container app certs & domains #107

Merged
merged 85 commits into from
May 19, 2022

Conversation

lil131
Copy link
Collaborator

@lil131 lil131 commented May 17, 2022


This checklist is used to make sure that common guidelines for a pull request are followed.

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?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

runefa and others added 30 commits April 7, 2022 17:57
* Skeleton code

* az containerapp env show

* List kube/managed environments

* Create kube environment, wait doesn't work yet

* Update containerapp stubs (check if it is supported now)

* Containerapp env delete, polling not working yet

* Added polling for create and delete

* Use Microsoft.App RP for show, list, delete command

* Create containerapp env using Microsoft.App RP

* Add optional containerapp env create arguments

* Remove old kube environment code, naming fixes

* Containerapp create almost done

* Done containerapp create, except for --yaml. Need to test

* Containerapp show, list

* Fix helptext

* Containerapp delete

* Containerapp update. Needs secrets api to be implemented, and testing

* Add scale command

* Various validations, small fixes

* listSecrets API for updates, autogen log analytics for env

* Use space delimiter for secrets and env variables

* Verify sub is registered to Microsoft.ContainerRegistration if creating vnet enabled env, remove logs-type parameter

* Containerapp create --yaml

* Fix updating registry to do create or update

* Fix containerapp update command. Add image-name parameter to support multi container updates. Fix updating registries, containers and secrets

* started update with --yaml. Need to do create or update for when an attribute is a list of items

* use space delimiter for startup_command and args, instead of comma delimiter

* Traffic weights

* List and show revisions

* az containerapp revision restart, activate, deactivate

* Add ability for users to clear args/command in az containerapp update

* Various fixes, traffic weights fixes

* Verify subnet subscription is registered to Microsoft.ContainerServices

* GitHub Actions Update (Azure#17)

* Added models. Finished transferring Calvin's previous work.

* Updated wrong models.

* Updated models in custom.py, added githubactionclient.

* Updated envelope to be correct.

* Small bug fixes.

* Updated error handling. Fixed bugs. Initial working state.

* Added better error handling.

* Added error messages for tokens with inappropriate access rights.

* Added back get_acr_cred.

* Fixed problems from merge conflict.

* Updated names of imports from ._models.py to fix pylance erros.

* Removed random imports.

Co-authored-by: Haroon Feisal <[email protected]>

* Remove --location since location must be same as managed env

* Add options for flag names: --env-vars and --registry-srever

* Empty string to clear env_vars

* Default revisions_mode to single

* Infer acr credentials if it is acr and credentials are not provided

* fix help msg

* if image is hosted on acr, and no registry server is supplied, infer the registry server

* Added subgroups (Ingress, Registry, Secret) and updated revisions (Azure#18)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Modified update_containerapp_yaml to support updating from non-current revision.

Authored-by: Haroon Feisal <[email protected]>

* More p0 fixes (Azure#20)

* Remove --registry-login-server, only allow --registry-server

* Rename --environment-variables to --env-vars

* If no image is supplied, use default quickstart image

* Update help text (Azure#21)

* Update help text

* Update punctuation

* master -> main

* New 1.0.1 version

* Added identity commands + --assign-identity flag to containerapp create (Azure#8)

* Added identity show and assign.

* Finisheed identity remove.

* Added helps, updated identity remove to work with identity names instead of requiring identity resource ids.

* Moved helper function to utils.

* Require --identities flag when removing identities.

* Added message for assign identity with no specified identity.

* Added --assign-identity flag to containerapp create.

* Moved assign-identity flag to containerapp create.

* Fixed small logic error on remove identities when passing duplicate identities. Added warnings for certain edge cases.

* Updated param definition for identity assign --identity default.

* Added identity examples in help.

* Made sure secrets were not removed when assigning identities. Added tolerance for [system] passed with capital letters.

* Fixed error from merge.

Co-authored-by: Haroon Feisal <[email protected]>

* Dapr Commands (Azure#23)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Added dapr enable and dapr disable. Need to test more.

* Added list, show, set dapr component. Added dapr enable, disable.

* Added delete dapr delete.

* Added helps and param text.

* Changed dapr delete to dapr remove to match with dapr set.

* Commented out managed identity for whl file.

* Uncommented.

Co-authored-by: Haroon Feisal <[email protected]>

* Rename --image-name to --container-name

* Remove allowInsecure since it was messing with the api parsing

* Fix for env var being empty string

* Rename to --dapr-instrumentation-key, only infer ACR credentials if --registry-server is provided

* Remove az containerapp scale

* Fix delete containerapp errors

* Remove ingress, dapr flags from az containerapp update/revision copy

* Fix revision list -o table

* Help text fix

* Bump extension to 0.1.2

* Update managed identities and Dapr help text (Azure#25)

* Update managed identities and Dapr help text

* Update Dapr flags

* Add secretref note

* Env var options + various bug fixes (Azure#26)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

Co-authored-by: Haroon Feisal <[email protected]>

* Fixed style issues, various bug fixes (Azure#27)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

Co-authored-by: Haroon Feisal <[email protected]>

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_scenario.py

Co-authored-by: Xing Zhou <[email protected]>

* Specific Error Types + Bugfixes (Help, remove app-subnet-resource-id, removed env-var alias, added help text for --name) (Azure#28)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

* Update helps for identity, revision. Removed env-var alias for set-env-vars. Added name param help.

* Removed app-subnet-resource-id.

* Updated infrastructure subnet param help.

* Check if containerapp resource exists before attempting to delete.

* Added check before deleting managed env.

* Changed error types to be more specific.

* Removed check before deletion. Removed comments.

Co-authored-by: Haroon Feisal <[email protected]>

* Reset to 0.1.0 version, remove unneeded options-list

* Update min cli core version

* Fixed style issues. (Azure#30)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix linter issues

* Use custom-show-command

* Removed --ids from revision, secret, registry list.

* Add linter exclusions

* Fix polling on delete containerapp

* Fix error handling

* Add Container App Service

* Fix flake linter

* Fix help text

* Mark extension as preview

* Add python 3.9 and 3.10 as supported

* Remove registries and secrets from az containerapp update, in favor of registry and secret subgroup

* Fix YAML not working

* Move import to inside deserialize function

* Ingress enable --transport default. Secret list returns empty array. Secret update prints message saying user needs to restart their apps. Added show-values flag to secret list. Fixed yaml datetime field issues, replaced x00 values that also came up during testing.

* Fixed dapr in create.

* Revert "Ingress enable --transport default. Secret list returns empty array. Secret update prints message saying user needs to restart their apps. Added show-values flag to secret list. Fixed yaml datetime field issues, replaced x00 values that also came up during testing."

This reverts commit 51bc543.

* Revert "Fixed dapr in create."

This reverts commit 37030ad.

* Ingress enable --transport default. Secret list returns empty array. Secret update prints message saying user needs to restart their apps. Added show-values flag to secret list. Fixed yaml datetime field issues, replaced x00 values that also came up during testing.

* Skeleton code

* az containerapp env show

* List kube/managed environments

* Create kube environment, wait doesn't work yet

* Update containerapp stubs (check if it is supported now)

* Containerapp env delete, polling not working yet

* Added polling for create and delete

* Use Microsoft.App RP for show, list, delete command

* Create containerapp env using Microsoft.App RP

* Add optional containerapp env create arguments

* Remove old kube environment code, naming fixes

* Containerapp create almost done

* Done containerapp create, except for --yaml. Need to test

* Containerapp show, list

* Fix helptext

* Containerapp delete

* Containerapp update. Needs secrets api to be implemented, and testing

* Add scale command

* Various validations, small fixes

* listSecrets API for updates, autogen log analytics for env

* Use space delimiter for secrets and env variables

* Verify sub is registered to Microsoft.ContainerRegistration if creating vnet enabled env, remove logs-type parameter

* Containerapp create --yaml

* Fix updating registry to do create or update

* Fix containerapp update command. Add image-name parameter to support multi container updates. Fix updating registries, containers and secrets

* started update with --yaml. Need to do create or update for when an attribute is a list of items

* use space delimiter for startup_command and args, instead of comma delimiter

* Traffic weights

* List and show revisions

* az containerapp revision restart, activate, deactivate

* Add ability for users to clear args/command in az containerapp update

* Various fixes, traffic weights fixes

* Verify subnet subscription is registered to Microsoft.ContainerServices

* GitHub Actions Update (Azure#17)

* Added models. Finished transferring Calvin's previous work.

* Updated wrong models.

* Updated models in custom.py, added githubactionclient.

* Updated envelope to be correct.

* Small bug fixes.

* Updated error handling. Fixed bugs. Initial working state.

* Added better error handling.

* Added error messages for tokens with inappropriate access rights.

* Added back get_acr_cred.

* Fixed problems from merge conflict.

* Updated names of imports from ._models.py to fix pylance erros.

* Removed random imports.

Co-authored-by: Haroon Feisal <[email protected]>

* Remove --location since location must be same as managed env

* Add options for flag names: --env-vars and --registry-srever

* Empty string to clear env_vars

* Default revisions_mode to single

* Infer acr credentials if it is acr and credentials are not provided

* fix help msg

* if image is hosted on acr, and no registry server is supplied, infer the registry server

* Added subgroups (Ingress, Registry, Secret) and updated revisions (Azure#18)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Modified update_containerapp_yaml to support updating from non-current revision.

Authored-by: Haroon Feisal <[email protected]>

* More p0 fixes (Azure#20)

* Remove --registry-login-server, only allow --registry-server

* Rename --environment-variables to --env-vars

* If no image is supplied, use default quickstart image

* Update help text (Azure#21)

* Update help text

* Update punctuation

* master -> main

* New 1.0.1 version

* Added identity commands + --assign-identity flag to containerapp create (Azure#8)

* Added identity show and assign.

* Finisheed identity remove.

* Added helps, updated identity remove to work with identity names instead of requiring identity resource ids.

* Moved helper function to utils.

* Require --identities flag when removing identities.

* Added message for assign identity with no specified identity.

* Added --assign-identity flag to containerapp create.

* Moved assign-identity flag to containerapp create.

* Fixed small logic error on remove identities when passing duplicate identities. Added warnings for certain edge cases.

* Updated param definition for identity assign --identity default.

* Added identity examples in help.

* Made sure secrets were not removed when assigning identities. Added tolerance for [system] passed with capital letters.

* Fixed error from merge.

Co-authored-by: Haroon Feisal <[email protected]>

* Dapr Commands (Azure#23)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Added dapr enable and dapr disable. Need to test more.

* Added list, show, set dapr component. Added dapr enable, disable.

* Added delete dapr delete.

* Added helps and param text.

* Changed dapr delete to dapr remove to match with dapr set.

* Commented out managed identity for whl file.

* Uncommented.

Co-authored-by: Haroon Feisal <[email protected]>

* Rename --image-name to --container-name

* Remove allowInsecure since it was messing with the api parsing

* Fix for env var being empty string

* Rename to --dapr-instrumentation-key, only infer ACR credentials if --registry-server is provided

* Remove az containerapp scale

* Fix delete containerapp errors

* Remove ingress, dapr flags from az containerapp update/revision copy

* Fix revision list -o table

* Help text fix

* Bump extension to 0.1.2

* Update managed identities and Dapr help text (Azure#25)

* Update managed identities and Dapr help text

* Update Dapr flags

* Add secretref note

* Env var options + various bug fixes (Azure#26)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

Co-authored-by: Haroon Feisal <[email protected]>

* Fixed style issues, various bug fixes (Azure#27)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

Co-authored-by: Haroon Feisal <[email protected]>

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_scenario.py

Co-authored-by: Xing Zhou <[email protected]>

* Specific Error Types + Bugfixes (Help, remove app-subnet-resource-id, removed env-var alias, added help text for --name) (Azure#28)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

* Update helps for identity, revision. Removed env-var alias for set-env-vars. Added name param help.

* Removed app-subnet-resource-id.

* Updated infrastructure subnet param help.

* Check if containerapp resource exists before attempting to delete.

* Added check before deleting managed env.

* Changed error types to be more specific.

* Removed check before deletion. Removed comments.

Co-authored-by: Haroon Feisal <[email protected]>

* Reset to 0.1.0 version, remove unneeded options-list

* Update min cli core version

* Fixed style issues. (Azure#30)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix linter issues

* Use custom-show-command

* Removed --ids from revision, secret, registry list.

* Add linter exclusions

* Fix polling on delete containerapp

* Fix error handling

* Add Container App Service

* Fix flake linter

* Fix help text

* Mark extension as preview

* Add python 3.9 and 3.10 as supported

* Remove registries and secrets from az containerapp update, in favor of registry and secret subgroup

* Fix YAML not working

* Move import to inside deserialize function

* Dapr moved from Template to Configuration

* Use aka.ms link for containerapps yaml

* Updated dapr enable/disable to current spec.

* Fixed oversight.

* Remove revisions-mode from containerapp update

* Fixed dapr enable property names. (Azure#47)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix exceptions with using --yaml in containerapp create/update

* Rename history msg

* Include fqdn in containerapp table output

* Added ingress messages.

* Revert history msg

* Reduced redundant code between revision copy and containerapp update.

* Fixed merge issues.

* Fixed merge conflicts, moved helper function

Co-authored-by: Calvin Chan <[email protected]>
Co-authored-by: Haroon Feisal <[email protected]>
Co-authored-by: Anthony Chu <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
* Added more specific MSI help text.

* Updated help text.

Co-authored-by: Haroon Feisal <[email protected]>
* Add tests for containerapp create

* All tests under the same function to share environment - need to figure how to get multiple functions to share environment

* Basic tests
* Added managed identity tests.

* Fixed msi tests.

Co-authored-by: Haroon Feisal <[email protected]>
* Added managed identity tests.

* Fixed msi tests.

* Added live_only to managed identity tests.

* Changed region to eastus2 from canary.

Co-authored-by: Haroon Feisal <[email protected]>
…mmands, log streaming commands (Azure#72)

* Skeleton code

* az containerapp env show

* List kube/managed environments

* Create kube environment, wait doesn't work yet

* Update containerapp stubs (check if it is supported now)

* Containerapp env delete, polling not working yet

* Added polling for create and delete

* Use Microsoft.App RP for show, list, delete command

* Create containerapp env using Microsoft.App RP

* Add optional containerapp env create arguments

* Remove old kube environment code, naming fixes

* Containerapp create almost done

* Done containerapp create, except for --yaml. Need to test

* Containerapp show, list

* Fix helptext

* Containerapp delete

* Containerapp update. Needs secrets api to be implemented, and testing

* Add scale command

* Various validations, small fixes

* listSecrets API for updates, autogen log analytics for env

* Use space delimiter for secrets and env variables

* Verify sub is registered to Microsoft.ContainerRegistration if creating vnet enabled env, remove logs-type parameter

* Containerapp create --yaml

* Fix updating registry to do create or update

* Fix containerapp update command. Add image-name parameter to support multi container updates. Fix updating registries, containers and secrets

* started update with --yaml. Need to do create or update for when an attribute is a list of items

* use space delimiter for startup_command and args, instead of comma delimiter

* Traffic weights

* List and show revisions

* az containerapp revision restart, activate, deactivate

* Add ability for users to clear args/command in az containerapp update

* Various fixes, traffic weights fixes

* Verify subnet subscription is registered to Microsoft.ContainerServices

* GitHub Actions Update (Azure#17)

* Added models. Finished transferring Calvin's previous work.

* Updated wrong models.

* Updated models in custom.py, added githubactionclient.

* Updated envelope to be correct.

* Small bug fixes.

* Updated error handling. Fixed bugs. Initial working state.

* Added better error handling.

* Added error messages for tokens with inappropriate access rights.

* Added back get_acr_cred.

* Fixed problems from merge conflict.

* Updated names of imports from ._models.py to fix pylance erros.

* Removed random imports.

Co-authored-by: Haroon Feisal <[email protected]>

* Remove --location since location must be same as managed env

* Add options for flag names: --env-vars and --registry-srever

* Empty string to clear env_vars

* Default revisions_mode to single

* Infer acr credentials if it is acr and credentials are not provided

* fix help msg

* if image is hosted on acr, and no registry server is supplied, infer the registry server

* Added subgroups (Ingress, Registry, Secret) and updated revisions (Azure#18)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Modified update_containerapp_yaml to support updating from non-current revision.

Authored-by: Haroon Feisal <[email protected]>

* More p0 fixes (Azure#20)

* Remove --registry-login-server, only allow --registry-server

* Rename --environment-variables to --env-vars

* If no image is supplied, use default quickstart image

* Update help text (Azure#21)

* Update help text

* Update punctuation

* master -> main

* New 1.0.1 version

* Added identity commands + --assign-identity flag to containerapp create (Azure#8)

* Added identity show and assign.

* Finisheed identity remove.

* Added helps, updated identity remove to work with identity names instead of requiring identity resource ids.

* Moved helper function to utils.

* Require --identities flag when removing identities.

* Added message for assign identity with no specified identity.

* Added --assign-identity flag to containerapp create.

* Moved assign-identity flag to containerapp create.

* Fixed small logic error on remove identities when passing duplicate identities. Added warnings for certain edge cases.

* Updated param definition for identity assign --identity default.

* Added identity examples in help.

* Made sure secrets were not removed when assigning identities. Added tolerance for [system] passed with capital letters.

* Fixed error from merge.

Co-authored-by: Haroon Feisal <[email protected]>

* Dapr Commands (Azure#23)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Added dapr enable and dapr disable. Need to test more.

* Added list, show, set dapr component. Added dapr enable, disable.

* Added delete dapr delete.

* Added helps and param text.

* Changed dapr delete to dapr remove to match with dapr set.

* Commented out managed identity for whl file.

* Uncommented.

Co-authored-by: Haroon Feisal <[email protected]>

* Rename --image-name to --container-name

* Remove allowInsecure since it was messing with the api parsing

* Fix for env var being empty string

* Rename to --dapr-instrumentation-key, only infer ACR credentials if --registry-server is provided

* Remove az containerapp scale

* Fix delete containerapp errors

* Remove ingress, dapr flags from az containerapp update/revision copy

* Fix revision list -o table

* Help text fix

* Bump extension to 0.1.2

* Update managed identities and Dapr help text (Azure#25)

* Update managed identities and Dapr help text

* Update Dapr flags

* Add secretref note

* Env var options + various bug fixes (Azure#26)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

Co-authored-by: Haroon Feisal <[email protected]>

* Fixed style issues, various bug fixes (Azure#27)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

Co-authored-by: Haroon Feisal <[email protected]>

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_scenario.py

Co-authored-by: Xing Zhou <[email protected]>

* Specific Error Types + Bugfixes (Help, remove app-subnet-resource-id, removed env-var alias, added help text for --name) (Azure#28)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

* Update helps for identity, revision. Removed env-var alias for set-env-vars. Added name param help.

* Removed app-subnet-resource-id.

* Updated infrastructure subnet param help.

* Check if containerapp resource exists before attempting to delete.

* Added check before deleting managed env.

* Changed error types to be more specific.

* Removed check before deletion. Removed comments.

Co-authored-by: Haroon Feisal <[email protected]>

* Reset to 0.1.0 version, remove unneeded options-list

* Update min cli core version

* Fixed style issues. (Azure#30)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix linter issues

* Use custom-show-command

* Removed --ids from revision, secret, registry list.

* Add linter exclusions

* Fix polling on delete containerapp

* Fix error handling

* Add Container App Service

* Fix flake linter

* Fix help text

* Mark extension as preview

* Add python 3.9 and 3.10 as supported

* Remove registries and secrets from az containerapp update, in favor of registry and secret subgroup

* Fix YAML not working

* Move import to inside deserialize function

* Dapr moved from Template to Configuration

* Use aka.ms link for containerapps yaml

* Updated dapr enable/disable to current spec.

* Fixed oversight.

* Remove revisions-mode from containerapp update

* Fixed dapr enable property names. (Azure#47)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix exceptions with using --yaml in containerapp create/update

* Rename history msg

* Include fqdn in containerapp table output

* Added ingress messages.

* Revert history msg

* Add basic test case

* Remove managed-identity support for first release of CLI

* Need to investigate test flakiness

* Update _help.py

removing duplicate help

* Added prototype of container up.

* Fixed deploy from acr registry image infer credentials issue.

* Tried to add source.

* Added acr build.

* Finished acr build functionality.

* Added acr create functionality and pull registry from existing containerapp if it exists.

* Fixed bugs.

* Check if rg exists and create one with name if it doesn't.

* initial containerapp ssh implementation

* fix interactive commands (vim); handle ctrl + c instead of exiting

* fix style and linter issues

* Added disable verbose. Moved utils into utils.py.

* fix for ssh for windows clients

* fix for unix

* Disable verbose now uses sdk poller so it gives running animation.

* Added helps for params. Added error handling for non acr registry passed with source. Ignore param disable_warnings for create and no_wait for up.

* Updated disable_warnings ignore. Removed disable_warnings from update_containerapp.

* add terminal resizing

* reorganize code; implement terminal resizing, add startup command param, etc

* organize code, remove token from warning output

* add validations, add replica commands

* use the correct API for fetching default container; remove is_preview

* Renamed silent to quiet.

* Fixed style issues.

* add log streaming, bump version number and add to HISTORY.rst

* add basic ssh test

* Added workspace name and fqdn to dry_run_str. Added indicators of if the resources are new or existing to dry_run_str.

* fix ssh test for windows

* Check RP for location when not provided. Open Dockerfile and use EXPOSE for CA port.

* fix windows arrow keys after exit

* fix typo, add logstream test, remove token from logstream output

* Removed print statement.

* Updated dockerfile expose automatic ingress feature.

* Removed dry run str, added dry run obj instead.

* use bearer auth; fix --command bug

* add handling for smooth transition to new URL path

* Fixed merge conflict.

* fix merge conflicts

* Create env if name passed and it doesn't exist.

* Added missing import from merge.

* Added prototype for new environment workflow.

* Finished environment logic.

* Minor updates before demo.

* add 'az containerapp github up' (wip)

* various fixes for demo

* rearrange github up code

* merge haroonf/containerappup

* start up refactor

* add --repo to up and refactor up

* reorganize code more; fix various bugs

* fix linter issues, fix lingering exec/tail improvements

* update history

* update output

* bug fixes for --repo

* fix --source bug

* fix --source

* minor bug fixes

* Added API change.

* Finished API change, added helloworld image auto ingress, checked provisioning state beforehand.

* fixes for sisira's comments

* fix minor typo

* bug fix where commands fail if providing registry creds

* Fixed style issues.

* Updated help and version text.

Co-authored-by: Calvin Chan <[email protected]>
Co-authored-by: Haroon Feisal <[email protected]>
Co-authored-by: Haroon Feisal <[email protected]>
Co-authored-by: Anthony Chu <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
Co-authored-by: Sisira Panchagnula <[email protected]>
…thub action complete, fixed datetime import issue from style fix.
_validate_subscription_registered(cmd, "Microsoft.App")

if not certificate_name and not thumbprint:
raise CLIError('Please specify one of parameters: --certificate-name or --thumbprint')
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should avoid raising CLIError directly as it violates the CLI error-handling guidelines. We should raise one of these error types. In this case, MutuallyExclusiveArgumentError is probably the right one to use

There are still a bunch of CLIErrors being raised in old CLI code, but the core CLI team always comments on it if they see it in a new PR


passed, message = is_hostname_validated(cmd, resource_group_name, name, hostname)
if not passed:
raise CLIError(message or 'Please configure the DNS records before adding the hostname.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

Comment on lines 2499 to 2502
if location and (app["location"] != location):
raise CLIError('Container app {} is not in location {}.'.format(name, location))
if environment and (_get_name(environment) != _get_name(app["properties"]["managedEnvironmentId"])):
raise CLIError('Container app {} is not under environment {}.'.format(name, environment))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

Comment on lines 2516 to 2524
raise CLIError('Please specify one of parameters: --certificate or --thumbprint')
if not environment and not certificate:
raise CLIError('Please specify one of parameters: --certificate or --environment')
if certificate and not is_valid_resource_id(certificate) and not environment:
raise CLIError('Please specify the parameters: --environment')

passed, message = is_hostname_validated(cmd, resource_group_name, name, hostname)
if not passed:
raise CLIError(message or 'Please configure the DNS records before adding the hostname.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

@lil131 lil131 requested a review from StrawnSC May 18, 2022 15:55
# Certificates Commands
helps['containerapp env certificate'] = """
type: group
short-summary: Commands to manage certificates for the Container Apps environment.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The usage of this ACA should be consistent across all help - since it is the name of the service. Looks like other commands call it 'container app' so please check & update all accordingly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For environment level operations, other commands like dapr-component and storage are using "Container Apps environment".
Keep it as is for certificate commands. Hostname commands are using "container app".

c.argument('thumbprint', options_list=['--thumbprint', '-t'], help='Thumbprint of the certificate.')

with self.argument_context('containerapp env certificate delete') as c:
c.argument('name', id_part=None)
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason we set id_part=None for certificates? The user should be able to use the resourceID of the certificate as well right?

@@ -1011,6 +1012,15 @@ def get_randomized_name(prefix, name=None, initial="rg"):
return default


def get_randomized_cert_name(thumbprint, prefix, initial="rg"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this more like generate_randomized_cert_name?

try:
p12 = crypto.load_pkcs12(file_data, cert_password)
except Exception as e:
raise FileOperationError('Failed to load the certificate file. This may be due to an incorrect or missing password. Please double check and try again.\nError: {}'.format(e)) from e
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to use a custom exception message here? Is there a possibility for other type of errors as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding a customized message because the error message from OpenSSL for wrong password is not helpful.
The original error message is also included at the end of the customized message in case it's another type of error.

from OpenSSL import crypto
import os

file_data = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: call this cert_data instead of file_data?

@StrawnSC
Copy link
Collaborator

@lil131 after this is approved, we should be sure to "squash and merge" instead of just merging the pull request, since this has ~80 old commits from the old main branch (instead of just the 5 or so related to certs & domains). This would keep the git history of the PR for the release more clear

if os.path.splitext(file_path)[1] in ['.pem']:
file_data = f.read()
x509 = crypto.load_certificate(crypto.FILETYPE_PEM, file_data)
digest_algorithm = 'sha1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

since sha1 is considered weak cipher, change this to at least sha256 or higher?

except Exception as e:
raise FileOperationError('Failed to load the certificate file. This may be due to an incorrect or missing password. Please double check and try again.\nError: {}'.format(e)) from e
x509 = p12.get_certificate()
digest_algorithm = 'sha1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment on using 'sha1' here & make this a constant since this is repeated.

examples:
- name: List certificates for an environment.
text: |
az containerapp env certificate list -g MyResourceGroup --name MyEnvironment
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't list command take a certificate name or ID as an input as well. Can you add more sample help commands here on usage?

examples:
- name: Add or update a certificate.
text: |
az containerapp env certificate upload -g MyResourceGroup --name MyEnvironment --certificate-file MyFilepath
Copy link
Collaborator

Choose a reason for hiding this comment

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

We allow user to provide their own cert name as well right? please add sample commands as examples here for usage.

examples:
- name: Delete a certificate from the Container Apps environment.
text: |
az containerapp env certificate delete -g MyResourceGroup --name MyEnvironment --certificate-name MyCertificate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given this takes thumbprint as input as well - add those examples as well for usage.

@@ -181,6 +181,14 @@
"tags": None
}

ContainerAppCertificate = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the certificate object Model not have other properties? like thumbprint, name etc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@panchagnula This model is only for the add cert api request. It is consistent with the api requirement.
I'll change the model name to make it more clear.


with self.argument_context('containerapp env certificate list') as c:
c.argument('name', id_part=None)
c.argument('certificate_name', options_list=['--certificate-name', '-c'], help='Name of the certificate which is unique within the Container Apps environment.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

For List - this can be the full cerificate_resource_id as well right? Please update help to clarify this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the CLI linter has a rule no_ids_for_list_commands that prevents using --ids with list commands

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see got it. this is for --ids. Can we validate the command itself behaves correctly / works when a user gives the full certificate resource ID & not just the name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@panchagnula I'll add a test for this.

@@ -262,6 +261,63 @@ def test_containerapp_ingress_traffic_e2e(self, resource_group):
for revision in revisions_list:
self.assertEqual(revision["properties"]["trafficWeight"], 50)

@AllowLargeResponse(8192)
@ResourceGroupPreparer(location="westeurope")
def test_containerapp_custom_domains_e2e(self, resource_group):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot for adding the tests. Some other tests to consider

  1. pem file support
  2. validation test that non pfx, .pem are not supported & throw a valid error message
  3. tests to list, show an existing certificate with name, resourceId, thumbprint.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also since this is an E2E scenarios test can we also add tests here to list (show), delete?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@panchagnula I'll add more tests for those. Thanks!

cert_password = 'test12'
hostname_1 = 'cli.antdomaincerttest.com'
hostname_2 = 'testing.antdomaincerttest.com'
self.cmd('containerapp ssl upload -n {} -g {} --environment {} --hostname {} --certificate-file "{}" --password {}'.format(ca_name, resource_group, env_name, hostname_1, pfx_file, cert_password), checks=[
Copy link
Collaborator

Choose a reason for hiding this comment

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

since in the API password is not required can we add a test case without password as well?

Copy link
Collaborator

@panchagnula panchagnula left a comment

Choose a reason for hiding this comment

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

Please check the comments.

@@ -262,6 +261,63 @@ def test_containerapp_ingress_traffic_e2e(self, resource_group):
for revision in revisions_list:
self.assertEqual(revision["properties"]["trafficWeight"], 50)

@AllowLargeResponse(8192)
@ResourceGroupPreparer(location="westeurope")
def test_containerapp_custom_domains_e2e(self, resource_group):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lil131 can you confirm that the test test_containerapp_custom_domains_e2e passes when you run it live? When I run it with --live it fails with azure.cli.core.azclierror.ValidationError: A TXT record pointing from asuid.cli.antdomaincerttest.com to <thumbprint> was not found.

Copy link
Collaborator Author

@lil131 lil131 May 18, 2022

Choose a reason for hiding this comment

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

@StrawnSC Yes, it passes on my end.
The hostname in this test case is configured for my subscription only. You will need to change its DNS settings or replace with your own hostname. I'll add comments for this in the test case. Thanks!

Copy link
Collaborator

@StrawnSC StrawnSC May 18, 2022

Choose a reason for hiding this comment

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

That might be an issue since the test recordings are run on the CI checks when we open a PR into the upstream repo. Is it possible to make the test more general? Presumably the web app tests in the core CLI found a way around this

Copy link
Collaborator

Choose a reason for hiding this comment

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

@StrawnSC this is a good call out. @lil131 for web apps we do the operation on default hostname (*.azurewebsites.net) so the test passes live or not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@panchagnula Got it. I'll try with the default hostname. Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@panchagnula Tried with the default hostname but still couldn't pass the validation because the txt record is not found.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@panchagnula @StrawnSC Added a new commit. but the test for hostname only covers listing hostnames and a failure case of adding hostname for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks Linlu - lets create an item on our end to figure out scenarios tests for this using ASD if possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@panchagnula Created WI 14496615 to track this.

@StrawnSC
Copy link
Collaborator

This should also update the 0.3.5 entry in the HISTORY.rst file

@lil131 lil131 requested review from panchagnula and StrawnSC May 19, 2022 00:20
Copy link
Collaborator

@panchagnula panchagnula left a comment

Choose a reason for hiding this comment

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

LGTM.

@StrawnSC StrawnSC merged commit 0eca228 into calvinsID:containerapp-0.3.5 May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants