Allow for_each arguments containing sensitive values if they aren't keys #27247
Conversation
Codecov Report
|
pselle
force-pushed
the
pselle/for_each-sensitive
branch
2 times, most recently
from
c412cc5 to
4961109
Compare
pselle
force-pushed
the
pselle/for_each-sensitive
branch
from
4961109 to
2a292c0
Compare
| diags = diags.Append(&hcl.Diagnostic{ | ||
| Severity: hcl.DiagError, | ||
| Summary: "Invalid for_each argument", | ||
| Detail: "Sensitive variables, or values derived from sensitive variables, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.", |
There was a problem hiding this comment.
Could this "or derived from" be left to communicate the function-result situation? Unsure.
There was a problem hiding this comment.
I like that phrase, personally, and think it covers the function-result situation, but not in a "blocking merge" way!
There was a problem hiding this comment.
Should we use a term like "sensitive values" here? I think this diagnostic applies to all of sensitive variables, outputs, and attributes (with the experiment enabled).
There was a problem hiding this comment.
@alisdair excellent point, "values" is correct rather than variables! And thanks for the input @mildwonkey, I'll add it back
pselle
force-pushed
the
pselle/for_each-sensitive
branch
2 times, most recently
from
613896c to
cf64288
Compare
pselle
force-pushed
the
pselle/for_each-sensitive
branch
from
cf64288 to
58717d2
Compare
| diags = diags.Append(&hcl.Diagnostic{ | ||
| Severity: hcl.DiagError, | ||
| Summary: "Invalid for_each argument", | ||
| Detail: "Sensitive variables, or values derived from sensitive variables, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.", |
There was a problem hiding this comment.
I like that phrase, personally, and think it covers the function-result situation, but not in a "blocking merge" way!
| diags = diags.Append(&hcl.Diagnostic{ | ||
| Severity: hcl.DiagError, | ||
| Summary: "Invalid for_each argument", | ||
| Detail: "Sensitive variables, or values derived from sensitive variables, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.", |
There was a problem hiding this comment.
Should we use a term like "sensitive values" here? I think this diagnostic applies to all of sensitive variables, outputs, and attributes (with the experiment enabled).
| func markSafeLengthInt(val cty.Value) int { | ||
| v, _ := val.UnmarkDeep() | ||
| return v.LengthInt() | ||
| } |
There was a problem hiding this comment.
Not for this PR, but I'm kind of surprised that LengthInt can't be called on marked values. Do you think we could adjust it to do so in the future?
There was a problem hiding this comment.
Hmm. We could investigate it. There's at least a couple of Unmarking in the format code because of LengthInt as well, iirc, i honestly took it (LengthInt())
for granted as something that shouldn't change, but perhaps it should.
Rather than disallow values that have any marks as for_each arguments, this makes the check more nuanced to disallow cases where the whole value is marked (a whole map, or any set). This allows cases where a user may pass a map that has marked values, but the keys are not sensitive
pselle
force-pushed
the
pselle/for_each-sensitive
branch
from
58717d2 to
a0c0273
Compare
|
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. |
ghost
locked as resolved and limited conversation to collaborators
This adjusts the previous behavior that would disallow any value involving marks to
for_eachto be more nuanced, and allow maps and objects where the keys are unmarked strings to pass through this evaluation unscathed.If the keys of an object are marked, a [cty] panic will occur when evaluating the expression, but there's an HCL PR to turn that into an error.
This is related to #27239, but does not fix it, a function call is used in the expression, and function calls always result in marked values when passed any marked arguments (docs PR that clarifies this: #27280) currently, since no functions are marks-aware at present.
Also related to #27180 but does not fix it, for the same reason (function calls).