-
Notifications
You must be signed in to change notification settings - Fork 1.6k
containerapp - Add --source and --repo to containerapp create #6463
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
Changes from 10 commits
887ecf6
011439a
e9fd59b
89241ca
916259c
1439120
fb7f4fe
d18d6c7
78181aa
d646bd1
56e68d4
1b70cf5
3a809ca
07e1be5
25862fb
0b2098b
3fd9fde
557f05e
c2fb2d6
ef8c6df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,7 +60,6 @@ | |
| LOG_ANALYTICS_RP, | ||
| CONTAINER_APPS_RP, | ||
| ACR_IMAGE_SUFFIX, | ||
| MAXIMUM_CONTAINER_APP_NAME_LENGTH, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: intended change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, |
||
| ACR_TASK_TEMPLATE, | ||
| DEFAULT_PORT) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,21 +6,33 @@ | |
|
|
||
| import re | ||
| from azure.cli.core.azclierror import (ValidationError, ResourceNotFoundError, InvalidArgumentValueError, | ||
| MutuallyExclusiveArgumentError) | ||
| MutuallyExclusiveArgumentError, RequiredArgumentMissingError) | ||
| from msrestazure.tools import is_valid_resource_id | ||
| from knack.log import get_logger | ||
|
|
||
| from ._clients import ContainerAppClient | ||
| from ._ssh_utils import ping_container_app | ||
| from ._utils import safe_get, is_registry_msi_system | ||
| from ._constants import ACR_IMAGE_SUFFIX, LOG_TYPE_SYSTEM | ||
| from ._constants import ACR_IMAGE_SUFFIX, LOG_TYPE_SYSTEM, MAXIMUM_SECRET_LENGTH | ||
| from urllib.parse import urlparse | ||
|
|
||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
| # called directly from custom method bc otherwise it disrupts the --environment auto RID functionality | ||
| def validate_create(registry_identity, registry_pass, registry_user, registry_server, no_wait): | ||
| def validate_create(registry_identity, registry_pass, registry_user, registry_server, no_wait, source=None, repo=None): | ||
| if source and repo: | ||
| raise MutuallyExclusiveArgumentError("Cannot use --source and --repo together. ""Can either deploy from a local directory or a Github repository") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: intended
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great catch! Not intended. |
||
| if source or repo: | ||
| if not registry_server or not registry_user or not registry_pass: | ||
| raise RequiredArgumentMissingError('Usage error: --registry-server, --registry-username and --registry-password are required while using --source or --repo.') | ||
| if repo and registry_server and "azurecr.io" in registry_server: | ||
| parsed = urlparse(registry_server) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cormacpayne we should check this condition for source also?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @snehapar9 we should since we have the condition above to validate that if source or repo:
if not registry_server or not registry_user or not registry_pass:
raise RequiredArgumentMissingError('Usage error: --registry-server, --registry-username and --registry-password are required while using --source or --repo.')
if "azurecr.io" in registry_server:
parsed = urlparse(registry_server)
registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0]
if registry_name and len(registry_name) > MAXIMUM_SECRET_LENGTH:
raise ValidationError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} "
"characters when using --repo")``` |
||
| registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0] | ||
| if registry_name and len(registry_name) > MAXIMUM_SECRET_LENGTH: | ||
| raise ValidationError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} " | ||
| "characters when using --repo") | ||
| if registry_identity and (registry_pass or registry_user): | ||
| raise MutuallyExclusiveArgumentError("Cannot provide both registry identity and username/password") | ||
| if is_registry_msi_system(registry_identity) and no_wait: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,8 @@ | |
| from ._constants import (CONTAINER_APPS_RP, | ||
| HELLO_WORLD_IMAGE) | ||
|
|
||
| from ._github_oauth import cache_github_token | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
|
|
@@ -152,6 +154,30 @@ def get_argument_no_wait(self): | |
| def get_argument_yaml(self): | ||
| return self.get_param("yaml") | ||
|
|
||
| def get_argument_source(self): | ||
| return self.get_param("source") | ||
|
|
||
| def get_argument_repo(self): | ||
| return self.get_param("repo") | ||
|
|
||
| def get_argument_branch(self): | ||
| return self.get_param("branch") | ||
|
|
||
| def get_argument_token(self): | ||
| return self.get_param("token") | ||
|
|
||
| def get_argument_context_path(self): | ||
| return self.get_param("context_path") | ||
|
|
||
| def get_argument_service_principal_client_id(self): | ||
| return self.get_param("service_principal_client_id") | ||
|
|
||
| def get_argument_service_principal_client_secret(self): | ||
| return self.get_param("service_principal_client_secret") | ||
|
|
||
| def get_argument_service_principal_tenant_id(self): | ||
| return self.get_param("service_principal_tenant_id") | ||
|
|
||
| def get_argument_image(self): | ||
| return self.get_param("image") | ||
|
|
||
|
|
@@ -314,7 +340,7 @@ def __init__( | |
|
|
||
| def validate_arguments(self): | ||
| validate_container_app_name(self.get_argument_name(), AppType.ContainerApp.name) | ||
| validate_create(self.get_argument_registry_identity(), self.get_argument_registry_pass(), self.get_argument_registry_user(), self.get_argument_registry_server(), self.get_argument_no_wait()) | ||
| validate_create(self.get_argument_registry_identity(), self.get_argument_registry_pass(), self.get_argument_registry_user(), self.get_argument_registry_server(), self.get_argument_no_wait(), self.get_argument_source(), self.get_argument_repo()) | ||
| validate_revision_suffix(self.get_argument_revision_suffix()) | ||
|
|
||
| def construct_containerapp(self): | ||
|
|
@@ -524,6 +550,10 @@ def construct_containerapp(self): | |
| set_managed_identity(self.cmd, self.get_argument_resource_group_name(), containerapp_def, system_assigned=True) | ||
| else: | ||
| set_managed_identity(self.cmd, self.get_argument_resource_group_name(), containerapp_def, user_assigned=[self.get_argument_registry_identity()]) | ||
|
|
||
| if self.get_argument_source(): | ||
| app = self.set_up_create_containerapp_if_source_or_repo(containerapp_def=containerapp_def) | ||
| containerapp_def = self.set_up_create_containerapp_source(app=app, containerapp_def=containerapp_def) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need two steps to construct container app payload? Is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. app is an instance of ContainerApp and is required in the |
||
| return containerapp_def | ||
|
|
||
| def create_containerapp(self, containerapp_def): | ||
|
|
@@ -553,7 +583,7 @@ def construct_containerapp_for_post_process(self, containerapp_def, r): | |
| registries_def["server"] = self.get_argument_registry_server() | ||
| registries_def["identity"] = self.get_argument_registry_identity() | ||
| safe_set(containerapp_def, "properties", "configuration", "registries", value=[registries_def]) | ||
| return containerapp_def | ||
| return containerapp_def | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: stylistically this looks correct, but just want to ensure this change was intended
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change was not intended. |
||
|
|
||
| def post_process_containerapp(self, containerapp_def, r): | ||
| if is_registry_msi_system(self.get_argument_registry_identity()): | ||
|
|
@@ -580,6 +610,62 @@ def post_process_containerapp(self, containerapp_def, r): | |
| linker_client.linker.begin_create_or_update(resource_uri=r["id"], | ||
| parameters=item["parameters"], | ||
| linker_name=item["linker_name"]).result() | ||
|
|
||
| if self.get_argument_repo(): | ||
| app = self.set_up_create_containerapp_if_source_or_repo(containerapp_def=containerapp_def) | ||
| r = self.set_up_create_containerapp_repo(app=app, r=r, env=app.env, env_rg=app.resource_group.name) | ||
| return r | ||
|
|
||
| def set_up_create_containerapp_if_source_or_repo(self, containerapp_def): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, I have refactored the containerapp decorator with this pr: containerapp refactor decorator by Greedygre · Pull Request #6511 · Azure/azure-cli-extensions (github.com) |
||
| from ._up_utils import (ContainerApp, ResourceGroup, ContainerAppEnvironment, _reformat_image) | ||
|
|
||
| # Parse resource group name and managed env name | ||
| env_id = containerapp_def["properties"]['environmentId'] | ||
| parsed_managed_env = parse_resource_id(env_id) | ||
| env_name = parsed_managed_env['name'] | ||
| env_rg = parsed_managed_env['resource_group'] | ||
|
|
||
| # Parse location | ||
| env_info = self.get_environment_client().show(cmd=self.cmd, resource_group_name=env_rg, name=env_name) | ||
| location = env_info['location'] | ||
|
|
||
| # Set image to None if it was previously set to the default image (case where image was not provided by the user) else reformat it | ||
| image = None if self.get_argument_image().__eq__(HELLO_WORLD_IMAGE) else _reformat_image(self.get_argument_source(), self.get_argument_repo(), self.get_argument_image()) | ||
|
|
||
| # Construct ContainerApp | ||
| resource_group = ResourceGroup(self.cmd, env_rg, location=location) | ||
| env = ContainerAppEnvironment(self.cmd, env_name, resource_group, location=location) | ||
| app = ContainerApp(self.cmd, self.get_argument_name(), resource_group, None, image, env, self.get_argument_target_port(), self.get_argument_registry_server(), self.get_argument_registry_user(), self.get_argument_registry_pass(), self.get_argument_env_vars(), self.get_argument_workload_profile_name(), self.get_argument_ingress()) | ||
|
|
||
| return app | ||
|
|
||
| def set_up_create_containerapp_source(self, app, containerapp_def): | ||
| from ._up_utils import (_get_registry_details, get_token, _has_dockerfile, _get_dockerfile_content, _get_ingress_and_target_port) | ||
| dockerfile = "Dockerfile" | ||
| token = get_token(self.cmd, self.get_argument_repo(), self.get_argument_token()) | ||
| _get_registry_details(self.cmd, app, self.get_argument_source()) # fetch ACR creds from arguments registry arguments | ||
|
|
||
| if self.get_argument_source() and not _has_dockerfile(self.get_argument_source(), dockerfile): | ||
| pass | ||
| else: | ||
| dockerfile_content = _get_dockerfile_content(self.get_argument_repo(), self.get_argument_branch(), token, self.get_argument_source(), self.get_argument_context_path(), dockerfile) | ||
| ingress, target_port = _get_ingress_and_target_port(self.get_argument_ingress(), self.get_argument_target_port(), dockerfile_content) | ||
|
|
||
| # Uses buildpacks to generate image if Dockerfile was not provided by the user | ||
| app.run_acr_build(dockerfile, self.get_argument_source(), quiet=False, build_from_source=not _has_dockerfile(self.get_argument_source(), dockerfile)) | ||
|
|
||
| # Update image | ||
| containerapp_def["properties"]["template"]["containers"][0]["image"] = HELLO_WORLD_IMAGE if app.image is None else app.image | ||
| return containerapp_def | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. with newest code, use |
||
|
|
||
| def set_up_create_containerapp_repo(self, app, r, env, env_rg): | ||
| from ._up_utils import (_create_github_action, get_token) | ||
| # Get GitHub access token | ||
| token = get_token(self.cmd, self.get_argument_repo(), self.get_argument_token()) | ||
| _create_github_action(app, env, self.get_argument_service_principal_client_id(), self.get_argument_service_principal_client_secret(), | ||
| self.get_argument_service_principal_tenant_id(), self.get_argument_branch(), token, self.get_argument_repo(), self.get_argument_context_path()) | ||
| cache_github_token(self.cmd, token, self.get_argument_repo()) | ||
| r = self.client.show(cmd=self.cmd, resource_group_name=env_rg, name=app.name) | ||
| return r | ||
|
|
||
| def set_up_create_containerapp_yaml(self, name, file_name): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the argument is in preview, add
is_preview=TrueThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These flags are not in preview.