json-output: Extended detail for unknown outputs

[alisdair]

Planned output changes are represented in the JSON output format using the same change object as planned resource changes. This structure includes an after value and a parallel after_unknown value, which can be combined to determine which specific parts of a value are known only at apply time.

Previously, structured output values would be marked in the JSON plan as coarsely known or unknown, even if only some subset of the structure will be known only at apply time. This simplification was unnecessary, and this commit reuses the same logic for resource changes to give more information to consumers of this format.

For example, consider this output:

output "bar" {
  value = tolist([
    "hello",
    timestamp(),
    "world",
  ])
}

The plan UI for this output would be:

+ bar = [
    + "hello",
    + (known after apply),
    + "world",
  ]

For the same plan, the JSON output was previously:

"bar": {
  "actions": [
    "create"
  ],
  "before": null,
  "after_unknown": true,
  "before_sensitive": false,
  "after_sensitive": false
}

After this commit, the output is instead:

"bar": {
  "actions": [
    "create"
  ],
  "before": null,
  "after": [
    "hello",
    null,
    "world"
  ],
  "after_unknown": [
    false,
    true,
    false
  ],
  "before_sensitive": false,
  "after_sensitive": false
}

[cam-stitt]

Looks good to me.

[apparentlymart]

This seems reasonable to me!

I notice that our own terraform-json implementation was already sharing the decoding code between the resource unknown properties and the output unknown properties anyway, and so should be improved rather than hurt by this change.

I also see that our documentation says that this should behave the way you made it behave here, rather than how it behaved before, and so this is defensible as a bug to be fixed per the statement in our v1.0 Compatibility Promises:

If the actual behavior of a feature differs from what we explicitly documented as the feature's behavior, we will usually treat that as a bug and change the feature to match the documentation, although we will avoid making such changes if they seem likely to cause broad compatibility problems. We cannot promise to always remain "bug-compatible" with previous releases, but we will consider such fixes carefully to minimize their impact.

I think our due diligence here then is to convince ourselves that changing this isn't likely to cause broad compatibility problems. The fact that terraform-json was acting as a client of what we documented even though the implementation doesn't match does seem like evidence of that -- third-party client implementations against the documented protocol should hopefully be behaving similarly to terraform-json and accepting it both ways.

Given that, this feels justified to me but let's be sure to include something explicit about it in the upgrade notes in the changelog and in our v1.3 Upgrade Guide when we get closer to release.

[alisdair]

I think our due diligence here then is to convince ourselves that changing this isn't likely to cause broad compatibility problems.

I poked around a bit trying to test how this might affect Sentinel, and found that the change representation seems to pass through the JSON plan data without interpretation. That means that it's down to the individual policies' interpretation of this data, and given that the data essentially wasn't available before, I can't see how we could be causing compatibility problems.

[apparentlymart]

Thanks for checking into that!

Our intended meaning of "broad" in that statement was to mean issues affecting a great number of different users or use-cases for a particular customer, and so I think individual Sentinel policies written against the buggy implementation rather than against the specification are unlikely to be numerous enough to meet that threshold, if there are any at all.

With that said, I wonder if we could help to absorb any individual impacts of that by adding something to the Terraform-specific Sentinel API which can be a drop-in replacement for the question of "is this output value wholly known?", which I assume would do something like scan the data structure and return true only if there are no true values anywhere inside the data structure. Then if anyone does run into the problem, we could hopefully advise them to drop in that function in place of their direct comparison with true and thus quickly restore their original intended behavior. Does that seem reasonable? (I guess this is more a question for @cam-stitt than @alisdair, since it's a question about what might might be feasible to do inside the Sentinel plugin for Terraform, rather than here in this codebase.)

[alisdair]

After trying to write some policies using the unknown structure for resources, I think more advanced support for the structure might be useful, not just for outputs. As you noted that's a prioritization issue for the Sentinel plugin maintainers, and since there's approval on this enhancement I'm merging for now. Thanks for the thoughts!

[github-actions]

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.