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

aws-apigateway: Documentation lacks information about stack separation for RestApi and Resources #29690

Closed
engineer-taro opened this issue Apr 2, 2024 · 4 comments · Fixed by #29691
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway documentation This is a problem with documentation. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@engineer-taro
Copy link
Contributor

Describe the issue

Overview

In this documentation, I would like to note that when separating the stacks for RestApi and Resource, Deployment is not automatically created, and to avoid this issue, it is necessary to use Deployment.addToLogicalId().

Details

When a Resource is added to an IRestApi imported via RestApi.fromRestApiAttribute or RestApi.fromRestApiId, Deployment is not created, so the latest resource state is not associated with the API Gateway stage, which is a known issue.
This has been discussed in several issues, including the following:

We understand that it is not easy to automatically create Deployment, and there may be differing opinions on whether it should be fixed.
However, since there is no mention of this issue in the documentation, we feel that many developers who are separating stacks related to API Gateway may unintentionally create an environment where Deployment is not created.
Due to this issue, developers need to manually deploy to API Gateway after running cdk deploy.

While it is possible to share a workaround implementation, it is difficult to implement an automatic Deployment creation that detects changes in Resource without increasing complexity in the current configuration.

Therefore, we would like to propose adding a note to the documentation first.

Links

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_apigateway-readme.html#breaking-up-methods-and-resources-across-stacks

@engineer-taro engineer-taro added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Apr 2, 2024
@github-actions github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Apr 2, 2024
@tim-finnigan tim-finnigan self-assigned this Apr 2, 2024
@tim-finnigan tim-finnigan added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Apr 2, 2024
@tim-finnigan
Copy link

Thanks for creating the issue/PR. There is a note in https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_apigateway.Deployment.html that says:

Normally, you don't need to define deployments manually. The RestApi construct manages a Deployment resource that represents the latest model. It can be accessed through restApi.latestDeployment (unless deploy: false is set when defining the RestApi).

If you manually define this resource, you will need to know that since deployments are immutable, as long as the resource's logical ID doesn't change, the deployment will represent the snapshot in time in which the resource was created. This means that if you modify the RestApi model (i.e. add methods or resources), these changes will not be reflected unless a new deployment resource is created.

To achieve this behavior, the method addToLogicalId(data) can be used to augment the logical ID generated for the deployment resource such that it will include arbitrary data. This is done automatically for the restApi.latestDeployment deployment.

Also here is a link to another workaround in that latter issue you referenced.

Maybe further clarification is needed in the docs but I'm not sure about just linking #13526 in your PR.

@tim-finnigan tim-finnigan added p2 feature-request A feature should be added or improved. effort/small Small work item – less than a day of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Apr 2, 2024
@tim-finnigan tim-finnigan removed their assignment Apr 2, 2024
@engineer-taro
Copy link
Contributor Author

@tim-finnigan
Thank you, tim-finnigan.

Also here is a link to another workaround in that latter issue you referenced.

The code provided in the other workaround you suggested does not solve the problem I presented.
#29079 (comment)

With that code as well, no new Deployment is created on the 2nd and subsequent cdk deploy, so the same issue of the latest resources not being reflected would still occur.

Maybe further clarification is needed in the docs but I'm not sure about just linking #13526 in your PR.

Thank you. I also don't think just linking the issue being discussed is the optimal solution.
I would appreciate if you could provide your opinion on whether we should add more details to other issues or the documentation.

@mergify mergify bot closed this as completed in #29691 Apr 18, 2024
mergify bot pushed a commit that referenced this issue Apr 18, 2024
### Issue #29690

Closes #29690

### Reason for this change

Regarding the stack separation of RestApi and Resource, there is no documentation about the fact that Deployment is not automatically created. When I actually add resources to the code documented and try cdk deploy for the second time and beyond, a new deployment is not created, and the latest resources are not reflected.

### Description of changes

I added a note and related links to the documentation.

### Description of how you validated changes

Nothing. It is just to change the description.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https:/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

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

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway documentation This is a problem with documentation. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants