Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/containerapp/azext_containerapp/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@
--image my-app:v1.0 --environment MyContainerappEnv \\
--secrets mysecret=secretvalue1 anothersecret="secret value 2" \\
--secret-volume-mount "mnt/secrets"
- name: Create a container app from a dockerfile in a GitHub repo (setting up github actions)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: "github actions" --> "GitHub Actions"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's remove the "Dockerfile" part of this since our GitHub Action doesn't require the application source to have one. It may be worth thinking about rephrasing this snippet as follows:

"Create a Container App from a new GitHub Actions workflow in the provided repository."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

text: |
az containerapp create -n MyContainerapp -g MyResourceGroup \\
--environment MyContainerappEnv --registry-server MyRegistryServer \\
--registry-user MyRegistryUser --registry-pass MyRegistryPass \\
--repo https://github.com/myAccount/myRepo
- name: Create a container app from a dockerfile in a local directory (or autogenerate a container if no dockerfile is found)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Similar comment as above: I would recommend removing the mention of "Dockerfile" and think about rephrasing this snippet as follows:

"Create a Container App from the provided application source."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

text: |
az containerapp create -n MyContainerapp -g MyResourceGroup \\
--environment MyContainerappEnv --registry-server MyRegistryServer \\
--registry-user MyRegistryUser --registry-pass MyRegistryPass \\
--source .
"""

helps['containerapp update'] = """
Expand Down
10 changes: 10 additions & 0 deletions src/containerapp/azext_containerapp/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ def load_arguments(self, _):
c.argument('workload_profile_name', options_list=['--workload-profile-name', '-w'], help="Name of the workload profile to run the app on.", is_preview=True)
c.argument('secret_volume_mount', help="Path to mount all secrets e.g. mnt/secrets", is_preview=True)
c.argument('termination_grace_period', type=int, options_list=['--termination-grace-period', '--tgp'], help="Duration in seconds a replica is given to gracefully shut down before it is forcefully terminated. (Default: 30)", is_preview=True)
c.argument('source', help="Local directory path containing the application source and Dockerfile for building the container image. Preview: If no Dockerfile is present, a container image is generated using buildpacks. If Docker is not running or buildpacks cannot be used, Oryx will be used to generate the image. See the supported Oryx runtimes here: https://github.com/microsoft/Oryx/blob/main/doc/supportedRuntimeVersions.md.")

Copy link
Copy Markdown
Contributor

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=True

Copy link
Copy Markdown
Contributor Author

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.


with self.argument_context('containerapp create', arg_group='Identity') as c:
c.argument('user_assigned', nargs='+', help="Space-separated user identities to be assigned.")
Expand All @@ -135,6 +136,15 @@ def load_arguments(self, _):
c.argument('service_type', help="The service information for dev services.")
c.ignore('service_type')

with self.argument_context('containerapp create', arg_group='Github Repo') as c:
c.argument('repo', help='Create an app via Github Actions. In the format: https://github.com/<owner>/<repository-name> or <owner>/<repository-name>')
c.argument('token', help='A Personal Access Token with write access to the specified repository. For more information: https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line. If not provided or not found in the cache (and using --repo), a browser page will be opened to authenticate with Github.')
c.argument('branch', options_list=['--branch', '-b'], help='The branch of the Github repo. Assumed to be the Github repo\'s default branch if not specified.')
c.argument('context_path', help='Path in the repo from which to run the docker build. Defaults to "./". Dockerfile is assumed to be named "Dockerfile" and in this directory.')
c.argument('service_principal_client_id', help='The service principal client ID. Used by Github Actions to authenticate with Azure.', options_list=["--service-principal-client-id", "--sp-cid"])
c.argument('service_principal_client_secret', help='The service principal client secret. Used by Github Actions to authenticate with Azure.', options_list=["--service-principal-client-secret", "--sp-sec"])
c.argument('service_principal_tenant_id', help='The service principal tenant ID. Used by Github Actions to authenticate with Azure.', options_list=["--service-principal-tenant-id", "--sp-tid"])

with self.argument_context('containerapp show') as c:
c.argument('show_secrets', help="Show Containerapp secrets.", action='store_true')

Expand Down
16 changes: 16 additions & 0 deletions src/containerapp/azext_containerapp/_up_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,22 @@ def _validate_up_args(cmd, source, image, repo, registry_server):
raise ValidationError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} "
"characters when using --repo")

def _validate_create_args(cmd, source, repo, registry_server, registry_user, registry_pass):
if source and repo:
raise MutuallyExclusiveArgumentError(
"Cannot use --source and --repo togther. "
"Can either deploy from a local directory or a Github repo"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: "Github repo" --> "GitHub repository"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

)
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)
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")


def _reformat_image(source, repo, image):
if source and (image or repo):
Expand Down
43 changes: 40 additions & 3 deletions src/containerapp/azext_containerapp/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ def create_containerapp(cmd,
resource_group_name,
yaml=None,
image=None,
source=None,
container_name=None,
managed_env=None,
min_replicas=None,
Expand Down Expand Up @@ -448,11 +449,22 @@ def create_containerapp(cmd,
registry_identity=None,
workload_profile_name=None,
termination_grace_period=None,
secret_volume_mount=None):
secret_volume_mount=None,
repo=None,
token=None,
branch=None,
context_path=None,
service_principal_client_id=None,
service_principal_client_secret=None,
service_principal_tenant_id=None):
from ._up_utils import (_validate_create_args,_reformat_image,_get_dockerfile_content, _get_ingress_and_target_port,ContainerApp,ResourceGroup,ContainerAppEnvironment, _get_registry_details, _create_github_action,
get_token, _has_dockerfile)
from ._github_oauth import cache_github_token
register_provider_if_needed(cmd, CONTAINER_APPS_RP)
validate_container_app_name(name, AppType.ContainerApp.name)
validate_create(registry_identity, registry_pass, registry_user, registry_server, no_wait)
validate_revision_suffix(revision_suffix)
_validate_create_args(cmd, source, repo, registry_server, registry_user, registry_pass)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please rebase newest code and move following validate logic to ContainerAppCreateDecorator.validate_arguments

_validate_create_args(cmd, source, repo, registry_server, registry_user, registry_pass)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Merged latest changes from main


if registry_identity and not is_registry_msi_system(registry_identity):
logger.info("Creating an acrpull role assignment for the registry identity")
Expand All @@ -465,9 +477,12 @@ def create_containerapp(cmd,
startup_command or args or tags:
not disable_warnings and logger.warning('Additional flags were passed along with --yaml. These flags will be ignored, and the configuration defined in the yaml will be used instead')
return create_containerapp_yaml(cmd=cmd, name=name, resource_group_name=resource_group_name, file_name=yaml, no_wait=no_wait)

imageNotProvided = False
if not image:
imageNotProvided = True
image = HELLO_WORLD_IMAGE
token = get_token(cmd, repo, token)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: can we move this line down to where it's used first? Preferably around the if-statement on 716

dockerfile = "Dockerfile"

if managed_env is None:
raise RequiredArgumentMissingError('Usage error: --environment is required if not using --yaml')
Expand Down Expand Up @@ -690,6 +705,24 @@ def create_containerapp(cmd,
else:
set_managed_identity(cmd, resource_group_name, containerapp_def, user_assigned=[registry_identity])
try:
image = None if imageNotProvided else _reformat_image(source,repo,image)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I understand the relationship between imageNotProvided and image if we set them to True and HELLO_WORLD_IMAGE, respectively, at the beginning of this function, but then revert image back to None if imageNotProvided is True. Would you mind elaborating on the purpose of this line?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed this line to 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())

Since image is defaulted to mcr.microsoft.com/k8se/quickstart:latest if it was not provided by the user, we need to reassign it to None so the final image pushed to the registry is in the format <registry-server>/<containerapp-name>:tag

resource_group = ResourceGroup(cmd, name=resource_group_name, location=location)
env = ContainerAppEnvironment(cmd, managed_env, resource_group, location=location)
app = ContainerApp(cmd, name, resource_group, None, image, env, target_port, registry_server, registry_user, registry_pass, env_vars, workload_profile_name, ingress)

if source or repo:
_get_registry_details(cmd,app,source) # fetch ACR creds from arguments registry arguments

if source and not _has_dockerfile(source, dockerfile):
pass
else:
dockerfile_content = _get_dockerfile_content(repo, branch, token, source, context_path, dockerfile)
ingress, target_port = _get_ingress_and_target_port(ingress, target_port, dockerfile_content)

app.create_acr_if_needed()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we able to remove this? Even if it doesn't create the ACR instance in any of the flows provided by this function, if the implementation changes under-the-hood, it could start causing some unexpected behavior when we don't want the ACR instance to be created in any circumstance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed this line. Since it is now required to set registry-server, registry-user and registry-pass args, the method create_acr_if_needed() is not longer required

if source:
app.run_acr_build(dockerfile, source, quiet=False, build_from_source=not _has_dockerfile(source, dockerfile))
container_def["image"] = app.image
r = ContainerAppClient.create_or_update(
cmd=cmd, resource_group_name=resource_group_name, name=name, container_app_envelope=containerapp_def, no_wait=no_wait)

Expand Down Expand Up @@ -731,7 +764,11 @@ def create_containerapp(cmd,
linker_client.linker.begin_create_or_update(resource_uri=r["id"],
parameters=item["parameters"],
linker_name=item["linker_name"]).result()

if repo:
_create_github_action(app, env, service_principal_client_id, service_principal_client_secret,
service_principal_tenant_id, branch, token, repo, context_path)
cache_github_token(cmd, token, repo)
r = ContainerAppClient.show(cmd=cmd, resource_group_name=resource_group_name, name=name)
return r
except Exception as e:
handle_raw_exception(e)
Expand Down