Skip to content

Commit

Permalink
fix(cfnspec): incorrectly handling array result from jsondiff (#23795)
Browse files Browse the repository at this point in the history
We are getting an error when updating the cfnspec:

```bash
Error: Unexpected array diff entry: [" "]
```

We are using `json-diff` to calculate diff entries, and expect that each entry is a 2-tuple array. Indeed, the [docs](https://github.com/andreyvit/json-diff#arrays) say this. Turns out, this is only true when supplying the `--full` option.
With `--full`, the output for equivalent entries is `[' ', 1]`. Without `--full`, the output for equivalent entries is `[' ']`.

I considered that this might be a bug/breaking change in `json-diff`, but I did some spelunking and this is behavior that is codified in tests that haven't been changed in 11 years:  https://github.com/andreyvit/json-diff/blob/35582a9d19f8b0b2773360d67937e57ce2866781/test/diff_test.coffee#L78

More likely, we were relying on an implicit default to `--full` that is now turned off, or we have never come across such an issue before (which feels unlikely). I don't think we actually want `--full`, because we want the difference and that's it. So I've instead handled this case that should be a one-off case.

I have run the update on my own machine and confirm it works as expected, and the output in `CHANGELOG.md` is correct.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

(cherry picked from commit 4a701f1)
  • Loading branch information
kaizencc authored and mergify[bot] committed Jan 23, 2023
1 parent d0a750b commit b1b8fb7
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion packages/@aws-cdk/cfnspec/build-tools/spec-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,10 @@ async function main() {
if (Array.isArray(update)) {
changes.push(`* ${namespace} ${prefix} (__changed__)`);
for (const entry of update) {
if (entry.length === 1 && entry[0] === ' ') {
// This means that this element of the array is unchanged
continue;
}
if (entry.length !== 2) {
throw new Error(`Unexpected array diff entry: ${JSON.stringify(entry)}`);
}
Expand All @@ -247,7 +251,7 @@ async function main() {
case '-':
throw new Error(`Something awkward happened: ${entry[1]} was deleted from ${namespace} ${prefix}!`);
case ' ':
// This entry is "context"
// This entry is "context"
break;
default:
throw new Error(`Unexpected array diff entry: ${JSON.stringify(entry)}`);
Expand Down

0 comments on commit b1b8fb7

Please sign in to comment.