Skip to content

Conversation

reedham-aws
Copy link
Contributor

@reedham-aws reedham-aws commented Aug 28, 2025

Issue #, if available

N/a

Description of changes

The BasePath attribute of Aws::Serverless::Api does not currently accept !Sub as a parameter, instead leaving it as blank or returning None. As an example,

Resources:
  ApiGatewayApi:
    Type: AWS::Serverless::Api
    Properties:
      StageName: prod
      CacheClusterEnabled: true
      CacheClusterSize: '0.5'
      MethodSettings:
        - ResourcePath: /
          HttpMethod: GET
          CachingEnabled: true
          CacheTtlInSeconds: 300
      Domain:
        BasePath: !Sub ${ Service }-api
        CertificateArn: !Ref CertificateArn
        DomainName: example.myinstance.com
        Route53:
          HostedZoneId: !Ref HostedZoneId

Transforms to the below, without the actual BasePath being added.

"ApiGatewayApiBasePathMapping": {
    "Type": "AWS::ApiGateway::BasePathMapping",
    "Properties": {
        "DomainName": {
            "Ref": "ApiGatewayDomainName619a1fb284"
        },
        "RestApiId": {
            "Ref": "ApiGatewayApi"
        },
        "Stage": {
            "Ref": "ApiGatewayApiprodStage"
        }
    }
}

After updates

With the updates to allow intrinsics for this property, the output now looks like:

"Parameters": {
    "Service": {
        "Type": "String",
        "Default": "NewServ"
    }
},
...
"TestApiBasePathMapping": {
    "Type": "AWS::ApiGateway::BasePathMapping",
    "Properties": {
        "BasePath": {
            "Fn::Sub": "NewServ-api"
         },
        "DomainName": {
            "Ref": "ApiGatewayDomainName54e3d11755"
        },
        "RestApiId": {
            "Ref": "TestApi"
        },
        "Stage": {
            "Ref": "TestApiprodStage"
        }
    }
}

Description of how you validated changes

Manually ran the transform with and without the updated changes and compared the templates.

Updated the transform tests to include this new case. I didn't add any new tests because there are other locations that have BasePath without the intrinsics, so I figured it was fine to add this in the api_with_basic_custom_domain_intrinsics tests.

Also confirmed that the added tests failed with the older transform version.

Checklist

Examples?

Please reach out in the comments if you want to add an example. Examples will be
added to sam init through aws/aws-sam-cli-app-templates.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@reedham-aws reedham-aws requested a review from a team as a code owner August 28, 2025 17:25
seshubaws
seshubaws previously approved these changes Aug 28, 2025
mapping_basepath = path if normalize_basepath else basepath
logical_id = "{}{}{}".format(self.logical_id, path, "BasePathMapping")
if is_intrinsic(basepath):
mapping_basepath = basepath
Copy link
Contributor

Choose a reason for hiding this comment

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

So the parameter NormalizeBasePath is not used anymore if the BasePath is passed as an intrinsic?

logical_id = "{}{}{}".format(self.logical_id, path, "BasePathMapping")
if is_intrinsic(basepath):
mapping_basepath = basepath
logical_id = self.logical_id + "BasePathMapping"
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if there are different base paths all defined with intrinsics? Will they just have the same logical id? Should we add some sort of hash for the intrinsic values here, so we actually create different resources?

@@ -58,7 +58,7 @@ Resources:
DomainName: !Sub 'example-${AWS::Region}.com'
CertificateArn: !Ref MyDomainCert
EndpointConfiguration: !Ref EndpointConf
BasePath: [/get, /fetch]
BasePath: !Sub '${AWS::Region}-api'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you still keep the list, but do with intrinsics instead?

Something like

Suggested change
BasePath: !Sub '${AWS::Region}-api'
BasePath:
- !Sub '${AWS::Region}-get'
- !Sub '${AWS::Region}-fetch'

basepath_value = self.domain.get("BasePath")
# Create BasepathMappings
if self.domain.get("BasePath") and isinstance(basepath_value, str):
if isinstance(basepath_value, str) or is_intrinsic(basepath_value):
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on adding this same support on http_api_generator.py file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants