diff --git a/terraform/policy/skills/terraform-policy/references/tfpolicy-author.md b/terraform/policy/skills/terraform-policy/references/tfpolicy-author.md index 1d1d8b4..0e02375 100644 --- a/terraform/policy/skills/terraform-policy/references/tfpolicy-author.md +++ b/terraform/policy/skills/terraform-policy/references/tfpolicy-author.md @@ -702,7 +702,7 @@ resource_policy "aws_s3_bucket_versioning" "versioning_enabled" { 6. Cover all variations of a resource family (e.g. AWS security groups: `aws_security_group`, `aws_security_group_rule`, `aws_vpc_security_group_ingress_rule`, `aws_default_security_group`). 7. Use `filter` to skip resources that don't apply (saves work and avoids false positives). 8. **Cache `core::getresources()` results in top-level locals** when the filter is a known literal or an existing resource ID — this avoids O(N) overhead per resource. **Exception:** when the filter depends on the current resource's own attribute (e.g. `{bucket = attrs.id}`, `{event_bus_name = attrs.name}`), the call cannot be pre-computed at top level because `attrs` is only available inside `resource_policy` — use the inline pattern instead (see item 15). ❌ Do NOT work around this by fetching all child resources at the top level with `{}` and building a lookup map — that is the same anti-pattern restructured. -9. **Avoid `core::getdatasource()` inside `resource_policy`** — it calls provider APIs. +9. **Cache `core::getdatasource()` results in top-level locals** when the filter is a known literal or constant — it makes a real provider API call, so repeating an identical call once per matching resource wastes work for no benefit. **Exception:** when the filter genuinely depends on the current resource's own attribute (e.g. validating that `attrs.ami` points to an AMI owned by an approved account, or that `attrs.kms_key_id` resolves to a key with the required policy), the call cannot be pre-computed at top level — use the inline pattern instead (see item 17). Scope inline calls with `filter` so the API call only runs for resources that actually need it, and wrap the result in `core::try()` since it calls a live API that can fail or return no match. 10. Build lookup maps once for O(1) matching when iterating many resources. 11. Keep each boolean expression on a single line (HCL parser limitation in beta). 12. Use clear variable names (`scanning_config`, not `sc`). @@ -714,6 +714,7 @@ resource_policy "aws_s3_bucket_versioning" "versioning_enabled" { - **When the enforcement goal is to ensure every parent has a compliant child, the dependent child must NEVER have a standalone `resource_policy` block.** Write the `resource_policy` block on the **parent type**. Fetch the dependent child inside the parent block via `core::getresources("", { = attrs.id_or_arn_or_name})`. Evaluate all attribute checks on those lookup results. Report all violations on the parent. When the goal is only to check every existing child's own attributes, a standalone `resource_policy` on the child type is valid — see Self-check above. - Concrete examples: `aws_s3_bucket_public_access_block`, `aws_s3_bucket_policy`, `aws_s3_bucket_acl` (all require `bucket`) → never standalone for any enforcement goal; always fetched inside `resource_policy "aws_s3_bucket"`. `aws_lb_listener` → standalone `resource_policy "aws_lb_listener"` is valid when checking every listener's own attributes (e.g., protocol, ssl_policy); use `resource_policy "aws_lb"` with inline lookup only when the goal is "every LB must have at least one compliant listener". - Always add this comment when using this pattern: *"This policy contains a cross-resource reference that will not resolve during plan time, but the policy will run successfully during apply time."* +17. **Inline `core::getdatasource()` for per-resource external validation:** use this when the check requires live provider data tied to *this specific resource* — e.g. confirming `attrs.ami` is owned by an approved account, `attrs.kms_key_id` has the required key policy, or a referenced secret/role actually exists. The filter is derived from `attrs.*`, so it cannot be hoisted to a top-level `locals` block. Use `filter` to skip resources that don't need the lookup (avoids the API call entirely for non-matching resources), and always wrap the call in `core::try()` — unlike `core::getresources()`, this hits a live provider API and can fail or return no match. ### Communication 1. Ask clarifying questions; don't assume requirements. diff --git a/terraform/policy/skills/terraform-policy/references/verified-syntax.md b/terraform/policy/skills/terraform-policy/references/verified-syntax.md index 3397141..a0e2f90 100644 --- a/terraform/policy/skills/terraform-policy/references/verified-syntax.md +++ b/terraform/policy/skills/terraform-policy/references/verified-syntax.md @@ -646,21 +646,10 @@ resource_policy "aws_s3_bucket" "public_access_required" { ``` See the `core::getresources()` decision guide in Critical Rule 2 above for the full pattern guidance. -### ❌ Mistake 14: Using core::getdatasource() Inside resource_policy -```hcl -# ❌ WRONG - Makes API calls for EVERY resource! -resource_policy "aws_s3_bucket" "check" { - locals { - account_id = core::getdatasource("aws_caller_identity", {}) - } -} +### ❌ Mistake 14: Using core::getdatasource() With a Static Filter Inside resource_policy +`core::getdatasource()` makes a real provider API call (not read from state/plan, not cached automatically). If the filter doesn't depend on the resource being evaluated, calling it inside `resource_policy` repeats the identical call once per matching resource for no benefit — cache it once in top-level `locals` instead. -# ✅ CORRECT - Cache in top-level locals -locals { - account_id = core::getdatasource("aws_caller_identity", {}) -} -``` -**Why:** Makes real provider API calls (not cached). Never use inside resource policies. +**Exception:** inline `core::getdatasource()` inside `resource_policy` is correct — not an anti-pattern — when the filter depends on the resource's own attributes (e.g. its own `attrs.*` value), since the lookup key is only known once that specific resource is being evaluated and a top-level cache is impossible. This is the same "filter depends on `attrs.*`" exception used for `core::getresources()` (see Mistake 13 above). In this case, use `filter` to skip resources that don't need the lookup, and wrap the call in `core::try()` since it hits a live provider API that can fail or return no match. ### ❌ Mistake 15: Using .attrs with core::getresources() ```hcl