Look Up Application ID Instead of Parsing It

Closes amazon-archives#34

Author: kolanos

.pylintrc

@@ -62,7 +62,8 @@ confidence=
 # --disable=W".
 disable=useless-object-inheritance,
         unused-argument,
-        too-many-instance-attributes
+        too-many-instance-attributes,
+        bad-continuation
 
 # Enable the message, report, category or checker with the given id(s). You can
 # either give multiple identifier separated by comma (,) or put this option

serverlessrepo/exceptions.py

@@ -4,7 +4,7 @@
 class ServerlessRepoError(Exception):
     """Base exception raised by serverlessrepo library."""
 
-    MESSAGE = ''
+    MESSAGE = ""
 
     def __init__(self, **kwargs):
         """Init the exception object."""
@@ -32,11 +32,13 @@ class InvalidApplicationPolicyError(ServerlessRepoError):
 class S3PermissionsRequired(ServerlessRepoError):
     """Raised when S3 bucket access is denied."""
 
-    MESSAGE = "The AWS Serverless Application Repository does not have read access to bucket '{bucket}', " \
-              "key '{key}'. Please update your Amazon S3 bucket policy to grant the service read " \
-              "permissions to the application artifacts you have uploaded to your S3 bucket. See " \
-              "https://docs.aws.amazon.com/serverlessrepo/latest/devguide/serverless-app-publishing-applications.html" \
-              " for more details."
+    MESSAGE = (
+        "The AWS Serverless Application Repository does not have read access to bucket '{bucket}', "
+        "key '{key}'. Please update your Amazon S3 bucket policy to grant the service read "
+        "permissions to the application artifacts you have uploaded to your S3 bucket. See "
+        "https://docs.aws.amazon.com/serverlessrepo/latest/devguide/serverless-app-publishing-applications.html"
+        " for more details."
+    )
 
 
 class InvalidS3UriError(ServerlessRepoError):
@@ -49,3 +51,9 @@ class ServerlessRepoClientError(ServerlessRepoError):
     """Wrapper for botocore ClientError."""
 
     MESSAGE = "{message}"
+
+
+class MultipleMatchingApplicationsError(ServerlessRepoError):
+    """Raised when multiple matching applications are found."""
+
+    MESSAGE = "{message}"

serverlessrepo/parser.py

@@ -1,6 +1,5 @@
 """Helper to parse JSON/YAML SAM template and dump YAML files."""
 
-import re
 import copy
 import json
 from collections import OrderedDict
@@ -12,9 +11,8 @@
 from .application_metadata import ApplicationMetadata
 from .exceptions import ApplicationMetadataNotFoundError
 
-METADATA = 'Metadata'
-SERVERLESS_REPO_APPLICATION = 'AWS::ServerlessRepo::Application'
-APPLICATION_ID_PATTERN = r'arn:[\w\-]+:serverlessrepo:[\w\-]+:[0-9]+:applications\/[\S]+'
+METADATA = "Metadata"
+SERVERLESS_REPO_APPLICATION = "AWS::ServerlessRepo::Application"
 
 
 def intrinsics_multi_constructor(loader, tag_prefix, node):
@@ -27,17 +25,17 @@ def intrinsics_multi_constructor(loader, tag_prefix, node):
     tag = node.tag[1:]
 
     # Some intrinsic functions doesn't support prefix "Fn::"
-    prefix = 'Fn::'
-    if tag in ['Ref', 'Condition']:
-        prefix = ''
+    prefix = "Fn::"
+    if tag in ["Ref", "Condition"]:
+        prefix = ""
 
     cfntag = prefix + tag
 
-    if tag == 'GetAtt' and isinstance(node.value, six.string_types):
+    if tag == "GetAtt" and isinstance(node.value, six.string_types):
         # ShortHand notation for !GetAtt accepts Resource.Attribute format
         # while the standard notation is to use an array
         # [Resource, Attribute]. Convert shorthand to standard format
-        value = node.value.split('.', 1)
+        value = node.value.split(".", 1)
 
     elif isinstance(node, ScalarNode):
         # Value of this node is scalar
@@ -90,8 +88,10 @@ def parse_template(template_str):
         # json parser.
         return json.loads(template_str, object_pairs_hook=OrderedDict)
     except ValueError:
-        yaml.SafeLoader.add_constructor(yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _dict_constructor)
-        yaml.SafeLoader.add_multi_constructor('!', intrinsics_multi_constructor)
+        yaml.SafeLoader.add_constructor(
+            yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _dict_constructor
+        )
+        yaml.SafeLoader.add_multi_constructor("!", intrinsics_multi_constructor)
         return yaml.safe_load(template_str)
 
 
@@ -110,20 +110,10 @@ def get_app_metadata(template_dict):
         return ApplicationMetadata(app_metadata_dict)
 
     raise ApplicationMetadataNotFoundError(
-        error_message='missing {} section in template Metadata'.format(SERVERLESS_REPO_APPLICATION))
-
-
-def parse_application_id(text):
-    """
-    Extract the application id from input text.
-
-    :param text: text to parse
-    :type text: str
-    :return: application id if found in the input
-    :rtype: str
-    """
-    result = re.search(APPLICATION_ID_PATTERN, text)
-    return result.group(0) if result else None
+        error_message="missing {} section in template Metadata".format(
+            SERVERLESS_REPO_APPLICATION
+        )
+    )
 
 
 def strip_app_metadata(template_dict):
@@ -141,7 +131,8 @@ def strip_app_metadata(template_dict):
     template_dict_copy = copy.deepcopy(template_dict)
 
     # strip the whole metadata section if SERVERLESS_REPO_APPLICATION is the only key in it
-    if not [k for k in template_dict_copy.get(METADATA) if k != SERVERLESS_REPO_APPLICATION]:
+    metadata = template_dict_copy.get(METADATA)
+    if not any(k for k in metadata if k != SERVERLESS_REPO_APPLICATION):
         template_dict_copy.pop(METADATA, None)
     else:
         template_dict_copy.get(METADATA).pop(SERVERLESS_REPO_APPLICATION, None)

serverlessrepo/publish.py

@@ -7,15 +7,17 @@
 from botocore.exceptions import ClientError
 
 from .application_metadata import ApplicationMetadata
