Add sensitivity and required-replace fields to plan file, and expose sensitivity to JSON plan #28201
Conversation
The stored planfile now serializes the required-replace path set and the collection of before/after sensitivity marks. This ensures that storing a plan and displaying it with `terraform show` renders the same output for plans with required-replace resources, and those with sensitive values in the diff.
Codecov Report
|
There was a problem hiding this comment.
Hey @alisdair, this looks like a great start.
One thing we specifically need in Sentinel is this detail propagated to variables and outputs in the plan as well. This does not seem to be happening when it comes to the fixtures - can you check? I'm specifically looking at https://github.com/hashicorp/terraform/pull/28201/files#diff-0ce6a3851c6b35d990b314c3995e5e560ba42e9f923c1c808d4957e7144763f0 and the values do not seem to be propagated there, either in the top-level variables object nor as [before|after])_sensitive in output_changes. For the former I'd recommend just a simple sensitive attribute as that field is not a change, since it does not have state.
Additionally, I'm curious what a "required-replace" field is - is that an updated name for ForceNew? If so, we don't necessarily need it in the JSON right now, as that field is satisfied with the action order, but it'd be nice as a quality-of-life thing, so long as it's consistent with the aforementioned.
|
@alisdair additionally, what do you think the possibility is to be able to indicate in at least |
|
Thanks for the review, @vancluever!
That's correct for variables: adding the Outputs are different from resource changes, and more similar to variables. Unlike other language values, Terraform does not consider an output to be partially sensitive: it's either sensitive or not, depending on the
Yes, Since I don't currently have any idea how to usefully represent a path set in JSON plan output, and it doesn't sound like it's urgent, I'm inclined not to block this change on figuring that out. I'd be happy to revisit it later though.
Terraform doesn't know anything about sensitivity of variables except via the configuration. For TFC sensitive variables which are not marked as sensitive in the ingressed configuration, TFC uses an override file to add the I hope all that makes sense! Let me know if I missed something or if you have any follow-up questions. |
Thinking on this a bit more, and if this is something that is easy to surface into FWIW:
That's basically the set of actions to be carried about by any particular change - ie: replace would be It's not a big deal if this is not included for now as it would mainly be redundant anyway. AFAIK this is not causing a DX issue in Sentinel, and we can always add convenience helpers if it does become one.
Alright, that sounds good. We can probably cross the bridge if need be if that ever gets changed with any upcoming architectural changes to de-couple CLI and core (any news on that btw? 🙂 ) |
Similar to `after_unknown`, `before_sensitive` and `after_sensitive` are values with similar structure to `before` and `after` which encode the presence of sensitive values in a planned change. These should be used to obscure sensitive values from human-readable output. These values follow the same structure as the `before` and `after` values, replacing sensitive values with `true`, and non-sensitive values with `false`. Following the `after_unknown` precedent, we omit non-sensitive `false` values for object attributes/map values, to make serialization more compact. One difference from `after_unknown` is that a sensitive complex value (collection or structural type) is replaced with `true`. If the complex value itself is sensitive, all of its contents should be obscured.
When an output value changes, we have a small amount of information we can convey about its sensitivity. If either the output was previously marked sensitive, or is currently marked sensitive in the config, this is tracked in the output change data. This commit encodes this boolean in the change struct's `before_sensitive` and `after_sensitive` fields, in the a way which matches resource value sensitivity. Since we have so little information to work with, these two values will always be booleans, and always equal each. This is logically consistent with how else we want to obscure sensitive data: a changing output which was or is marked sensitive should not have the value shown in human-readable output.
6068f60
to
5e30d58
Compare
Ah, I was missing this context, thank you! This suggestion makes sense to me, and I've updated terraform/plans/changes_src.go Lines 109 to 113 in 1577901 |
Yeah, that's fine and how the unknown values behave right now from the looks of it, so it's consistent. Specifically referring to the opaque values here, but before and after being the same is fine here too I think as I'm not too sure if there will be concern with relying on one over the other. If there is, we can cross that bridge when we get to it! |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
plans/planfile: Add required-replace and sensitiveThe stored planfile now serializes the required-replace path set and the collection of before/after sensitivity marks. This ensures that storing a plan and displaying it with
terraform showrenders the same output for plans with required-replace resources, and those with sensitive values in the diff.command/jsonplan: Add sensitive value mapping dataSimilar to
after_unknown,before_sensitiveandafter_sensitiveare values with similar structure tobeforeandafterwhich encode the presence of sensitive values in a planned change. These should be used to obscure sensitive values from human-readable output.These values follow the same structure as the
beforeandaftervalues, replacing sensitive values withtrue, and non-sensitive values withfalse. Following theafter_unknownprecedent, we omit non-sensitivefalsevalues for object attributes/map values, to make serialization more compact.One difference from
after_unknownis that a sensitive complex value (collection or structural type) is replaced withtrue. If the complex value itself is sensitive, all of its contents should be obscured.command/jsonconfig: Add variable sensitive flagPrompts for reviewers:
Screenshots and sample JSON
Before
After
JSON output
The JSON below is for the plan in the above screenshots. Here's the relevant
resource_changesvalue:[ { "address": "random_pet.pet", "mode": "managed", "type": "random_pet", "name": "pet", "provider_name": "registry.terraform.io/hashicorp/random", "change": { "actions": [ "delete", "create" ], "before": { "id": "foo-firm-eagle", "keepers": null, "length": 2, "prefix": "foo", "separator": "-" }, "after": { "keepers": { "foo": "foo" }, "length": 2, "prefix": "foo", "separator": "-" }, "after_unknown": { "id": true, "keepers": {} }, "before_sensitive": {}, "after_sensitive": { "keepers": { "foo": true }, "prefix": true } } } ]Entire JSON plan, which is quite long, so hidden in this description by default
{ "format_version": "0.1", "terraform_version": "0.15.0-dev", "variables": { "foo": { "value": "foo" } }, "planned_values": { "outputs": { "pet": { "sensitive": true } }, "root_module": { "resources": [ { "address": "random_pet.pet", "mode": "managed", "type": "random_pet", "name": "pet", "provider_name": "registry.terraform.io/hashicorp/random", "schema_version": 0, "values": { "keepers": { "foo": "foo" }, "length": 2, "prefix": "foo", "separator": "-" } } ] } }, "resource_changes": [ { "address": "random_pet.pet", "mode": "managed", "type": "random_pet", "name": "pet", "provider_name": "registry.terraform.io/hashicorp/random", "change": { "actions": [ "delete", "create" ], "before": { "id": "foo-firm-eagle", "keepers": null, "length": 2, "prefix": "foo", "separator": "-" }, "after": { "keepers": { "foo": "foo" }, "length": 2, "prefix": "foo", "separator": "-" }, "after_unknown": { "id": true, "keepers": {} }, "before_sensitive": {}, "after_sensitive": { "keepers": { "foo": true }, "prefix": true } } } ], "output_changes": { "pet": { "actions": [ "update" ], "before": "foo-firm-eagle", "after_unknown": true } }, "prior_state": { "format_version": "0.1", "terraform_version": "0.15.0", "values": { "outputs": { "pet": { "sensitive": false, "value": "foo-firm-eagle" } }, "root_module": { "resources": [ { "address": "random_pet.pet", "mode": "managed", "type": "random_pet", "name": "pet", "provider_name": "registry.terraform.io/hashicorp/random", "schema_version": 0, "values": { "id": "foo-firm-eagle", "keepers": null, "length": 2, "prefix": "foo", "separator": "-" } } ] } } }, "configuration": { "root_module": { "outputs": { "pet": { "sensitive": true, "expression": { "references": [ "random_pet.pet" ] } } }, "resources": [ { "address": "random_pet.pet", "mode": "managed", "type": "random_pet", "name": "pet", "provider_config_key": "random", "expressions": { "keepers": { "references": [ "var.foo" ] }, "prefix": { "references": [ "var.foo" ] } }, "schema_version": 0 } ], "variables": { "foo": { "default": "foo" } } } } }Fixes #27577.