Skip to content

jsonplan and jsonstate: include sensitive_values in state representations#28889

Merged
mildwonkey merged 2 commits intomainfrom
mildwonkey/show-state-sensitive
Jun 14, 2021
Merged

jsonplan and jsonstate: include sensitive_values in state representations#28889
mildwonkey merged 2 commits intomainfrom
mildwonkey/show-state-sensitive

Conversation

@mildwonkey
Copy link
Copy Markdown
Contributor

A sensitive_values field has been added to the resource in state and planned values which is a map of all sensitive attributes with the values set to true. To achieve this, I stole and exported the sensitiveAsBool function that was in jsonplan (it had to be moved to jsonstate to avoid import cycles).

It wasn't entirely clear to me if the values in state would suffice, or if we also need to consult the schema - I believe that this is sufficient for state files written since v0.15, and if that's incorrect or insufficient, I'll add in the provider schema step as well.

I also updated the documentation, and, since we've considered this before, bumped the FormatVersions for both jsonstate and jsonplan.

@mildwonkey mildwonkey requested a review from a team June 7, 2021 17:20
@mwhooker
Copy link
Copy Markdown
Contributor

mwhooker commented Jun 7, 2021

did you read my mind?


// SensitiveValues is similar to AttributeValues, but with all sensitive
// values replaced with true, and all non-sensitive leaf values omitted.
SensitiveValues map[string]bool `json:"sensitive_values,omitempty"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using a map[string]bool here I think we reduce the granularity of sensitivity when compared to the precise level used in change representation (which uses a json.RawMessage for BeforeSensitive/AfterSensitive). This means that for the following config:

resource "null_resource" "none" {
  triggers = {
    boop = "beep"
    flap = sensitive("honk")
  }
}

the sensitive_values output would be {"triggers":true} rather than {"triggers":{"flap":true}}. As a result, a state renderer would have to hide the entire triggers attribute, rather than just the one sensitive member.

This is compatible with the JSON plan sensitivity output, just more coarse. Is that necessary or could we consider keeping the full sensitivity information?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 you're absolutely correct, that ain't right, I shall fix that.

…ions

A sensitive_values field has been added to the resource in state and planned values which is a map of all sensitive attributes with the values set to true.

It wasn't entirely clear to me if the values in state would suffice, or if we also need to consult the schema - I believe that this is sufficient for state files written since v0.15, and if that's incorrect or insufficient, I'll add in the provider schema check as well.

I also updated the documentation, and, since we've considered this before, bumped the FormatVersions for both jsonstate and jsonplan.
@mildwonkey mildwonkey force-pushed the mildwonkey/show-state-sensitive branch from dc5de76 to f18e035 Compare June 11, 2021 13:58
@mildwonkey mildwonkey requested review from alisdair and jbardin June 11, 2021 14:02
@mildwonkey mildwonkey merged commit ac03d35 into main Jun 14, 2021
mildwonkey added a commit that referenced this pull request Jun 14, 2021
…ions (#28889)

* jsonplan and jsonstate: include sensitive_values in state representations

A sensitive_values field has been added to the resource in state and planned values which is a map of all sensitive attributes with the values set to true.

It wasn't entirely clear to me if the values in state would suffice, or if we also need to consult the schema - I believe that this is sufficient for state files written since v0.15, and if that's incorrect or insufficient, I'll add in the provider schema check as well.

I also updated the documentation, and, since we've considered this before, bumped the FormatVersions for both jsonstate and jsonplan.
alisdair pushed a commit that referenced this pull request Jun 18, 2021
…ions (#28889)

* jsonplan and jsonstate: include sensitive_values in state representations

A sensitive_values field has been added to the resource in state and planned values which is a map of all sensitive attributes with the values set to true.

It wasn't entirely clear to me if the values in state would suffice, or if we also need to consult the schema - I believe that this is sufficient for state files written since v0.15, and if that's incorrect or insufficient, I'll add in the provider schema check as well.

I also updated the documentation, and, since we've considered this before, bumped the FormatVersions for both jsonstate and jsonplan.
@mildwonkey mildwonkey deleted the mildwonkey/show-state-sensitive branch June 21, 2021 14:54
@github-actions
Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants