fix(apigateway): stack deployment fails when a Stage is explicitly sp…

…ecified (aws#6165) Per the documentation from CloudFormation, when an explicit AWS::ApiGateway::Deployment resource is specified, it needs to contain an explicit 'DependsOn' field declared to all of the AWS::ApiGateway::Method resources that are part of the RestAPI. Doc - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-deployment.html#aws-resource-apigateway-deployment--example fixes aws#6068 Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
atsushi-ishibashi · Feb 7, 2020 · 879601e · 879601e
1 parent dc2ec38
commit 879601e
Show file tree

Hide file tree

Showing 5 changed files with 169 additions and 95 deletions.
diff --git a/packages/@aws-cdk/aws-apigateway/lib/deployment.ts b/packages/@aws-cdk/aws-apigateway/lib/deployment.ts
@@ -1,7 +1,7 @@
-import { Construct, Lazy, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
+import { CfnResource, Construct, Lazy, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
 import * as crypto from 'crypto';
 import { CfnDeployment, CfnDeploymentProps } from './apigateway.generated';
-import { IRestApi } from './restapi';
+import { IRestApi, RestApi } from './restapi';
 
 export interface DeploymentProps  {
   /**
@@ -90,6 +90,30 @@ export class Deployment extends Resource {
   public addToLogicalId(data: any) {
     this.resource.addToLogicalId(data);
   }
+
+  /**
+   * Hook into synthesis before it occurs and make any final adjustments.
+   */
+  protected prepare() {
+    if (this.api instanceof RestApi) {
+      // Ignore IRestApi that are imported
+
+      /*
+       * https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-deployment.html
+       * Quoting from CloudFormation's docs - "If you create an AWS::ApiGateway::RestApi resource and its methods (using AWS::ApiGateway::Method) in
+       * the same template as your deployment, the deployment must depend on the RestApi's methods. To create a dependency, add a DependsOn attribute
+       * to the deployment. If you don't, AWS CloudFormation creates the deployment right after it creates the RestApi resource that doesn't contain
+       * any methods, and AWS CloudFormation encounters the following error: The REST API doesn't contain any methods."
+       */
+
+      /*
+       * Adding a dependency between LatestDeployment and Method construct, using ConstructNode.addDependencies(), creates additional dependencies
+       * between AWS::ApiGateway::Deployment and the AWS::Lambda::Permission nodes (children under Method), causing cyclic dependency errors. Hence,
+       * falling back to declaring dependencies between the underlying CfnResources.
+       */
+      this.api.methods.map(m => m.node.defaultChild as CfnResource).forEach(m => this.resource.addDependsOn(m));
+    }
+  }
 }
 
 class LatestDeploymentResource extends CfnDeployment {

diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts
@@ -212,7 +212,11 @@ export class RestApi extends Resource implements IRestApi {
    */
   public deploymentStage!: Stage;
 
-  private readonly methods = new Array<Method>();
+  /**
+   * The list of methods bound to this RestApi
+   */
+  public readonly methods = new Array<Method>();
+
   private _domainName?: DomainName;
   private _latestDeployment: Deployment | undefined;
 

diff --git a/...ges/@aws-cdk/aws-apigateway/test/integ.lambda-api.latebound-deploymentstage.expected.json b/...ges/@aws-cdk/aws-apigateway/test/integ.lambda-api.latebound-deploymentstage.expected.json
@@ -361,7 +361,11 @@
         "RestApiId": {
           "Ref": "lambdarestapiF559E4F2"
         }
-      }
+      },
+      "DependsOn": [
+        "lambdarestapiproxyANYC900233F",
+        "lambdarestapiANYB9BB3FB2"
+      ]
     },
     "stage0661E4AC": {
       "Type": "AWS::ApiGateway::Stage",

diff --git a/packages/@aws-cdk/aws-apigateway/test/test.deployment.ts b/packages/@aws-cdk/aws-apigateway/test/test.deployment.ts
@@ -19,37 +19,40 @@ export = {
     expect(stack).toMatch({
       Resources: {
         apiGETECF0BD67: {
-        Type: "AWS::ApiGateway::Method",
-        Properties: {
-          HttpMethod: "GET",
-          ResourceId: {
-          "Fn::GetAtt": [
-            "apiC8550315",
-            "RootResourceId"
-          ]
-          },
-          RestApiId: {
-          Ref: "apiC8550315"
-          },
-          AuthorizationType: "NONE",
-          Integration: {
-          Type: "MOCK"
+          Type: "AWS::ApiGateway::Method",
+          Properties: {
+            HttpMethod: "GET",
+            ResourceId: {
+              "Fn::GetAtt": [
+                "apiC8550315",
+                "RootResourceId"
+              ]
+            },
+            RestApiId: {
+              Ref: "apiC8550315"
+            },
+            AuthorizationType: "NONE",
+            Integration: {
+              Type: "MOCK"
+            }
           }
-        }
         },
         apiC8550315: {
-        Type: "AWS::ApiGateway::RestApi",
-        Properties: {
-          Name: "api"
-        }
+          Type: "AWS::ApiGateway::RestApi",
+          Properties: {
+            Name: "api"
+          }
         },
         deployment33381975: {
-        Type: "AWS::ApiGateway::Deployment",
-        Properties: {
-          RestApiId: {
-          Ref: "apiC8550315"
-          }
-        }
+          Type: "AWS::ApiGateway::Deployment",
+          Properties: {
+            RestApiId: {
+              Ref: "apiC8550315"
+            }
+          },
+          DependsOn: [
+            "apiGETECF0BD67"
+          ]
         }
       }
     });
@@ -70,39 +73,42 @@ export = {
     expect(stack).toMatch({
       Resources: {
         apiGETECF0BD67: {
-        Type: "AWS::ApiGateway::Method",
-        Properties: {
-          HttpMethod: "GET",
-          ResourceId: {
-          "Fn::GetAtt": [
-            "apiC8550315",
-            "RootResourceId"
-          ]
-          },
-          RestApiId: {
-          Ref: "apiC8550315"
-          },
-          AuthorizationType: "NONE",
-          Integration: {
-          Type: "MOCK"
+          Type: "AWS::ApiGateway::Method",
+          Properties: {
+            HttpMethod: "GET",
+            ResourceId: {
+              "Fn::GetAtt": [
+                "apiC8550315",
+                "RootResourceId"
+              ]
+            },
+            RestApiId: {
+              Ref: "apiC8550315"
+            },
+            AuthorizationType: "NONE",
+            Integration: {
+              Type: "MOCK"
+            }
           }
-        }
         },
         apiC8550315: {
-        Type: "AWS::ApiGateway::RestApi",
-        Properties: {
-          Name: "api"
-        }
-        },
-        deployment33381975: {
-        Type: "AWS::ApiGateway::Deployment",
-        Properties: {
-          RestApiId: {
-          Ref: "apiC8550315"
+          Type: "AWS::ApiGateway::RestApi",
+          Properties: {
+            Name: "api"
           }
         },
-        DeletionPolicy: "Retain",
-        UpdateReplacePolicy: "Retain"
+        deployment33381975: {
+          Type: "AWS::ApiGateway::Deployment",
+          Properties: {
+            RestApiId: {
+            Ref: "apiC8550315"
+            }
+          },
+          DeletionPolicy: "Retain",
+          UpdateReplacePolicy: "Retain",
+          DependsOn: [
+            "apiGETECF0BD67"
+          ]
         }
       }
     });
@@ -172,7 +178,10 @@ export = {
     deployment.node.addDependency(dep);
 
     expect(stack).to(haveResource('AWS::ApiGateway::Deployment', {
-      DependsOn: [ "MyResource" ],
+      DependsOn: [
+        "apiGETECF0BD67",
+        "MyResource"
+      ],
     }, ResourcePart.CompleteDefinition));
 
     test.done();
@@ -210,6 +219,36 @@ export = {
     expect(stack2).to(haveResource('AWS::ApiGateway::Stage', {
       DeploymentId: { Ref: 'myapiDeploymentB7EF8EB77c517352b0f7ab73c333e36585c8f1f3' }
     }));
+    test.done();
+  },
+
+  'deployment resource depends on all restapi methods defined'(test: Test) {
+    const stack = new Stack();
+    const restapi = new apigateway.RestApi(stack, 'myapi', {
+      deploy: false
+    });
+    restapi.root.addMethod('GET');
+
+    const deployment = new apigateway.Deployment(stack, 'mydeployment', {
+      api: restapi
+    });
+    const stage = new apigateway.Stage(stack, 'mystage', {
+      deployment
+    });
+    restapi.deploymentStage = stage;
+
+    restapi.root.addMethod('POST');
+    const resource = restapi.root.addResource('myresource');
+    resource.addMethod('GET');
+
+    expect(stack).to(haveResource('AWS::ApiGateway::Deployment', {
+      DependsOn: [
+        'myapiGET9B7CD29E',
+        'myapimyresourceGET732851A5',
+        'myapiPOST23417BD2'
+      ]
+    }, ResourcePart.CompleteDefinition));
+
     test.done();
   }
 };
diff --git a/packages/@aws-cdk/aws-apigateway/test/test.stage.ts b/packages/@aws-cdk/aws-apigateway/test/test.stage.ts
@@ -18,49 +18,52 @@ export = {
     expect(stack).toMatch({
       Resources: {
         testapiD6451F70: {
-        Type: "AWS::ApiGateway::RestApi",
-        Properties: {
-          Name: "test-api"
-        }
+          Type: "AWS::ApiGateway::RestApi",
+          Properties: {
+            Name: "test-api"
+          }
         },
         testapiGETD8DE4ED1: {
-        Type: "AWS::ApiGateway::Method",
-        Properties: {
-          HttpMethod: "GET",
-          ResourceId: {
-          "Fn::GetAtt": [
-            "testapiD6451F70",
-            "RootResourceId"
-          ]
-          },
-          RestApiId: {
-          Ref: "testapiD6451F70"
-          },
-          AuthorizationType: "NONE",
-          Integration: {
-          Type: "MOCK"
+          Type: "AWS::ApiGateway::Method",
+          Properties: {
+            HttpMethod: "GET",
+            ResourceId: {
+              "Fn::GetAtt": [
+                "testapiD6451F70",
+                "RootResourceId"
+              ]
+            },
+            RestApiId: {
+              Ref: "testapiD6451F70"
+            },
+            AuthorizationType: "NONE",
+            Integration: {
+              Type: "MOCK"
+            }
           }
-        }
         },
         mydeployment71ED3B4B: {
-        Type: "AWS::ApiGateway::Deployment",
-        Properties: {
-          RestApiId: {
-          Ref: "testapiD6451F70"
-          }
-        }
+          Type: "AWS::ApiGateway::Deployment",
+          Properties: {
+            RestApiId: {
+              Ref: "testapiD6451F70"
+            }
+          },
+          DependsOn: [
+            "testapiGETD8DE4ED1"
+          ]
         },
         mystage7483BE9A: {
-        Type: "AWS::ApiGateway::Stage",
-        Properties: {
-          RestApiId: {
-          Ref: "testapiD6451F70"
-          },
-          DeploymentId: {
-          Ref: "mydeployment71ED3B4B"
-          },
-          StageName: "prod"
-        }
+          Type: "AWS::ApiGateway::Stage",
+          Properties: {
+            RestApiId: {
+              Ref: "testapiD6451F70"
+            },
+            DeploymentId: {
+              Ref: "mydeployment71ED3B4B"
+            },
+            StageName: "prod"
+          }
         }
       }
     });