Skip to content

feat: Linking Authorizers to Lambda functions using the invocation URI #5196

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.

Sign up for GitHub

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

Merged
moelasmar merged 14 commits into from
May 27, 2023
Merged

feat: Linking Authorizers to Lambda functions using the invocation URI #5196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a286a70
Link authorizer to lambda function invoke URI
lucashuy May 24, 2023
1abb682
Merge branch 'develop' of github.com:aws/aws-sam-cli into link_tf_aut…
lucashuy May 24, 2023
11677e4
Updated doc string
lucashuy May 24, 2023
bee4eab
Updated exception messages back
lucashuy May 24, 2023
8c3f56c
Merge branch 'develop' into link_tf_authorizers
lucashuy May 25, 2023
cec2f98
Merge branch 'develop' of github.com:aws/aws-sam-cli into link_tf_aut…
lucashuy May 25, 2023
01d3a18
Merge branch 'link_tf_authorizers' of github.com:lucashuy/aws-sam-cli…
lucashuy May 25, 2023
5d861db
Added check for one element in reference list
lucashuy May 25, 2023
9dd1412
Merge branch 'develop' of github.com:aws/aws-sam-cli into link_tf_aut…
lucashuy May 26, 2023
fdbbdd4
Updated empty ref list check to not block
lucashuy May 26, 2023
eff7ad8
Updated log message
lucashuy May 26, 2023
1c9541d
Merge branch 'develop' of github.com:aws/aws-sam-cli into link_tf_aut…
lucashuy May 26, 2023
e1a008e
Fix long line lint error
lucashuy May 26, 2023
11d6180
Merge branch 'develop' into link_tf_authorizers
moelasmar May 26, 2023
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
14 changes: 14 additions & 0 deletions samcli/hook_packages/terraform/hooks/prepare/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,20 @@ class GatewayResourceToApiGatewayIntegrationResponseLocalVariablesLinkingLimitat
"""


class OneGatewayAuthorizerToLambdaFunctionLinkingLimitationException(OneResourceLinkingLimitationException):
"""
Exception specific for Gateway Authorizer linking to more than one Lambda Function
"""


class GatewayAuthorizerToLambdaFunctionLocalVariablesLinkingLimitationException(
LocalVariablesLinkingLimitationException
):
"""
Exception specific for Gateway Authorizer linking to Lambda Function using locals.
"""


class InvalidSamMetadataPropertiesException(UserException):
pass

Expand Down
89 changes: 89 additions & 0 deletions samcli/hook_packages/terraform/hooks/prepare/resource_linking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@

from samcli.hook_packages.terraform.hooks.prepare.exceptions import (
FunctionLayerLocalVariablesLinkingLimitationException,
GatewayAuthorizerToLambdaFunctionLocalVariablesLinkingLimitationException,
GatewayResourceToApiGatewayIntegrationLocalVariablesLinkingLimitationException,
GatewayResourceToApiGatewayIntegrationResponseLocalVariablesLinkingLimitationException,
GatewayResourceToApiGatewayMethodLocalVariablesLinkingLimitationException,
GatewayResourceToGatewayRestApiLocalVariablesLinkingLimitationException,
InvalidResourceLinkingException,
LambdaFunctionToApiGatewayIntegrationLocalVariablesLinkingLimitationException,
LocalVariablesLinkingLimitationException,
OneGatewayAuthorizerToLambdaFunctionLinkingLimitationException,
OneGatewayResourceToApiGatewayIntegrationLinkingLimitationException,
OneGatewayResourceToApiGatewayIntegrationResponseLinkingLimitationException,
OneGatewayResourceToApiGatewayMethodLinkingLimitationException,
Expand Down Expand Up @@ -1086,6 +1088,10 @@ def _link_gateway_resource_to_gateway_rest_apis_rest_api_id_call_back(
if len(referenced_rest_apis_values) > 1:
raise InvalidResourceLinkingException("Could not link multiple Rest APIs to one Gateway resource")

if not referenced_rest_apis_values:
LOG.info("Unable to find any references to Rest APIs, skip linking Gateway resources")
return

logical_id = referenced_rest_apis_values[0]
gateway_cfn_resource["Properties"]["RestApiId"] = (
{"Ref": logical_id.value} if isinstance(logical_id, LogicalIdReference) else logical_id.value
Expand All @@ -1111,6 +1117,10 @@ def _link_gateway_resource_to_gateway_resource_call_back(
if len(referenced_gateway_resource_values) > 1:
raise InvalidResourceLinkingException("Could not link multiple Gateway Resources to one Gateway resource")

if not referenced_gateway_resource_values:
LOG.info("Unable to find any references to the Gateway Resource, skip linking Gateway resources")
return

logical_id = referenced_gateway_resource_values[0]
gateway_resource_cfn_resource["Properties"]["ResourceId"] = (
{"Ref": logical_id.value} if isinstance(logical_id, LogicalIdReference) else logical_id.value
Expand All @@ -1137,6 +1147,10 @@ def _link_gateway_resource_to_gateway_rest_apis_parent_id_call_back(
if len(referenced_rest_apis_values) > 1:
raise InvalidResourceLinkingException("Could not link multiple Rest APIs to one Gateway resource")

if not referenced_rest_apis_values:
LOG.info("Unable to find any references to Rest APIs, skip linking Rest API to Gateway resource")
return

logical_id = referenced_rest_apis_values[0]
gateway_cfn_resource["Properties"]["ParentId"] = (
{"Fn::GetAtt": [logical_id.value, "RootResourceId"]}
Expand Down Expand Up @@ -1351,6 +1365,12 @@ def _link_gateway_integration_to_function_call_back(
"Could not link multiple Lambda functions to one Gateway integration resource"
)

if not referenced_gateway_resource_values:
LOG.info(
"Unable to find any references to Lambda functions, skip linking Lambda function to Gateway integration"
)
return

logical_id = referenced_gateway_resource_values[0]
gateway_integration_cfn_resource["Properties"]["Uri"] = (
{"Fn::Sub": INVOKE_ARN_FORMAT.format(function_logical_id=logical_id.value)}
Expand Down Expand Up @@ -1477,3 +1497,72 @@ def _link_gateway_integration_responses_to_gateway_resource(
linking_exceptions=exceptions,
)
ResourceLinker(resource_linking_pair).link_resources()


def _link_gateway_authorizer_to_lambda_function_call_back(
gateway_authorizer_cfn_resource: Dict, lambda_function_resource_values: List[ReferenceType]
) -> None:
"""
Callback function that is used by the linking algorithm to update a CFN Authorizer Resource with
a reference to the Lambda function's invocation URI

