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
Merged
Show file tree
Hide file tree
Changes from 84 commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
1de51ee
Marchp1s and add back Identity (#57)
runefa Apr 7, 2022
7735595
Fix help for linter
calvinsID Apr 9, 2022
2e1be89
various fixes, helptext (#59)
calvinsID Apr 11, 2022
5fcd785
Fixes (#60)
calvinsID Apr 11, 2022
a1929c8
Updated managed identity + help. (#61)
runefa Apr 11, 2022
df5cc99
Added user-assigned and system-assigned to containerapp create. (#62)
runefa Apr 11, 2022
0dedf19
Bump version to 0.1.1 (#63)
calvinsID Apr 11, 2022
1474d69
Added more specific MSI help text. (#64)
runefa Apr 11, 2022
5afc2b1
Bump to 0.3.0 (#65)
calvinsID Apr 11, 2022
77dcaa0
Merge branch 'main' into calcha-main
calvinsID Apr 12, 2022
5cc0aa8
Container App Test suite (#67)
calvinsID Apr 15, 2022
1f1b31a
use new GH actions API
StrawnSC Apr 17, 2022
7f70e2f
remove live only recordings
StrawnSC Apr 17, 2022
fa539a8
update CODEOWNERS
StrawnSC Apr 17, 2022
f935df8
fix API version naming
StrawnSC Apr 18, 2022
7da2e9a
Merge branch 'main' into main
StrawnSC Apr 18, 2022
fcc3d87
Managed Identity Tests (#69)
runefa Apr 18, 2022
cb6820e
resolve review comments
StrawnSC Apr 18, 2022
f6efbd2
Managed Identity Fixes (#71)
runefa Apr 18, 2022
4e805bf
Update src/containerapp/azext_containerapp/_params.py
panchagnula Apr 19, 2022
7f7f1fe
4/26 release: Up with --repo/--browse, exec (ssh) command, replica co…
StrawnSC Apr 22, 2022
3d794b3
Merge branch 'main' into main
runefa Apr 22, 2022
e87accc
Fixed small issue with test.
Apr 22, 2022
552850f
Removed flake exclusions and removed type=str from params.
Apr 25, 2022
b86d217
Fixed repo bug when searching for dockerfile, increased timeout on gi…
Apr 25, 2022
31002f8
Added env var changes.
Apr 25, 2022
c292ac9
Assume port if ingress is provided with image and port is not.
Apr 25, 2022
b9fce25
Fixed small helloworld error.
Apr 25, 2022
077bf20
Fixed logger typo.
Apr 25, 2022
c100f1f
Search for acr before creating one.
Apr 26, 2022
44faaf7
Fixed bug where only --environment is passed. Changed hash on acr nam…
Apr 26, 2022
197913a
error out if dockerfile not found (--repo)
StrawnSC Apr 26, 2022
595cdf5
Fixed bug with --image. Changed logger warning output. Disabled warni…
Apr 26, 2022
272146b
Disabled no_wait. Added better error handling for up API calls. Updat…
Apr 26, 2022
988d901
fix ACR length cap; enforce name/secret limits; trigger GH action if …
StrawnSC Apr 26, 2022
e935bf2
Merge branch 'main' into findacr2
StrawnSC Apr 26, 2022
50617e1
Merge pull request #77 from haroonf/findacr2
StrawnSC Apr 26, 2022
757dcf4
force exact match for ACR retrieval (prevents secrets issues for --repo)
StrawnSC Apr 27, 2022
cbeac7a
fix hashing and add GH validations
StrawnSC Apr 27, 2022
44aa7ff
don't retrieve a registry if one provided; take RG from env if possible
StrawnSC Apr 27, 2022
a016f35
Fixed --registry-server with --image bug. (#78)
runefa Apr 27, 2022
c59d257
use SP creds if provided
StrawnSC Apr 27, 2022
bd2cc8b
Merge branch 'main' of github.com:calvinsid/azure-cli-extensions into…
StrawnSC Apr 27, 2022
2b58d27
fix github actions (less polling)
StrawnSC Apr 27, 2022
668b7d9
Added prototype for env check.
Apr 27, 2022
25843f1
Honor location and environment passed to create new containerapp (eve…
runefa Apr 27, 2022
612d1a5
print created SP name/id; prevent using ACR names longer than 20 char…
StrawnSC Apr 27, 2022
900e7b6
fix style; add license header
StrawnSC Apr 27, 2022
6710636
Finished core logic.
Apr 28, 2022
50e0754
add max core cli version 2.36.0
StrawnSC Apr 28, 2022
d53550b
make ACR name more unique (must be globally unique)
StrawnSC Apr 28, 2022
66878c7
Finished logic.
Apr 28, 2022
37627de
sort workflows by date before selecting one
StrawnSC Apr 28, 2022
5f5308e
log workflow
StrawnSC Apr 29, 2022
7b0bd7d
Added error message with eligible locations if users pass uneligible …
Apr 29, 2022
c534455
Added function to check if env already exists so we don't try to upda…
Apr 29, 2022
3e576de
Added error handling for location northcentralusstage. Added list of …
Apr 29, 2022
724249a
merge main
StrawnSC Apr 29, 2022
a1ce4dd
Small fixes, implemented check_env_name_on_rg.
Apr 29, 2022
4fc7267
Merge pull request #80 from haroonf/managedenvcheck
StrawnSC May 2, 2022
a8a6ac1
location bug fix
StrawnSC May 2, 2022
5570ad9
fix style
StrawnSC May 2, 2022
2ccbce0
bump version number
StrawnSC May 2, 2022
34aabca
Updates to tests (#82)
calvinsID May 3, 2022
6b3468a
Remove recordings (#83)
calvinsID May 3, 2022
53ddc45
prevent using --only-show-errors, --output, -o in up
StrawnSC May 3, 2022
711bbd4
Added FileShare commands. (#84)
runefa May 4, 2022
65cb30c
Fixed bug. (#86)
runefa May 4, 2022
13f217a
register log analytics resource provider if not registered when creat…
StrawnSC May 4, 2022
e86e089
fix style
StrawnSC May 4, 2022
8960d75
Fixed linter issue.
May 4, 2022
a8737a0
fix linter issues
StrawnSC May 5, 2022
1165dc9
update history file
StrawnSC May 5, 2022
a4590f4
Moved constant to constants.py.
May 5, 2022
1b8c7e0
add timeout to container app ping for ssh/logstream
StrawnSC May 5, 2022
40ca5b9
Various tests (Ingress, Traffic, Dapr, Env) (#87)
runefa May 5, 2022
4e7c369
Revert "Various tests (Ingress, Traffic, Dapr, Env) (#87)" (#88)
runefa May 5, 2022
0b15002
Reverted fileshare. (#90)
runefa May 5, 2022
34eab57
Tests (dapr-components, env, ingress, traffic) (#89)
runefa May 6, 2022
0a32abd
merge Azure/main->calvinsid/main
StrawnSC May 6, 2022
af7f143
adding certs cmd & test
lil131 May 13, 2022
7c6b49e
adding hostname cmds & test
lil131 May 17, 2022
61db16e
resolve conflicts
lil131 May 17, 2022
75d1323
changes based on comments
lil131 May 18, 2022
2e6f013
changes based on comments
lil131 May 19, 2022
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
104 changes: 104 additions & 0 deletions src/containerapp/azext_containerapp/_clients.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,22 @@ def get_auth_token(cls, cmd, resource_group_name, name):
r = send_raw_request(cmd.cli_ctx, "POST", request_url)
return r.json()

@classmethod
def validate_domain(cls, cmd, resource_group_name, name, hostname):
management_hostname = cmd.cli_ctx.cloud.endpoints.resource_manager
sub_id = get_subscription_id(cmd.cli_ctx)
url_fmt = "{}/subscriptions/{}/resourceGroups/{}/providers/Microsoft.App/containerApps/{}/listCustomHostNameAnalysis?api-version={}&customHostname={}"
request_url = url_fmt.format(
management_hostname.strip('/'),
sub_id,
resource_group_name,
name,
STABLE_API_VERSION,
hostname)

r = send_raw_request(cmd.cli_ctx, "POST", request_url)
return r.json()


class ManagedEnvironmentClient():
@classmethod
Expand Down Expand Up @@ -584,6 +600,94 @@ def list_by_resource_group(cls, cmd, resource_group_name, formatter=lambda x: x)

return env_list

@classmethod
def show_certificate(cls, cmd, resource_group_name, name, certificate_name):
management_hostname = cmd.cli_ctx.cloud.endpoints.resource_manager
api_version = STABLE_API_VERSION
sub_id = get_subscription_id(cmd.cli_ctx)
url_fmt = "{}/subscriptions/{}/resourceGroups/{}/providers/Microsoft.App/managedEnvironments/{}/certificates/{}?api-version={}"
request_url = url_fmt.format(
management_hostname.strip('/'),
sub_id,
resource_group_name,
name,
certificate_name,
api_version)

r = send_raw_request(cmd.cli_ctx, "GET", request_url, body=None)
return r.json()

@classmethod
def list_certificates(cls, cmd, resource_group_name, name, formatter=lambda x: x):
certs_list = []

management_hostname = cmd.cli_ctx.cloud.endpoints.resource_manager
api_version = STABLE_API_VERSION
sub_id = get_subscription_id(cmd.cli_ctx)
url_fmt = "{}/subscriptions/{}/resourceGroups/{}/providers/Microsoft.App/managedEnvironments/{}/certificates?api-version={}"
request_url = url_fmt.format(
management_hostname.strip('/'),
sub_id,
resource_group_name,
name,
api_version)

r = send_raw_request(cmd.cli_ctx, "GET", request_url, body=None)
j = r.json()
for cert in j["value"]:
formatted = formatter(cert)
certs_list.append(formatted)
return certs_list

@classmethod
def create_or_update_certificate(cls, cmd, resource_group_name, name, certificate_name, certificate):
management_hostname = cmd.cli_ctx.cloud.endpoints.resource_manager
api_version = STABLE_API_VERSION
sub_id = get_subscription_id(cmd.cli_ctx)
url_fmt = "{}/subscriptions/{}/resourceGroups/{}/providers/Microsoft.App/managedEnvironments/{}/certificates/{}?api-version={}"
request_url = url_fmt.format(
management_hostname.strip('/'),
sub_id,
resource_group_name,
name,
certificate_name,
api_version)

r = send_raw_request(cmd.cli_ctx, "PUT", request_url, body=json.dumps(certificate))
return r.json()

@classmethod
def delete_certificate(cls, cmd, resource_group_name, name, certificate_name):
management_hostname = cmd.cli_ctx.cloud.endpoints.resource_manager
api_version = STABLE_API_VERSION
sub_id = get_subscription_id(cmd.cli_ctx)
url_fmt = "{}/subscriptions/{}/resourceGroups/{}/providers/Microsoft.App/managedEnvironments/{}/certificates/{}?api-version={}"
request_url = url_fmt.format(
management_hostname.strip('/'),
StrawnSC marked this conversation as resolved.
Show resolved Hide resolved
sub_id,
resource_group_name,
name,
certificate_name,
api_version)

return send_raw_request(cmd.cli_ctx, "DELETE", request_url, body=None)

@classmethod
def check_name_availability(cls, cmd, resource_group_name, name, name_availability_request):
management_hostname = cmd.cli_ctx.cloud.endpoints.resource_manager
api_version = STABLE_API_VERSION
sub_id = get_subscription_id(cmd.cli_ctx)
url_fmt = "{}/subscriptions/{}/resourceGroups/{}/providers/Microsoft.App/managedEnvironments/{}/checkNameAvailability?api-version={}"
request_url = url_fmt.format(
StrawnSC marked this conversation as resolved.
Show resolved Hide resolved
management_hostname.strip('/'),
sub_id,
resource_group_name,
name,
api_version)

r = send_raw_request(cmd.cli_ctx, "POST", request_url, body=json.dumps(name_availability_request))
return r.json()


class GitHubActionClient():
@classmethod
Expand Down
2 changes: 2 additions & 0 deletions src/containerapp/azext_containerapp/_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@
LOG_ANALYTICS_RP = "Microsoft.OperationalInsights"

MAX_ENV_PER_LOCATION = 2

CHECK_CERTIFICATE_NAME_AVAILABILITY_TYPE = "Microsoft.App/managedEnvironments/certificates"
75 changes: 75 additions & 0 deletions src/containerapp/azext_containerapp/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,38 @@
az containerapp env storage remove -g MyResourceGroup --storage-name MyStorageName -n MyEnvironment
"""

# 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".

"""

helps['containerapp env certificate list'] = """
type: command
short-summary: List certificates for an environment.
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?

"""

helps['containerapp env certificate upload'] = """
type: command
short-summary: Add or update a certificate.
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.

"""

helps['containerapp env certificate delete'] = """
type: command
short-summary: Delete a certificate from the Container Apps environment.
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.

"""

# Identity Commands
helps['containerapp identity'] = """
Expand Down Expand Up @@ -714,3 +746,46 @@
text: |
az containerapp dapr disable -n MyContainerapp -g MyResourceGroup
"""

# custom domain Commands
helps['containerapp ssl'] = """
type: group
short-summary: Upload certificate to a managed environment, add hostname to an app in that environment, and bind the certificate to the hostname
"""

helps['containerapp ssl upload'] = """
type: command
short-summary: Upload certificate to a managed environment, add hostname to an app in that environment, and bind the certificate to the hostname
"""

helps['containerapp hostname'] = """
type: group
short-summary: Commands to manage hostnames of a container app.
"""

helps['containerapp hostname bind'] = """
type: command
short-summary: Add or update the hostname and binding with an existing certificate.
examples:
- name: Add or update hostname and binding.
text: |
az containerapp hostname bind -n MyContainerapp -g MyResourceGroup --hostname MyHostname --certificate MyCertificateId
"""

helps['containerapp hostname delete'] = """
type: command
short-summary: Delete hostnames from a container app.
examples:
- name: Delete secrets from a container app.
text: |
az containerapp hostname delete -n MyContainerapp -g MyResourceGroup --hostname MyHostname
"""

helps['containerapp hostname list'] = """
type: command
short-summary: List the hostnames of a container app.
examples:
- name: List the hostnames of a container app.
text: |
az containerapp hostname list -n MyContainerapp -g MyResourceGroup
"""
24 changes: 24 additions & 0 deletions src/containerapp/azext_containerapp/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

"location": None,
"properties": {
"password": None,
"value": None
}
}

DaprComponent = {
"properties": {
"componentType": None, # String
Expand Down Expand Up @@ -232,6 +240,22 @@
"subscriptionId": None # str
}

ContainerAppCustomDomainEnvelope = {
"properties": {
"configuration": {
"ingress": {
"customDomains": None
}
}
}
}

ContainerAppCustomDomain = {
"name": None,
"bindingType": "SniEnabled",
"certificateId": None
}

AzureFileProperties = {
"accountName": None,
"accountKey": None,
Expand Down
38 changes: 38 additions & 0 deletions src/containerapp/azext_containerapp/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,22 @@ def load_arguments(self, _):
with self.argument_context('containerapp env show') as c:
c.argument('name', name_type, help='Name of the Container Apps Environment.')

with self.argument_context('containerapp env certificate upload') as c:
c.argument('name', id_part=None)
c.argument('certificate_file', options_list=['--certificate-file', '-f'], help='The filepath of the .pfx or .pem file')
c.argument('certificate_name', options_list=['--certificate-name', '-c'], help='Name of the certificate which should be unique within the Container Apps environment.')
c.argument('certificate_password', options_list=['--password', '-p'], help='The certificate file password')

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.

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?

c.argument('certificate_name', options_list=['--certificate-name', '-c'], help='Name of the certificate which is unique within the Container Apps environment.')
c.argument('thumbprint', options_list=['--thumbprint', '-t'], help='Thumbprint of the certificate.')

with self.argument_context('containerapp env storage') as c:
c.argument('name', id_part=None)
c.argument('storage_name', help="Name of the storage.")
Expand Down Expand Up @@ -257,3 +273,25 @@ def load_arguments(self, _):
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 ssl upload') as c:
c.argument('name', id_part=None)
c.argument('hostname', help='The custom domain name.')
c.argument('environment', options_list=['--environment', '-e'], help='Name or resource id of the Container App environment.')
c.argument('certificate_file', options_list=['--certificate-file', '-f'], help='The filepath of the .pfx or .pem file')
c.argument('certificate_password', options_list=['--password', '-p'], help='The certificate file password')
c.argument('certificate_name', options_list=['--certificate-name', '-c'], help='Name of the certificate which should be unique within the Container Apps environment.')

with self.argument_context('containerapp hostname bind') as c:
c.argument('name', id_part=None)
c.argument('hostname', help='The custom domain name.')
c.argument('thumbprint', options_list=['--thumbprint', '-t'], help='Thumbprint of the certificate.')
c.argument('certificate', options_list=['--certificate', '-c'], help='Name or resource id of the certificate.')
c.argument('environment', options_list=['--environment', '-e'], help='Name or resource id of the Container App environment.')

with self.argument_context('containerapp hostname list') as c:
c.argument('name', id_part=None)

with self.argument_context('containerapp hostname delete') as c:
c.argument('name', id_part=None)
c.argument('hostname', help='The custom domain name.')
Loading