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

(apiGateway): path may have some resources are already exist in api_gateway, the CDK still will create all resources for the path. #31093

Open
1 task
lulala1126 opened this issue Aug 13, 2024 · 1 comment
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@lulala1126
Copy link

lulala1126 commented Aug 13, 2024

Describe the bug

Hi team, there are some very tricky errors when we use RestApi.from_rest_api_attributes and 'resource.resource_for_path(path)'.

We are using Python lib.

Our team wants to add methods and resources to an existing API on several stacks.

First, we are using from_rest_api_attributes to get an existing restApi:
api = aws_apigateway.RestApi.from_rest_api_attributes( scope=scope, id='Api', rest_api_id=api_id, root_resource_id=root_resource_id )

then we want to add those resources on the A stack:
res = api.root.resource_for_path('/api/test/list') res.add_method("get") res = api.root.resource_for_path('/api/test/get') res.add_method("get")

and add resources on the B stack:
res = api.root.resource_for_path('/api/test/update') res.add_method("get") res = api.root.resource_for_path('/api/test/delete') res.add_method("get")
Result: The A stack was deployed successfully, but the B stack was deployed failed by '/api, /test resource already exists'.

The reason was that in resourceForPath, CDK is using getResource to determine if api resource is a child of /, and recursive execute each part of the path. but how each resource knows their children, is when resource init they will add themselves to the parent resoure's child list. like: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-apigateway/lib/resource.ts#L446. because this, if we only know the root resource, we can't know the all children of the root resource.

So to fix this we found two solutions, but only one is working.

Solution One:
create our customized resourceForPath method like:

  def get_resource_from_api( apigateway: str, path: str) -> Resource: # apigateway are like: "rest_id:root_resources_id"
        if ':' in apigateway and apigateway[0] != ':' and apigateway[-1] != ':':  
            rest_id, root_resources_id = apigateway.split(':')
            all_exist_resources = self.api_gateway_client.client.get_resources(restApiId=rest_id)
            return self.get_resource_from_exist_api(all_exist_resources, path, api.root, 0)
        return api.root.resource_for_path(path)

  def get_resource_from_exist_api(all_exist_resources: dict, path: str, root: any, index) -> Resource:
      if index > len(path.split('/')):
          return root
      if path.startswith('/'):
          return get_resource_from_exist_api(all_exist_resources, path[1:], root, index+1)
      parts = path.split('/')
      part_path = '/' + '/'.join(parts[:index])
      part_path_resource_id = _get_part_path_resource_id_from_api_resources(all_exist_resources, part_path)

      if part_path_resource_id:
          print(f"${part_path} exist, id is ${part_path_resource_id}")
          resource = Resource.from_resource_attributes(self, id=part_path+"resource_id", path=part_path, resource_id=part_path_resource_id,
                                                       rest_api=self.api)
      if not part_path_resource_id:
          print(f"${part_path} not exist, create it")
          resource = root.add_resource(parts[index-1])
          all_exist_resources = None
      return self.get_resource_from_exist_api(all_exist_resources, path, resource, index + 1)

def _get_part_path_resource_id_from_api_resources(all_exist_resources: dict, path: str) -> str | None:
    if all_exist_resources:
        for item in all_exist_resources['items']:
            if item['path'] == path:
                return item['id']
    return None

But this solution failed when we try updated the path resources on the stack, for example:

In the first deployment, we create path /test/v1, and this stack newly creates and manages each resource.

second deployment, we update /test/v1 to test/v2, and the code will think the /test resource already exists, so will remove this resource from the stack, so all resources belonging to /test/v2 will disappear.

We try to use the resource. stack to confirm if this resource is managed by this stack so we can re-handler it, but this stack param doesn't work as we expect, so we don't know how to fix it.

Solution Two(accepted):

we consider maybe we can use a separate stack to create and manage all path resources and apigateway, so in our deployment stack, we can just import an existing path and add a method for it.

for example, we have an existing path /test/v1 resource id is 545etf, and we use 545etf as the root_resourse_id set on from_rest_api_attributes to get API gateway, so we can add method directly to the root resource, code is like:

api = aws_apigateway.RestApi.from_rest_api_attributes(
            scope=scope,
            id='Api',
            rest_api_id=api_id,
            root_resource_id='545etf'
        )
resource = api.root.resource_for_path("/")
resource.add_method("get")

But, this solution also failed, because even if we set the root_resource to 545etf, we can get the correct resource id on the api.root.resource_id, but the api.root.path result is just /. so the method was added succeed on the apigateway side, but the lambda will trigger by a wrong apigateway path and cause the error.

The reason is in CDK we just create new resource for this root_resource_id, not to find exist one.
we found a complex way to avoid this error, the final workaround is like:

api = aws_apigateway.RestApi.from_rest_api_attributes(
            scope=scope,
            id='Api',
            rest_api_id=api_id,
            root_resource_id='545etf'
        )
resource = api.root.resource_for_path("/")
root_path = api_gateway_client.client.get_resource(restApiId=rest_id,
                                                                        resourceId=`545etf`)['path']
if root_path != '/':
    full_path = root_path + item.path if item.path != '/' else root_path
    res = Resource.from_resource_attributes(self, id=full_path + "resource_id", path=full_path,
                                            resource_id=res.resource_id,
                                            rest_api=cast(aws_apigateway.RestApi,API))

Option 2 is working normally for me now, but we think it's valuable to raise an issue with your team and see if you have a better solution, or if these two errors can be fixed by the CDK side.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

Resource.resource_for_path method can GETS or create all resources leading up to the specified path.

when we set a different root resource id on from_rest_api_attributes, we can get the correct path when calling api. root.path.

Current Behavior

Resource.resource_for_path method will creates all resources leading up to the specified path, even some resources are already existed.

when we set a different root resource id on from_rest_api_attributes, the api.root.path always return /.

Reproduction Steps

follow the code on the bug description.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.113.0

Framework Version

No response

Node.js Version

v20.12.2

OS

mac 14.6

Language

Python

Language Version

python 3.10

Other information

No response

@lulala1126 lulala1126 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 13, 2024
@github-actions github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Aug 13, 2024
@pahud pahud self-assigned this Aug 13, 2024
@pahud
Copy link
Contributor

pahud commented Aug 13, 2024

Thank you for your insight and suggested solutions.

I am bringing this up to the team for inputs.

@pahud pahud added feature-request A feature should be added or improved. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 13, 2024
@pahud pahud removed their assignment Aug 13, 2024
@pahud pahud added effort/medium Medium work item – several days of effort p2 labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

2 participants