aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.65k stars 3.91k forks source link

The RestApi construct in @aws-cdk/aws-apigateway could be more useful for split stacks #20231

Open dls314 opened 2 years ago

dls314 commented 2 years ago

Describe the feature

The RestApi construct in @aws-cdk/aws-apigateway could be more useful for split stacks if it had relaxed validation when deploy: false is specified in its RestApiProps.

The split-stack technique (https://docs.aws.amazon.com/cdk/api/v1/docs/aws-apigateway-readme.html#breaking-up-methods-and-resources-across-stacks) is very useful when managing an API Gateway, but to use the split-stack technique with the RestApi construct, at least one method must be added to the RestApi in the stack where it is defined; however, in the split-stack technique, that stack usually doesn't define any actual methods, so the method becomes a throw-away.

For example, from the docs example

    const restApi = new RestApi(this, 'RestApi', {
      deploy: false,
    });
    restApi.root.addMethod('ANY');

The method is being added to pass validation, here: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-apigateway/lib/restapi.ts#L831-L840; however, validation is only critical if there is a deployment.

It would be nice if I could get the benefits of using the RestApi construct while using the split-stack technique without having to create a throw-away method in that root stack.

Use Case

I use the split-stack technique in combination with a Serverless Framework project that defines the stage, where that Serverless Framework project imports the API Gateway Rest API (these stacks are not nested). This technique is documented by the Serverless Framework, here: https://www.serverless.com/framework/docs/providers/aws/events/apigateway#share-api-gateway-and-api-resources

I would like to use the CDK to manage the API Gateway Rest API, and other configuration at that layer, and I would like to use the RestApi construct instead of the CfnRestApi class to do so.

But I'm frustrated that I have a throw-away method defined and deployed that could contribute to stack drift once the real method definitions are applied by deploying the Serverless Framework project.

Proposed Solution

At https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-apigateway/lib/restapi.ts#L831-L840

I think that the change could be from this current check

  /**
   * Performs validation of the REST API.
   */
  protected validate() {
    if (this.methods.length === 0) {
      return ["The REST API doesn't contain any methods"];
    }

    return [];
  }

to a check that invalidates only if there is a deployment

  /**
   * Performs validation of the REST API.
   */
  protected validate() {
    if (this.methods.length === 0 && this.latestDeployment !== undefined) {
      return ["The REST API doesn't contain any methods"];
    }

    return [];
  }

Other Information

No response

Acknowledgements

CDK version used

1.155.0

Environment details (OS name and version, etc.)

OS and version independent

dls314 commented 2 years ago

This work around also works for me

import { RestApi, RestApiProps } from '@aws-cdk/aws-apigateway';
import { Construct } from '@aws-cdk/core';

export class CompatibleRestApi extends RestApi {
  constructor(scope: Construct, id: string, props: RestApiProps = {}) {
    super(scope, id, props);
  }

  /**
   * Performs validation of the REST API.
   */
  protected validate() {
    if (this.methods.length === 0 && this.latestDeployment !== undefined) {
      return ["The REST API doesn't contain any methods"];
    }

    return [];
  }
}
dls314 commented 2 years ago

I think that in CDK v2, this method is changed to be the private validateRestApi method and I'm not able to override it in this way.

Is there an answer that I can use to migrate to CDK v2?

I'd prefer not to duplicate the whole RestApi class; since RootResource is not exported, it gets kind of verbose

dls314 commented 2 years ago

[ ] This feature might incur a breaking change

I looked at contributing this change, but the original overly-tight validation in RestApi has leaked throughout the related CDK documentation and integration tests, so I've decided to fork for my use instead.

I still believe that this is a good change that the CDK should make to better support the split-stack use of RestApi -- but it's beyond my scope to contribute at this point.

gerrardcowburn commented 5 months ago

I agree it would be useful to relax this validation and/or have it apply only when a deployment is happening. Ideally it should be possible to create a RestApi with deploy: false and skip this validation for initial deployment. The base L2 construct allows you to apply parameters like defaultMethodOptions which then propagate to all future methods which is a very useful feature and would be included in a foundation stack in the use case above, but it should be possible to cdk synth/deploy this stack on its own pending dependent stacks being created.