-from .parser import (
-    yaml_dump, parse_template, get_app_metadata,
-    parse_application_id, strip_app_metadata
+from .parser import yaml_dump, parse_template, get_app_metadata, strip_app_metadata
+from .exceptions import (
+    MultipleMatchingApplicationsError,
+    ServerlessRepoClientError,
+    S3PermissionsRequired,
+    InvalidS3UriError,
 )
-from .exceptions import ServerlessRepoClientError, S3PermissionsRequired, InvalidS3UriError
 
-CREATE_APPLICATION = 'CREATE_APPLICATION'
-UPDATE_APPLICATION = 'UPDATE_APPLICATION'
-CREATE_APPLICATION_VERSION = 'CREATE_APPLICATION_VERSION'
+CREATE_APPLICATION = "CREATE_APPLICATION"
+UPDATE_APPLICATION = "UPDATE_APPLICATION"
+CREATE_APPLICATION_VERSION = "CREATE_APPLICATION_VERSION"
 
 
 def publish_application(template, sar_client=None):
@@ -31,10 +33,10 @@ def publish_application(template, sar_client=None):
     :raises ValueError
     """
     if not template:
-        raise ValueError('Require SAM template to publish the application')
+        raise ValueError("Require SAM template to publish the application")
 
     if not sar_client:
-        sar_client = boto3.client('serverlessrepo')
+        sar_client = boto3.client("serverlessrepo")
 
     template_dict = _get_template_dict(template)
     app_metadata = get_app_metadata(template_dict)
@@ -43,15 +45,14 @@ def publish_application(template, sar_client=None):
     try:
         request = _create_application_request(app_metadata, stripped_template)
         response = sar_client.create_application(**request)
-        application_id = response['ApplicationId']
+        application_id = response["ApplicationId"]
         actions = [CREATE_APPLICATION]
     except ClientError as e:
         if not _is_conflict_exception(e):
             raise _wrap_client_error(e)
 
         # Update the application if it already exists
-        error_message = e.response['Error']['Message']
-        application_id = parse_application_id(error_message)
+        application_id = _get_application_id(sar_client, app_metadata)
         try:
             request = _update_application_request(app_metadata, application_id)
             sar_client.update_application(**request)
@@ -62,17 +63,19 @@ def publish_application(template, sar_client=None):
         # Create application version if semantic version is specified
         if app_metadata.semantic_version:
             try:
-                request = _create_application_version_request(app_metadata, application_id, stripped_template)
+                request = _create_application_version_request(
+                    app_metadata, application_id, stripped_template
+                )
                 sar_client.create_application_version(**request)
                 actions.append(CREATE_APPLICATION_VERSION)
             except ClientError as e:
                 if not _is_conflict_exception(e):
                     raise _wrap_client_error(e)
 
     return {
-        'application_id': application_id,
-        'actions': actions,
-        'details': _get_publish_details(actions, app_metadata.template_dict)
+        "application_id": application_id,
+        "actions": actions,
+        "details": _get_publish_details(actions, app_metadata.template_dict),
     }
 
 
@@ -89,10 +92,12 @@ def update_application_metadata(template, application_id, sar_client=None):
     :raises ValueError
     """
     if not template or not application_id:
-        raise ValueError('Require SAM template and application ID to update application metadata')
+        raise ValueError(
+            "Require SAM template and application ID to update application metadata"
+        )
 
     if not sar_client:
-        sar_client = boto3.client('serverlessrepo')
+        sar_client = boto3.client("serverlessrepo")
 
     template_dict = _get_template_dict(template)
     app_metadata = get_app_metadata(template_dict)
@@ -116,7 +121,7 @@ def _get_template_dict(template):
     if isinstance(template, dict):
         return copy.deepcopy(template)
 
-    raise ValueError('Input template should be a string or dictionary')
+    raise ValueError("Input template should be a string or dictionary")
 
 
 def _create_application_request(app_metadata, template):
@@ -130,21 +135,21 @@ def _create_application_request(app_metadata, template):
     :return: SAR CreateApplication request body
     :rtype: dict
     """
-    app_metadata.validate(['author', 'description', 'name'])
+    app_metadata.validate(["author", "description", "name"])
     request = {
-        'Author': app_metadata.author,
-        'Description': app_metadata.description,
-        'HomePageUrl': app_metadata.home_page_url,
-        'Labels': app_metadata.labels,
-        'LicenseBody': app_metadata.license_body,
-        'LicenseUrl': app_metadata.license_url,
-        'Name': app_metadata.name,
-        'ReadmeBody': app_metadata.readme_body,
-        'ReadmeUrl': app_metadata.readme_url,
-        'SemanticVersion': app_metadata.semantic_version,
-        'SourceCodeUrl': app_metadata.source_code_url,
-        'SpdxLicenseId': app_metadata.spdx_license_id,
-        'TemplateBody': template
+        "Author": app_metadata.author,
+        "Description": app_metadata.description,
+        "HomePageUrl": app_metadata.home_page_url,
+        "Labels": app_metadata.labels,
+        "LicenseBody": app_metadata.license_body,
+        "LicenseUrl": app_metadata.license_url,
+        "Name": app_metadata.name,
+        "ReadmeBody": app_metadata.readme_body,
+        "ReadmeUrl": app_metadata.readme_url,
+        "SemanticVersion": app_metadata.semantic_version,
+        "SourceCodeUrl": app_metadata.source_code_url,
+        "SpdxLicenseId": app_metadata.spdx_license_id,
+        "TemplateBody": template,
     }
     # Remove None values
     return {k: v for k, v in request.items() if v}
@@ -162,13 +167,13 @@ def _update_application_request(app_metadata, application_id):
     :rtype: dict
     """
     request = {
-        'ApplicationId': application_id,
-        'Author': app_metadata.author,
-        'Description': app_metadata.description,
-        'HomePageUrl': app_metadata.home_page_url,
-        'Labels': app_metadata.labels,
-        'ReadmeBody': app_metadata.readme_body,
-        'ReadmeUrl': app_metadata.readme_url
+        "ApplicationId": application_id,
+        "Author": app_metadata.author,
+        "Description": app_metadata.description,
+        "HomePageUrl": app_metadata.home_page_url,
+        "Labels": app_metadata.labels,
+        "ReadmeBody": app_metadata.readme_body,
+        "ReadmeUrl": app_metadata.readme_url,
     }
     return {k: v for k, v in request.items() if v}
 
@@ -186,12 +191,12 @@ def _create_application_version_request(app_metadata, application_id, template):
     :return: SAR CreateApplicationVersion request body
     :rtype: dict
     """
-    app_metadata.validate(['semantic_version'])
+    app_metadata.validate(["semantic_version"])
     request = {
-        'ApplicationId': application_id,
-        'SemanticVersion': app_metadata.semantic_version,
-        'SourceCodeUrl': app_metadata.source_code_url,
-        'TemplateBody': template
+        "ApplicationId": application_id,
+        "SemanticVersion": app_metadata.semantic_version,
+        "SourceCodeUrl": app_metadata.source_code_url,
+        "TemplateBody": template,
     }
     return {k: v for k, v in request.items() if v}
 
@@ -204,8 +209,8 @@ def _is_conflict_exception(e):
     :type e: ClientError
     :return: True if e is ConflictException
     """
-    error_code = e.response['Error']['Code']
-    return error_code == 'ConflictException'
+    error_code = e.response["Error"]["Code"]
+    return error_code == "ConflictException"
 
 
 def _wrap_client_error(e):
@@ -216,12 +221,12 @@ def _wrap_client_error(e):
     :type e: ClientError
     :return: S3PermissionsRequired or InvalidS3UriError or general ServerlessRepoClientError
     """
-    error_code = e.response['Error']['Code']
-    message = e.response['Error']['Message']
+    error_code = e.response["Error"]["Code"]
+    message = e.response["Error"]["Message"]
 
-    if error_code == 'BadRequestException':
+    if error_code == "BadRequestException":
         if "Failed to copy S3 object. Access denied:" in message:
-            match = re.search('bucket=(.+?), key=(.+?)$', message)
+            match = re.search("bucket=(.+?), key=(.+?)$", message)
             if match:
                 return S3PermissionsRequired(bucket=match.group(1), key=match.group(2))
         if "Invalid S3 URI" in message:
@@ -250,11 +255,36 @@ def _get_publish_details(actions, app_metadata_template):
         ApplicationMetadata.HOME_PAGE_URL,
         ApplicationMetadata.LABELS,
         ApplicationMetadata.README_URL,
-        ApplicationMetadata.README_BODY
+        ApplicationMetadata.README_BODY,
     ]
 
     if CREATE_APPLICATION_VERSION in actions:
         # SemanticVersion and SourceCodeUrl can only be updated by creating a new version
-        additional_keys = [ApplicationMetadata.SEMANTIC_VERSION, ApplicationMetadata.SOURCE_CODE_URL]
+        additional_keys = [
+            ApplicationMetadata.SEMANTIC_VERSION,
+            ApplicationMetadata.SOURCE_CODE_URL,
+        ]
         include_keys.extend(additional_keys)
     return {k: v for k, v in app_metadata_template.items() if k in include_keys and v}
+
+
+def _get_application_id(sar_client, metadata):
+    """
+    Gets the application ID of rhte matching application name.
+
+    :param sar_client: The boto3 SAR client.
+    :param metadata: The application meta data.
+    :return: The matching application ID.
+    :rtype: str
+    :raises: MultipleMatchingApplicationsError
+    """
+    application_ids = []
+    pager = sar_client.get_paginator("list_applications")
+    for application in pager.paginate():
+        if application["Name"] == metadata.name:
+            application_ids.append(application["ApplicationId"])
+    if len(application_ids) > 1:
+        raise MultipleMatchingApplicationsError(
+            message='Multiple applications with the name "%s"' % metadata.name
+        )
+    return application_ids[0] if len(application_ids) == 1 else None