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

NestedStack parent resource reference generates Circular Dependency #5888

Closed
xoadrian opened this issue Jan 21, 2020 · 2 comments · Fixed by #7187
Closed

NestedStack parent resource reference generates Circular Dependency #5888

xoadrian opened this issue Jan 21, 2020 · 2 comments · Fixed by #7187
Assignees
Labels
@aws-cdk/aws-cloudformation Related to AWS CloudFormation bug This issue is a bug. in-progress This issue is being actively worked on. p1

Comments

@xoadrian
Copy link

xoadrian commented Jan 21, 2020

I have the following stack structure:

Root(Stack) => AppSync(NestedStack) => Resolvers(NestedStack)

In AppSync(NestedStack) I am creating API, schema and instantiating nested stack for Resolvers, main reason for this this that there are many resolvers and to not hit 200 Resrouce limit I will be manually splitting resources between multiple Resolvers nested stack. Currently I don't know an automated way for splitting resources, if there is please, please share :).

I also tried changing the structure to have nested stacks as siblings, which get's rid of Circular dependency but then it just fails on deploy:

Root(Stack) => AppSync(NestedStack) => ApiStack(NestedStack)
                                    => Resolvers(NestedStack)
creating CloudFormation changeset...
 0/3 | 2:07:56 PM | CREATE_IN_PROGRESS   | AWS::CDK::Metadata         | CDKMetadata
 0/3 | 2:07:56 PM | CREATE_IN_PROGRESS   | AWS::CloudFormation::Stack | appsync-bizon.NestedStack/appsync-bizon.NestedStackResource (appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF)
 0/3 | 2:07:57 PM | CREATE_IN_PROGRESS   | AWS::CloudFormation::Stack | appsync-bizon.NestedStack/appsync-bizon.NestedStackResource (appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF) Resource creation Initiated
 0/3 | 2:07:58 PM | CREATE_IN_PROGRESS   | AWS::CDK::Metadata         | CDKMetadata Resource creation Initiated
 1/3 | 2:07:58 PM | CREATE_COMPLETE      | AWS::CDK::Metadata         | CDKMetadata
 2/3 | 2:08:19 PM | CREATE_FAILED        | AWS::CloudFormation::Stack | appsync-bizon.NestedStack/appsync-bizon.NestedStackResource (appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF) Embedded stack arn:aws:cloudformation:eu-west-2:452364846299:stack/ProductName-bizon-appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF-4NAE12ZLOQGT/a92a4b40-3c46-11ea-9fec-06452e71541c was not successfully created: The following resource(s) failed to create: [dynamodbstackbizonNestedStackdynamodbstackbizonNestedStackResource0AACEAF0, productnameresolversNestedStackproductnameresolversNestedStackResource42DF9B6A].

Reproduction Steps

cdk.ts:

new RootStack(app, getResourceName(PRODUCT_NAME), {
  tags: TAGS
});

root-stack.ts:

new AppSyncStack(this, getResourceName('appsync'));

appsync-stack.ts

  export class AppSyncStack extends cfn.NestedStack {
  public readonly api: CfnGraphQLApi;
  public readonly schema: CfnGraphQLSchema;

  constructor(scope: cdk.Construct, id: string, props?: cfn.NestedStackProps) {
    super(scope, id, props);

    this.api = this.create();
    this.schema = this.createSchema();
    this.createTable();
    this.createResolvers();
  }

  private create(): CfnGraphQLApi {
    const name = getResourceName(`${getProductName()}-appsync`);
    return new CfnGraphQLApi(this, name, {
      name,
      authenticationType: 'AMAZON_COGNITO_USER_POOLS',
      additionalAuthenticationProviders: [{
        authenticationType: 'API_KEY'
      }]
    });
  }

  private createSchema(): CfnGraphQLSchema {
    return new CfnGraphQLSchema(this, getResourceName(`${getProductName()}-schema`), {
      apiId: this.api.attrApiId,
      definition: fs.readFileSync(SCHEMA_FILE_PATH).toString()
    });
  }

  private createTable(): void {
    // tslint:disable-next-line:no-unused-expression
    new DynamoDbStack(this, getResourceName('dynamodb-stack'));
  }

  private createResolvers(): void {
    // tslint:disable-next-line:no-unused-expression
    new ResolversStack(this, getResourceName(`${getProductName()}-resolvers`),{
      parameters: {
        apiId: this.api.attrApiId
      }
    });
  }
}

resolvers-stack:

new CfnResolver(this, name, {
  typeName,
  fieldName,
  requestMappingTemplate,
  responseMappingTemplate,
  apiId: this.props?.parameters?.apiId || '',
});

Error Log

creating CloudFormation changeset...

 ❌  ProducName-bizon failed: ValidationError: Circular dependency between resources: [appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF]
Circular dependency between resources: [appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF]

CFN template generates reference to itself(root-stack.template):

"appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF": {
      "Type": "AWS::CloudFormation::Stack",
      "Properties": {
        ...
        },
        "Parameters": {
          "referencetoProductNamebizonappsyncbizonproductnameappsyncbizon6498FF74ApiId": {
            "Fn::GetAtt": [
              "appsyncbizonNestedStackappsyncbizonNestedStackResource814A53CF",
              "Outputs.ProuctNamebizonappsyncbizonproductnameappsyncbizon6498FF74ApiId"
            ]
          },

appsync-stack.template:

"productnameresolversbizonNestedStackproductnameresolversbizonNestedStackResource7E007811": {
      "Type": "AWS::CloudFormation::Stack",
      "Properties": {
        ...
        },
        "Parameters": {
          "apiId": {
            "Fn::GetAtt": [
              "productnameappsyncbizon",
              "ApiId"
            ]
          }
        }
      .....
      "Outputs": {
    "ProductNamebizonappsyncbizonprodcutnameappsyncbizon6498FF74ApiId": {
      "Value": {
        "Fn::GetAtt": [
          "productnameappsyncbizon",
          "ApiId"
        ]
      }
    }
  }

resolvers-stack.template:

{
  "Resources": {
    "AreaprimaryContact": {
      "Type": "AWS::AppSync::Resolver",
      "Properties": {
        "ApiId": {
          "Ref": "referencetoProductNamebizonappsyncbizonproductnameappsyncbizon6498FF74ApiId"
        },
        "FieldName": "primaryContact",
        "TypeName": "Area",
        "RequestMappingTemplate": "\n\n#set( $customerId = $context.identity.claims.get(\"custom:customer\") )\n#if( $util.isNullOrEmpty($customerId) )\n  #set( $customerId = \"mydev\" )\n#end\n    \n#set( $pk = \"Person#$customerId\" )\n#set( $sk = \"Person#$customerId#$ctx.source.ownerId\" )\n{\n    \"version\": \"2017-02-28\",\n    \"operation\": \"GetItem\",\n    \"key\": {\n        \"pk\": $util.dynamodb.toDynamoDBJson($pk),\n        \"sk\": $util.dynamodb.toDynamoDBJson($sk)\n    }\n}\n    ",
        "ResponseMappingTemplate": "\n$util.toJson($ctx.result)\n    "
      },
      "Metadata": {
        "aws:cdk:path": "ProductName-bizon/appsync-bizon/productname-resolvers-bizon/Area.primaryContact"
      }
    }
  }
}

Environment

  • CLI Version: 1.21.0
  • Framework Version:
  • OS: Windows
  • Language: Typescript

Other


This is 🐛 Bug Report

@xoadrian xoadrian added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 21, 2020
@xoadrian
Copy link
Author

@eladb Based on the history you are most related to NestedStack, maybe you can have a quick look over it? I have debugged it:

const factory = this.determineCrossReferenceFactory(targetStack);
// if one side is a nested stack (has "parentStack"), we let it create the reference
// since it has more knowledge about the world.
const consumedValue = factory.prepareCrossReference(this, reference);

In the above stack structure factory is same with target, so in the context of the above issue AppSyncStack is invoking prepareCrossReference and the target is also AppSyncStack

and it falls under the 1st condition from inside. Even if stacks are siblings it still doesn't work even though there is no Circular Dependency validation error in that case. The deploy just fails.

@SomayaB SomayaB added the @aws-cdk/aws-appsync Related to AWS AppSync label Jan 21, 2020
@SomayaB SomayaB assigned shivlaks and unassigned eladb Jan 21, 2020
@shivlaks
Copy link
Contributor

@xoadrian - thanks for reporting. I'll take a look later today and run through your reproduction steps

@shivlaks shivlaks added @aws-cdk/aws-cloudformation Related to AWS CloudFormation and removed @aws-cdk/aws-appsync Related to AWS AppSync labels Jan 24, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Jan 28, 2020
@eladb eladb added the p1 label Feb 3, 2020
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Apr 6, 2020
eladb pushed a commit that referenced this issue Apr 10, 2020
#7187)

Fixes #6473 by centralizing all the logic to resolve cross-references into `App.prepare()`. This allows reasoning about the entire application once from a single code flow, dramatically simplified the algorithm and solved the issue at hand (and probably a handful of other issues).

This logic is implemented in a function called `prepareApp` which is normally called from `App.prepare()` but if a `Stack` is created as a root (normally in unit tests, we invoke this logic from there to retain current behavior).

This algorithm takes care of both resolving cross references and add template assets for nested stacks. This is because assets are currently addressed using CFN parameters, which means that when we adding them to the parent of a nested stack, the parent is mutated, so we need to rectify references again. To make sure this is done correctly, we always create assets in DFS order.


Fixes #7059
Fixes #5888
@mergify mergify bot closed this as completed in #7187 Apr 16, 2020
mergify bot pushed a commit that referenced this issue Apr 16, 2020
#7187)

Fixes #6473 by centralizing the logic to resolve cross-references and prepare nested stack template assets in `App.prepare`, which has a global view of the app and is the last prepare to execute before synthesis. This dramatically simplified reference resolution and allows dealing with nested stack assets only after all cross references have been resolved (the root cause for #7059).

This logic is implemented in a function called `prepareApp` which is normally called from `App.prepare()` but if a `Stack` is created as a root (normally in unit tests, we invoke this logic from there to retain current behavior).

This algorithm takes care of both resolving cross references and add template assets for nested stacks. This is because assets are currently addressed using CFN parameters, which means that when we adding them to the parent of a nested stack, the parent is mutated, so we need to rectify references again. To make sure this is done correctly, we always create assets in DFS order.

All changes to the test snapshots stem from new asset IDs of nested stack templates, not the template themselves. The change is a result of the fact that the refactor caused the "Parameters" section to appear in a different place in the template, but the template itself is identical.

Fixes #7059 by first resolving all references in the app and only then calculating the hash of the nested stack templates for their assets.

Fixes #5888 but this was not verified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudformation Related to AWS CloudFormation bug This issue is a bug. in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants