JSON diffs don't pretty print for nested properties

[jaxxstorm]

What happened?

We made some changes in #5831 whereby we pretty print JSON diffs for things like IAM roles to help with debugging.

However, this doesn't appear to work for nested properties, specifically for an AWS ECS Task Definition with a nested container definition, see the attached screenshot

Example

**

**

Output of pulumi about

N/A

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

[gunzy83]

This is still happening for IAM Policy objects on update as well:

This diff was far worse before I ran the generated policy document through JSON.stringify(JSON.parse(policyDoc), null, 0) to minify it.

Create operations seem to identify the input as JSON:

I think this might have something to do with how the JSON is stored in state which prevents the JSON diff from happening, we get a plaintext diff instead.

Both of the diffs above are running through the same codepaths in the same project/stack.

Pulumi version: 3.60.1
@pulumi/aws: 5.31.0

[gunzy83]

I believe this change #11803 introduced a bug where the json diff is never reached because the string comparison before it will cause it to always print the string diff.

Someone who is less green with go can correct me if I am wrong but if I am understanding this correctly, it might make more sense to implement this later in the function:

if diff == nil {
    if old != new {
        p.printTextDiff(strconv.Quote(old), strconv.Quote(new))
        return true
    }
    p.withOp(deploy.OpSame).printPropertyValue(oldValue)
    return true
}

[Frassle]

I don't think that's a bug, looks like #11803 is doing what it was intended to do. There is no semantic JSON diff between the text strings so it shows the text diff instead.
It looks like that's the same for Lee's example in the initial message here as well.

This behaviour was suggested by @lukehoban at #11799 (comment) because the original behaviour was confusing but clearly this current style is also confusing just in a different way.

Maybe something as simple as prefixing it with "(json equivalent, showing text diff)" would help?

[gunzy83]

I have added more to my comment above, maybe I am misunderstanding where the json diff is being done, to me it looks like the text diff is being done before the json diff. See my screenshot below for what I mean:

The json is clearly not equivalent, I added an extra character only :)

[gunzy83]

This behaviour was suggested by @lukehoban at #11799 (comment) because the original behaviour was confusing but clearly this current style is also confusing just in a different way.

I just read that thread, I agree with the logic. I am just not convinced the json diff is getting printed with priority which makes the previews we get from this for large policy changes next to useless.

Maybe something as simple as prefixing it with "(json equivalent, showing text diff)" would help?

I think that would help in the case it is just a diff in the input vs stored text in state but I think we are talking about different things here?

[Frassle]

The json is clearly not equivalent, I added an extra character only :)

Yeh ok that's clearly bugged then. I think your assessment that #11803 broke it is correct.

[D3-LucaPiombino]

We are hitting a similar issue.
We have a lot of json resources (e.g. in config maps), and diffs are very difficult to parse at the moment.
It seems to works (recognize the string as (json)) on creation or for example if the whole string is changed (e.g. changing from secret to non secret)

Is there any plan to pick this up/revisit?

Sorry, i did an attempt to see if i could contribute (instead of simply complaining :)) by playing around a little bit with the codebase.
Unfortunately i did not have much success as i am not very familiar with the go ecosystem/stack.