Parameters
----------
gateway_authorizer_cfn_resource: Dict
API Gateway Authorizer CFN resource
lambda_function_resource_values: List[ReferenceType]
List of referenced Lambda Functions either as the logical id of Lambda Function reosurces
defined in the customer project, or ARN values for actual Lambda Functions defined
in customer's account. This list should always contain one element only.
"""
if len(lambda_function_resource_values) > 1:
raise InvalidResourceLinkingException("Could not link multiple Lambda functions to one Gateway Authorizer")

if not lambda_function_resource_values:
LOG.info(
"Unable to find any references to Lambda functions, skip linking Lambda function to Gateway Authorizer"
)
return

logical_id = lambda_function_resource_values[0]
Copy link
Contributor

@mndeveci mndeveci May 25, 2023

Choose a reason for hiding this comment

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

we are checking if lambda_function_resource_values are greater than 1 but we are not checking if there is at least 1 element in it. Should we also check that before accessing it directly?

Copy link
Contributor Author

@lucashuy lucashuy May 25, 2023

Choose a reason for hiding this comment

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

In theory, lambda_function_resource_values will contain something, whether the linking happens with an applied resource, or a non applied resource.

With that being said though, I don't think it would add too much we just coded more defensively since both these checks can be done in a helper method, and applied to all the different callbacks.

Copy link
Contributor Author

@lucashuy lucashuy May 25, 2023

Choose a reason for hiding this comment

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

After some discussion with another team member, a new check will likely happen earlier in the call stack in a different PR to not expand the scope of this PR.

Copy link
Contributor

@mndeveci mndeveci May 25, 2023

Choose a reason for hiding this comment

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

Why not change if statement above with if len(lambda_function_resource_values) != 1: and also update the exception message?

I do understand that we are adding another check in the flow, but that still leaves a possible error in the future since it will be added in another place. If the execution flow changes or if we call this method from somewhere else then we will get an unexpected exception here.

Copy link
Contributor Author

@lucashuy lucashuy May 25, 2023

Choose a reason for hiding this comment

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

That's a good point about using the call back methods elsewhere. Was going just log the failed linking attempt (if there aren't any references) and skip linking, but failing early at the hook stage is likely better customer UX rather than waiting until it gets to the start-api phase to see an error.

Copy link
Contributor

@moelasmar moelasmar May 26, 2023

Choose a reason for hiding this comment

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

I think we should not raise exception if this list is empty. I see if this list is empty, so it means we could not find any destination resources to be linked to the source resource. It may be invalid semantically, but the purpose of the hook is to map between the TF project and the CFN, and these type of validations are left to the core SAM functionalities.

For example, if the customers want to run sam local invoke to test lambda function, but the TF project is not complete, and the customers do not add all the required API Gateway resources, I do not think we should stop them from using SAM CLI.

Copy link
Contributor

@moelasmar moelasmar May 26, 2023

Choose a reason for hiding this comment

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

I am ok for raising some exception here in the callback, but not to call the callback function at all if there is no destinations to be linked.

gateway_authorizer_cfn_resource["Properties"]["AuthorizerUri"] = (
{"Fn::Sub": INVOKE_ARN_FORMAT.format(function_logical_id=logical_id.value)}
if isinstance(logical_id, LogicalIdReference)
else logical_id.value
)


def _link_gateway_authorizer_to_lambda_function(
authorizer_config_resources: Dict[str, TFResource],
authorizer_cfn_resources: Dict[str, List],
authorizer_tf_resources: Dict[str, Dict],
) -> None:
"""
Iterate through all the resources and link the corresponding Authorizer to each Lambda Function

