Nested stacks template will not be republished when a reference is added/updated

[NetaNir]

When a user creates a NestedStack that reference a property in a sibling NestedStack, e.g:

                            +--------+                 
                       +----| Parent |-------+         
                       |    +--------+       |         
                       |                     |         
                       |                     |         
                       v                     v  
                +------------+      +------------------+
                |  Nested1   |      |     Nested2      |
                |            |      |                  |
                | Producer   |      |     Consumer     |
                | SNSTopic   |      |   Ref:SNSTopic   |
                +------------+      +------------------+

The CDK will create an output of the property, the Topic in the picture, on the producer stack. And the corresponding parameter on the consumer stack.
The outputs and parameters are created in the prepare phase of the CDK. In this phase, prepare is called on every construct in the tree, from leaves to root, ensuring that when a construct is being "prepared" it has all the knowledge it needs since all of its children have already been "prepared".
Since sibling nested stacks does not have a parent-child dependency it is not guaranteed that the consumer stack will be prepared before the producer stack, the order is dependent on the order in which the children array is processed.

Nested stack templates are uploaded to S3 using the CDK assets mechanism. In order to avoid uploading an unchanged assets, a hash is calculate for each asset and is used as the object key in S3. The hash is calculated differently for every type of assets. For CloudFormation templates, the hash is calculated based on all the template elements, including outputs and parameters, ensuring that every change to the template will result in a new file uploaded to S3.
The nested stack template file hash is calculated in the stack prepare method:

    if (this.nestedStackParent) {
      // add the nested stack template as an asset
      const cfn = JSON.stringify(this._toCloudFormation());
      const templateHash = crypto.createHash('sha256').update(cfn).digest('hex');
      const parent = this.nestedStackParent;
      const templateLocation = parent.addFileAsset({
        packaging: FileAssetPackaging.FILE,
        sourceHash: templateHash,
        fileName: this.templateFile
      });

full code

In the consumer producer example, the outputs and parameters on the respective stacks will be created when prepare is called on the consumer stack (the consumer stack is the one that needs the reference). This means that if the producer stack is prepared before the consumer stack, when the hash of the producer template file is calculated, it will not include the output, which means that the updated template will not be uploaded to S3, resulting in deployment failure.
Simply put, the hash is not calculated on the final template.

Why didn't our integration tests catch this?

you don't need to read the bellow to fix the described issue, it's just interesting :)

To traverse the tree from leaves to root in the prepare phase we create the topological sorting of the tree using:

root.findAll(ConstrcutOrder.Preorder).reverse()

code

Which means that the children in each level of the tree are processed from right to left.
Given the way the children are stored on the parent node and get children() implementation, the right to left order of the children array is opposite to the order in which the children were added to the node(1), which means that this code:

const producer = new ProducerNestedStack(this, 'Nested1');
const consumer =  new ConsumerNestedStack(this, 'Nested2', producer.topic);

Will result in the consumer stack being "prepared" before the producer stack.
Luckily, although not required, in most cases when referencing a property between resources, the producer resource will be created (and added to the children array) before the consumer resource. When the producer is created before the consumer the current logic works.

The producer resource must exist when the reference is added to the consumer resource but this does not implies anything on the creations order, for example:

consumer =  new ConsumerNestedStack(this, 'Nested2');
producer = new ProducerNestedStack(this, 'Nested1');
consumer.topic = producer.topic;

So how did this suddenly break?

As apart of the fix in aws/constructs#23, the topological sorting was changed from:

root.findAll(ConstrcutOrder.Preorder).reverse()

To:

root.findAll(ConstrcutOrder.Postorder)

Resulting in the children array processing order to change from right->left to left->right, causing the producer to be prepared before the consumer, breaking(2) the nested stacks integ test.
Since this change in the constructs module broke the CDK integ test we reverted the change and kept the original order.

(1) We store the children in a map based on their id (string), and retrieve the actual constructs using Object.values(children) which returns string keys in the order in which they were added to the object (see reference)
(2) The integration test only fail because it was run as part of an integration test suite. The test suite that includes a test which produces the same template as the one produced by the test that fail, minus the outputs element. This resulted in the template with the outputs not being uploaded, failing the deployment. If the test was run independently, it would have passed :/ (AI, more extensive integ test for assets?)

[eladb]

Nice catch!