SK-173 deref vars

[drmorr0]

Related Links

• https://linear.app/acrl/issue/SK-173/allow-dereferencing-variables-in-the-skel-parser

Description and Rationale

• Allow dereferencing previously-defined variables in a conditional, so that we can do things like "remove all volume mounts that match the name of a secret volume"
• We remove the ability to reference variables in the middle of a path/pointer, e.g. metadata.labels.$x is no longer valid. Allowing this makes everything more complicated, and it doesn't make sense anyways, because variables are always defined to be an absolute path starting from the root of the JSON object.

How

• Slight modification to the grammar to define a variable_prefixed_resource_path which must start with a variable; this lets us catch some errors at parse time, e.g., defining a variable and then not using it at all.
• The MatchContext previously just stored the defined variables and the JSON pointer that they resolve to. Now we've modified the MatchContext so that it stores both the JSON pointer(s) as well as the actual values that the point to. Then we can just look those values up relatively quickly when we encounter it on the RHS of a conditional.
• We refactored out the conditional testing code into its own method, which I think makes things a little more clear.

Test Steps

• Existing tests pass
• New itest passes for the volume functionality we want to support
• need some more unit tests
• manual testing with the new sanitize.skel

Other Notes

The DSL currently makes a distinction between the LHS and RHS of a conditional; specifically, if a variable is defined in a line, the left-hand-side of the conditional must reference that value, and the right-hand-side of the conditional can only reference previously-defined (and stored in the MatchContext) variables.

For example, this is valid:

remove($x := spec.template.spec.volumes[*] | exists($x.secret)
    && $y := spec.template.spec.containers[*].volumeMounts[*] | $y.name == $x.name,
    $y);

But this is not, because the left-hand side of the conditional on the second line does not reference $y (and, if it could get that far before panicking, the right-hand-side does reference $y, which isn't stored in the MatchContext yet):

remove($x := spec.template.spec.volumes[*] | exists($x.secret)
    && $y := spec.template.spec.containers[*].volumeMounts[*] | $x.name == $y.name,
    $y);

We could do the work to make this symmetric, but I don't really think it's worth-it right now, and it seems a bit unnatural to support anyways. If people complain we can revisit.

• I certify that this PR does not contain any code that has been generated with GitHub Copilot or any other AI-based code generation tool, in accordance with this project's policies.

[linear]

SK-173 Allow "dereferencing" variables in the SKEL parser

[codecov]

Codecov Report

❌ Patch coverage is 84.15842% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.14%. Comparing base (2e223b4) to head (02470eb).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
sk-cli/src/skel/ast.rs	60.00%	10 Missing ⚠️
sk-cli/src/skel/engine.rs	89.47%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #222      +/-   ##
==========================================
+ Coverage   74.35%   75.14%   +0.78%     
==========================================
  Files          63       63              
  Lines        3709     3721      +12     
  Branches      181      182       +1     
==========================================
+ Hits         2758     2796      +38     
+ Misses        846      820      -26     
  Partials      105      105              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.