Parameters
----------
authorizer_config_resources: Dict[str, TFResource]
Dictionary of configuration Authorizer resources
authorizer_cfn_resources: Dict[str, List]
Dictionary containing resolved configuration address of CFN Authorizer resources
lambda_layers_terraform_resources: Dict[str, Dict]
Dictionary of all actual terraform layers resources (not configuration resources). The dictionary's key is the
calculated logical id for each resource
"""
exceptions = ResourcePairExceptions(
multiple_resource_linking_exception=OneGatewayAuthorizerToLambdaFunctionLinkingLimitationException,
local_variable_linking_exception=GatewayAuthorizerToLambdaFunctionLocalVariablesLinkingLimitationException,
)
resource_linking_pair = ResourceLinkingPair(
source_resource_cfn_resource=authorizer_cfn_resources,
source_resource_tf_config=authorizer_config_resources,
destination_resource_tf=authorizer_tf_resources,
tf_destination_attribute_name="invoke_arn",
terraform_link_field_name="authorizer_uri",
cfn_link_field_name="AuthorizerUri",
terraform_resource_type_prefix=LAMBDA_FUNCTION_RESOURCE_ADDRESS_PREFIX,
cfn_resource_update_call_back_function=_link_gateway_authorizer_to_lambda_function_call_back,
linking_exceptions=exceptions,
)
ResourceLinker(resource_linking_pair).link_resources()
9 changes: 9 additions & 0 deletions samcli/hook_packages/terraform/hooks/prepare/resources/apigw.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ def __init__(self):
super(ApiGatewayStageProperties, self).__init__()


class ApiGatewayAuthorizerProperties(ResourceProperties):
"""
Contains the collection logic of the required properties for linking the aws_api_gateway_authorizer resources.
"""

def __init__(self):
super(ApiGatewayAuthorizerProperties, self).__init__()


def add_integrations_to_methods(
gateway_methods_cfn: Dict[str, List], gateway_integrations_cfn: Dict[str, List]
) -> None:
Expand Down
7 changes: 7 additions & 0 deletions samcli/hook_packages/terraform/hooks/prepare/resources/resource_links.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import List

from samcli.hook_packages.terraform.hooks.prepare.property_builder import (
TF_AWS_API_GATEWAY_AUTHORIZER,
TF_AWS_API_GATEWAY_INTEGRATION,
TF_AWS_API_GATEWAY_INTEGRATION_RESPONSE,
TF_AWS_API_GATEWAY_METHOD,
Expand All @@ -11,6 +12,7 @@
TF_AWS_LAMBDA_LAYER_VERSION,
)
from samcli.hook_packages.terraform.hooks.prepare.resource_linking import (
_link_gateway_authorizer_to_lambda_function,
_link_gateway_integration_responses_to_gateway_resource,
_link_gateway_integration_responses_to_gateway_rest_apis,
_link_gateway_integrations_to_function_resource,
Expand Down Expand Up @@ -71,4 +73,9 @@
dest=TF_AWS_API_GATEWAY_RESOURCE,
linking_func=_link_gateway_integration_responses_to_gateway_resource,
),
LinkingPairCaller(
source=TF_AWS_API_GATEWAY_AUTHORIZER,
dest=TF_AWS_LAMBDA_FUNCTION,
linking_func=_link_gateway_authorizer_to_lambda_function,
),
]
3 changes: 3 additions & 0 deletions samcli/hook_packages/terraform/hooks/prepare/resources/resource_properties.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from typing import Dict

from samcli.hook_packages.terraform.hooks.prepare.property_builder import (
TF_AWS_API_GATEWAY_AUTHORIZER,
TF_AWS_API_GATEWAY_INTEGRATION,
TF_AWS_API_GATEWAY_INTEGRATION_RESPONSE,
TF_AWS_API_GATEWAY_METHOD,
Expand All @@ -12,6 +13,7 @@
TF_AWS_LAMBDA_LAYER_VERSION,
)
from samcli.hook_packages.terraform.hooks.prepare.resources.apigw import (
ApiGatewayAuthorizerProperties,
ApiGatewayMethodProperties,
ApiGatewayResourceProperties,
ApiGatewayRestApiProperties,
Expand Down Expand Up @@ -46,4 +48,5 @@ def get_resource_property_mapping() -> Dict[str, ResourceProperties]:
TF_AWS_API_GATEWAY_STAGE: ApiGatewayStageProperties(),
TF_AWS_API_GATEWAY_INTEGRATION: InternalApiGatewayIntegrationProperties(),
TF_AWS_API_GATEWAY_INTEGRATION_RESPONSE: InternalApiGatewayIntegrationResponseProperties(),
TF_AWS_API_GATEWAY_AUTHORIZER: ApiGatewayAuthorizerProperties(),
}
Loading