Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ext/dynblock: Allow callers to veto for_each values #634

Merged
merged 1 commit into from Oct 16, 2023

Conversation

apparentlymart
Copy link
Member

Callers might have additional rules for what's acceptable in a for_each value for a dynamic block. For example, Terraform wants to forbid using sensitive values here because it would cause the expansion to disclose the length of the given collection, but currently ends up returning a very low-quality (highly misleading) error in that case, as discussed in hashicorp/terraform#29744 .

Therefore this provides a hook point for callers to insert additional checks just after the for_each expression has been evaluated and before any of the built-in checks are run. Terraform can use this to check whether the for_each value is sensitive, and if so to return a specialized error about that which:

  • Has the given expression and eval context included in the diagnostic, so Terraform's diagnostic UI code can show all of the external values that contributed to the for_each expression.
  • Is annotated using the Terraform-specific "extra" annotation that an error is caused by sensitive values, which permits the UI to mention which values are sensitive as part of the error message. (Terraform doesn't do that by default, because we found that folks often assumed that Terraform would only mention that a value is sensitive if that was what caused the error, and and so often skipped reading any other part of the error message as soon as there was any mention of that.)

This would therefore allow a future change to Terraform itself to make it return a helpful error message in this situation, similar to the one it already returns for its own for_each features in the same situation.

This introduces the "functional options" pattern for dynblock.ExpandBlock for the first time, as a way to extend the API without breaking compatibility with existing callers. There is currently only this one option.

Callers might have additional rules for what's acceptable in a for_each
value for a dynamic block. For example, Terraform wants to forbid using
sensitive values here because it would cause the expansion to disclose the
length of the given collection.

Therefore this provides a hook point for callers to insert additional
checks just after the for_each expression has been evaluated and before
any of the built-in checks are run.

This introduces the "functional options" pattern for ExpandBlock for the
first time, as a way to extend the API without breaking compatibility with
existing callers. There is currently only this one option.
@apparentlymart apparentlymart requested a review from a team October 13, 2023 23:51
@apparentlymart apparentlymart self-assigned this Oct 13, 2023
@apparentlymart apparentlymart merged commit 0af4fe2 into main Oct 16, 2023
7 checks passed
@apparentlymart apparentlymart deleted the f-dynblock-foreach-check-hook branch October 16, 2023 16:20
@apparentlymart
Copy link
Member Author

(Just for the sake of creating a backlink in GitHub.)

This also includes a new building block that we can use to resolve hashicorp/terraform#29744 by implementing a Terraform-specific error message about sensitive values in the for_each position, but it'll take some more work on the Terraform side before we actually benefit from that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants