Keep resolve() returning the target logical ID for custom Rego rules while built-in policies see the __ref marker - #429
Merged
Conversation
…while built-in policies see the __ref marker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#425 changed how the Rego
resolve()builtin renders aRef/Fn::GetAttto a template resource: the target'slogical ID string became the
{"__ref": target}marker object. The built-in policies needed that so a format orenum check never judges a logical ID as if it were the literal (six false positives fixed there). It was also a
breaking change for every custom Rego rule written against the documented contract "references become target IDs",
and it broke silently:
input.resources[resolve(...)],resolve(...) == "LogicalId", and string operations on theresult all stop matching, so the rule stops firing rather than erroring.
This change keeps both contracts by selecting the rendering per evaluating package:
engine the Composite engine layers on top.
How:
rego-engine/src/eval_context.rs:ReferenceRendering { Marker, TargetId }in a thread-localCell, with anRAII
ReferenceRenderingScopethat restores the previous rendering on drop, andcurrent_reference_rendering().rego-engine/src/engine.rs:PolicyPackageKind { BuiltIn, Custom }replaces the loosesource_label/originparameters of
eval_package_intoand installs the scope around each package'seval_rulequery. This is safebecause Regorus clears
rule_valuesandbuiltins_cachebetweeneval_rulecalls, so a value rendered for onepackage never reaches the next.
rego-engine/src/builtins.rs:resolved_to_rego/resolved_all_to_regotake the rendering. This covers exactlythe two shapes Fix false positives where unresolved or referenced values were treated as missing or literal #425 changed - a reference that is the resolved value, and a reference inside a resolved list
(
resolveandresolve_all). Map values were already rendered as markers before Fix false positives where unresolved or referenced values were treated as missing or literal #425 and stay that way. A barereference still contributes no value to
resolve_all.follow_ref,authored_form,flatten_list,has_propertyare unchanged; theinputdocument is unchanged.CUSTOM_RULES.mdrestores theresolve/resolve_allcontract for custom-rule authors (withnot input.resources[value],follow_ref, orauthored_formas the ways to exclude or read a reference);the rego-engine README gains a "Reference rendering" section explaining the two audiences.
Consumers that already migrated to
follow_refneed no change:resolveandfollow_refreturn the same logicalID for a
ReforGetAtttarget, andfollow_refstays undefined for literals.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.