Skip to content

Commit

Permalink
Merge pull request #235 from adanaja/morosi-fix
Browse files Browse the repository at this point in the history
Fix RestApi referencing $default in LambdaPermission
  • Loading branch information
adanaja authored Jan 12, 2024
2 parents e887671 + 5ef82f6 commit 09835af
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 48 deletions.
27 changes: 12 additions & 15 deletions src/e3/aws/troposphere/apigateway/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from troposphere import AWSObject
from troposphere.certificatemanager import Certificate, DomainValidationOption
import json
import re

if TYPE_CHECKING:
from e3.aws.troposphere import Stack
Expand Down Expand Up @@ -195,6 +194,9 @@ class _AliasTargetAttributes(TypedDict):
DNSName: str
HostedZoneId: str

# The default stage name
DEFAULT_STAGE_NAME = "$default"

def __init__(
self,
name: str,
Expand Down Expand Up @@ -240,7 +242,9 @@ def __init__(
self.authorizers: dict[str, Any] = {}
# By default, make sure to have a $default stage
self.stages_config = (
stages_config if stages_config else [StageConfiguration("$default")]
stages_config
if stages_config
else [StageConfiguration(self.DEFAULT_STAGE_NAME)]
)

@cached_property
Expand Down Expand Up @@ -697,6 +701,9 @@ def resources(self, stack: Stack) -> list[AWSObject]:
class RestApi(Api):
"""Rest API support."""

# Apigateway v1 only allows a-zA-Z0-9_
DEFAULT_STAGE_NAME = "default"

def __init__(
self,
name: str,
Expand Down Expand Up @@ -878,8 +885,7 @@ def declare_stage(
DeploymentId=Ref(deployment_name),
Description=f"stage {stage_name}",
MethodSettings=[method_settings],
# Stage name only allows a-zA-Z0-9_
StageName=re.sub("[^a-zA-Z0-9_]", "", stage_name),
StageName=stage_name,
**parameters,
)
)
Expand Down Expand Up @@ -948,12 +954,7 @@ def _declare_method(
for config in self.stages_config:
result.append(
awslambda.Permission(
# Retain old behavior for the $default stage
name_to_id(
"{}-{}LambdaPermission".format(
id_prefix, "" if config.name == "$default" else config.name
)
),
name_to_id(f"{id_prefix}-{config.name}LambdaPermission"),
Action="lambda:InvokeFunction",
FunctionName=lambda_arn,
Principal="apigateway.amazonaws.com",
Expand Down Expand Up @@ -1006,11 +1007,7 @@ def _declare_api_mapping(
apigateway.BasePathMapping(
# Retain old behavior for the $default stage
name_to_id(
"{}{}-{}BasePathMapping".format(
self.name,
domain_name.DomainName,
"" if config.name == "$default" else config.name,
)
f"{self.name}{domain_name.DomainName}-{config.name}BasePathMapping"
),
**mapping_params,
)
Expand Down
6 changes: 3 additions & 3 deletions tests/tests_e3_aws/troposphere/apigateway/apigateway_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ def test_rest_api_stages(stack: Stack, lambda_fun: PyFunction) -> None:
Method("ANY"),
],
stages_config=[
StageConfiguration("$default"),
StageConfiguration("default"),
StageConfiguration(
"beta", api_mapping_key="beta", variables={"somevar": "somevalue"}
),
Expand Down Expand Up @@ -574,7 +574,7 @@ def test_rest_api_lambda_alias(stack: Stack, lambda_fun: PyFunction) -> None:
],
stages_config=[
StageConfiguration(
"$default",
"default",
lambda_arn_permission=lambda_aliases.blue.ref,
variables={"lambdaAlias": lambda_aliases.blue.name},
),
Expand Down Expand Up @@ -667,7 +667,7 @@ def test_rest_api_custom_domain_stages(stack: Stack, lambda_fun: PyFunction) ->
Method("ANY", authorizer_name="testauthorizer"),
],
stages_config=[
StageConfiguration("$default"),
StageConfiguration("default"),
StageConfiguration(
"beta", api_mapping_key="beta", variables={"somevar": "somevalue"}
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
},
"TestapiDefaultDeployment": {
"Properties": {
"Description": "Deployment resource of $default stage",
"Description": "Deployment resource of default stage",
"RestApiId": {
"Ref": "Testapi"
}
Expand All @@ -72,7 +72,7 @@
"DeploymentId": {
"Ref": "TestapiDefaultDeployment"
},
"Description": "stage $default",
"Description": "stage default",
"MethodSettings": [
{
"ResourcePath": "/*",
Expand Down Expand Up @@ -116,7 +116,7 @@
},
"Type": "AWS::ApiGateway::Method"
},
"TestapiANYLambdaPermission": {
"TestapiANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -125,7 +125,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/*",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/*",
{
"api": {
"Ref": "Testapi"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
},
"TestapiDefaultDeployment": {
"Properties": {
"Description": "Deployment resource of $default stage",
"Description": "Deployment resource of default stage",
"RestApiId": {
"Ref": "Testapi"
}
Expand All @@ -98,7 +98,7 @@
"DeploymentId": {
"Ref": "TestapiDefaultDeployment"
},
"Description": "stage $default",
"Description": "stage default",
"MethodSettings": [
{
"ResourcePath": "/*",
Expand Down Expand Up @@ -145,7 +145,7 @@
},
"Type": "AWS::ApiGateway::Method"
},
"TestapiANYLambdaPermission": {
"TestapiANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -154,7 +154,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/*",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/*",
{
"api": {
"Ref": "Testapi"
Expand Down Expand Up @@ -188,7 +188,7 @@
},
"Type": "AWS::ApiGateway::DomainName"
},
"TestapiapiexamplecomBasePathMapping": {
"TestapiapiexamplecomDefaultBasePathMapping": {
"Properties": {
"DomainName": {
"Ref": "TestapiapiexamplecomDomain"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
},
"TestapiDefaultDeployment": {
"Properties": {
"Description": "Deployment resource of $default stage",
"Description": "Deployment resource of default stage",
"RestApiId": {
"Ref": "Testapi"
}
Expand All @@ -98,7 +98,7 @@
"DeploymentId": {
"Ref": "TestapiDefaultDeployment"
},
"Description": "stage $default",
"Description": "stage default",
"MethodSettings": [
{
"ResourcePath": "/*",
Expand Down Expand Up @@ -212,7 +212,7 @@
},
"Type": "AWS::Lambda::Permission"
},
"TestapiANYLambdaPermission": {
"TestapiANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -221,7 +221,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/*",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/*",
{
"api": {
"Ref": "Testapi"
Expand Down Expand Up @@ -255,7 +255,7 @@
},
"Type": "AWS::ApiGateway::DomainName"
},
"TestapiapiexamplecomBasePathMapping": {
"TestapiapiexamplecomDefaultBasePathMapping": {
"Properties": {
"DomainName": {
"Ref": "TestapiapiexamplecomDomain"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
},
"TestapiDefaultDeployment": {
"Properties": {
"Description": "Deployment resource of $default stage",
"Description": "Deployment resource of default stage",
"RestApiId": {
"Ref": "Testapi"
}
Expand All @@ -118,7 +118,7 @@
"DeploymentId": {
"Ref": "TestapiDefaultDeployment"
},
"Description": "stage $default",
"Description": "stage default",
"MethodSettings": [
{
"ResourcePath": "/*",
Expand Down Expand Up @@ -177,7 +177,7 @@
},
"Type": "AWS::Lambda::Permission"
},
"TestapiANYLambdaPermission": {
"TestapiANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -186,7 +186,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/*",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/*",
{
"api": {
"Ref": "Testapi"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
},
"TestapiDefaultDeployment": {
"Properties": {
"Description": "Deployment resource of $default stage",
"Description": "Deployment resource of default stage",
"RestApiId": {
"Ref": "Testapi"
}
Expand Down Expand Up @@ -104,7 +104,7 @@
"DeploymentId": {
"Ref": "TestapiDefaultDeployment"
},
"Description": "stage $default",
"Description": "stage default",
"MethodSettings": [
{
"ResourcePath": "/*",
Expand Down Expand Up @@ -208,7 +208,7 @@
},
"Type": "AWS::ApiGateway::Method"
},
"TestapiAccountsANYLambdaPermission": {
"TestapiAccountsANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -217,7 +217,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/accounts",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/accounts",
{
"api": {
"Ref": "Testapi"
Expand All @@ -229,7 +229,7 @@
},
"Type": "AWS::Lambda::Permission"
},
"TestapiProductsANYLambdaPermission": {
"TestapiProductsANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -238,7 +238,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/products",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/products",
{
"api": {
"Ref": "Testapi"
Expand All @@ -250,7 +250,7 @@
},
"Type": "AWS::Lambda::Permission"
},
"TestapiProductsAbcdANYLambdaPermission": {
"TestapiProductsAbcdANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -259,7 +259,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/products/abcd",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/products/abcd",
{
"api": {
"Ref": "Testapi"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
},
"TestapiDefaultDeployment": {
"Properties": {
"Description": "Deployment resource of $default stage",
"Description": "Deployment resource of default stage",
"RestApiId": {
"Ref": "Testapi"
}
Expand All @@ -118,7 +118,7 @@
"DeploymentId": {
"Ref": "TestapiDefaultDeployment"
},
"Description": "stage $default",
"Description": "stage default",
"MethodSettings": [
{
"ResourcePath": "/*",
Expand Down Expand Up @@ -183,7 +183,7 @@
},
"Type": "AWS::Lambda::Permission"
},
"TestapiANYLambdaPermission": {
"TestapiANYDefaultLambdaPermission": {
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
Expand All @@ -192,7 +192,7 @@
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/$default/${method}/*",
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${api}/default/${method}/*",
{
"api": {
"Ref": "Testapi"
Expand Down

0 comments on commit 09835af

Please sign in to comment.