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

[core] Removing implicit references breaks deployment #9672

Closed
asterikx opened this issue Aug 13, 2020 · 7 comments
Closed

[core] Removing implicit references breaks deployment #9672

asterikx opened this issue Aug 13, 2020 · 7 comments
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@asterikx
Copy link
Contributor

asterikx commented Aug 13, 2020

I'm using CDK Pipelines to deploy two stacks, an ApiStack defining an AppSync API and a WebStack defining a CloudFront distribution that makes the AppSync API available under a custom domain.
Since the WebStack depends on the ApiStack, CDK Pipelines updates the ApiStack before it updates the WebStack.

Now, I decided to remove the AppSync API from the distribution. After removing the graphQLApi from the WebStack, deployment fails with the CloudFormation error:

Export Staging-Api:StagingApiExportsOutputFnGetAttGraphQLApi6F81E747GraphQLUrl26BE4366 cannot be deleted as it is in use by Staging-Web

I think the error is due to the fact that CDK is more clever (or rather, has complete knowledge) compared to the CloudFormation actions.
Since the CDK knows, that ApiStack.graphQLApi is no longer used, it removes the export from the stack. When updating the ApiStack, however, CloudFormation is not aware that the WebStack (which is going to be updated in the next step) no longer imports the GraphQLApi and fails deployment.

I'm not sure how this kind of error can be fixed or prevented by the CDK ...

Reproduction Steps

Before change:

const apiStack = new ApiStack(app, 'ApiStack');
new WebStack(app, 'WebStack', {
  graphQLApi: apiStack.graphQLApi,
});
export class ApiStack extends cdk.Stack {
  public readonly graphQLApi: appsync.GraphQLApi;

  constructor(scope: cdk.Construct, id: string, props: ApiStackProps) {
    super(scope, id, props);

    this.graphQLApi = new appsync.GraphQLApi(this, 'GraphQLApi', {
      name: `GraphQLApi`,
      // ...
    });

    // ...
  }
}
interface WebStackProps extends cdk.StackProps {
  graphQLApi: appsync.GraphQLApi;
}

export class WebStack extends cdk.Stack {
  // ...

  constructor(scope: cdk.Construct, id: string, props: WebStackProps) {
    super(scope, id, props);

    const noHttps = cdk.Fn.select(1, cdk.Fn.split('//', graphQLApi.graphQlUrl));
    const graphQLApiDomain = cdk.Fn.select(0, cdk.Fn.split('/', noHttps));
    
    new cloudfront.CloudFrontWebDistribution(this, 'Distribution', {
      originConfigs: [
        {
          customOriginSource: {
            domainName: graphQLApiDomain,
          },
          behaviors: [
            {
              pathPattern: '/graphql',
              // ...
            }
          ]
        },
        // ...
      ],
      // ...
    });
  }
}

After change:

new ApiStack(app, 'ApiStack');
new WebStack(app, 'WebStack');
export class WebStack extends cdk.Stack {
  // ...

  constructor(scope: cdk.Construct, id: string, props: WebStackProps) {
    super(scope, id, props);
    
    new cloudfront.CloudFrontWebDistribution(this, 'Distribution', {
      originConfigs: [
        // CUSTOM ORIGIN FOR GRAPHQL API REMOVED
        // ...
      ],
      // ...
    });
  }

}

What did you expect to happen?

No error. The CodePipeline created by CDK Pipelines is able to deploy both stacks without error.

What actually happened?

Environment

  • CLI Version : 1.58.0
  • Framework Version: 1.58.0
  • Node.js Version: v12.16.1
  • OS : macOS
  • Language (Version): TypeScript (3.9.7)

This is 🐛 Bug Report

@asterikx asterikx added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 13, 2020
@asterikx asterikx changed the title Deploy fails after removing [cdk-pipelines] Deploy fails after removing passed-in prop from another stack Aug 13, 2020
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Aug 13, 2020
@asterikx asterikx changed the title [cdk-pipelines] Deploy fails after removing passed-in prop from another stack [pipelines] Deploy fails after removing passed-in prop from another stack Aug 13, 2020
@rix0rrr rix0rrr added @aws-cdk/core Related to core CDK functionality and removed @aws-cdk/pipelines CDK Pipelines library labels Aug 17, 2020
@rix0rrr rix0rrr changed the title [pipelines] Deploy fails after removing passed-in prop from another stack [core] Removing implicit references breaks deployment Aug 17, 2020
@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort p1 labels Aug 17, 2020
@eladb eladb removed their assignment Aug 17, 2020
@asterikx
Copy link
Contributor Author

asterikx commented Oct 8, 2020

How can this class of problems be resolved?

Just run into it again. My DatabaseStack uses the new ServerlessCluster which didn't expose the default secret until recently. So I had to provide it with a custom one that I can expose to the rest of my app. Now, the ServerlessCluster exposes the default secret.
After replacing my custom secret with the default one in all my stacks, I get the error:

Export Staging-Db:StagingDbExportsOutputRefDbSecret685A0FA55952A3B0 cannot be deleted as it is in use by Staging-Api and Staging-Auth

@asterikx
Copy link
Contributor Author

A blog post from @skinny85 on the subject: https://www.endoflineblog.com/cdk-tips-03-how-to-unblock-cross-stack-references

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Nov 6, 2020
@beatkyo
Copy link

beatkyo commented Dec 16, 2020

I think this problem has been raised before. As explained in the blog article you linked it can be handled with CfnOutputs.

But for us it is a real show stopper. It is tedious and error prone. The problem is even worse when cdk must replace a dependency. You often end up with changing IDs because the old and new resources must exists at the same time.

Even worse when you have a chain of dependencies like stack A depends on stack B which depends on stack C. Replacing a resource in C could break an output across the whole chain and you have to do the CfnOutput trick multiple times. I did, it's hard and time consuming.

I think cdk should be able to sequence partial updates that would lead to the final desired state without errors. Now I understand that this could lead to partial state not consistent with the definition due to rollbacks not being possible. But I would be glad to have it has an option.

At the very least some simple way of handling the CfnOutput trick could be a solution.

This is really hard to drop cdk because of this problem. Apart from that it is really a great tool which could help us a lot.

@ssaarela
Copy link

ssaarela commented Apr 1, 2021

We've had a lot of problems with this. When e.g. renaming resources or moving from a stack to another. We also have a lot of context-based deployment options and for a true/false option this causes problems both ways when toggling. When removing a resource, CfnOutput is required, but if it's toggled on again that CfnOutput fails as duplicate for the implicit output.

Would it be possible to allow the user to (optionally) control what resources are exposed as outputs? Something like myResource.makeOutput(<optional name>) that would override the implicit output?
Of course that would still require multiphase rollout for first deploying the new resource and then removing the old, but a simple thing like that would help a lot in this process.

@eladb
Copy link
Contributor

eladb commented Apr 1, 2021

I believe you should be able to use stack.exportValue(). See #12778

@rix0rrr can this be closed?

@ssaarela
Copy link

ssaarela commented Apr 6, 2021

I believe you should be able to use stack.exportValue(). See #12778

@rix0rrr can this be closed?

That seems to do the trick! Thank you for the hint.

@github-actions
Copy link

github-actions bot commented Aug 9, 2022

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

No branches or pull requests

7 participants