fix: handle artifact rule evaluation differently

[rdimitrov]

The following PR proposes a change for how we process artifacts and their versions and also how we evaluate the rules in case of artifact entities.

Currently we evaluate a policy/rule in the same way for all entities. This happens on an entity event basis:

• single entity event is published (either through GitHub webhook or when we register a repo or create a policy)
• we collect all policies matching its group
• get the rules from those policies that are relevant to this entity (discard the rest)
• run each rule against this entity

Artifacts can change over time, i.e. a signed image tagged with [latest, v1.0.0] can pass its policy, but then if we push another image tagged only with [latest] this policy should now be failing.

The proposed change includes two things:

1. We publish an entity event when the tags of an existing entry has been updated (the example from above)
2. An artifact entity event is published with the artifact and all of its versions. This way we can ensure that the rule result is evaluated properly based on the whole picture and not just for this single event at this given time.
3. Moves away from jq and uses rego for evaluating versioned artifact rules. This was needed because we no longer pass a single object to the evaluator, but a list instead and rego enabled that.
4. Updated the tests, but I plan to revisit this at some point to try capture more scenarios.

Fixes #961
Fixes #967

[evankanderson]

You have some lint errors around headers as well (some ordering, one that you introduced a header that's shadowed by a method argument in one place).

[rdimitrov]

You have some lint errors around headers as well (some ordering, one that you introduced a header that's shadowed by a method argument in one place).
Thanks for your feedback! 👍 This is mostly a proof-of-concept PR, but I think some of the issues you outlined might be solved by implementing this functionality as a separate evaluator for artifacts and try to keep the executor code entity agnostic.

[jhrozek]

I realise this comment is not very useful without a concrete proposal, but what makes me uneasy about this approach is that we leak a lot of details about how artifacts are laid out in the DB into the evaluator code. Would it be possible to create a method on the EntityInfoWrapper that would return a slice of generic protobuf objects the executor could iterate on without having to call into the DB layer for artifacts?

I'll try to poke at the code more tomorrow, just leaving this comment here for now in hopes someone can articulate what I mean better..

[evankanderson]

I think that's a pretty reasonable callout, though I don't have a concrete proposal at the moment, either.

[rdimitrov]

I tried doing something different which hopefully makes more sense. Combined all versioned artifacts to be part of an artifact and that made more of the things a bit clearer, but I still have to verify it properly since I tried it with a patched evaluator. I tried doing rego unsuccessfully for creating a new artifacts rule type since now we pass a list of metadata for each versioned artifact that passed our filtering so we want to iterate over it. Even though it's not working properly, I've added it to the PR so Ozz can share his feedback and once it's okay it will replace the existing one which uses jq.

[rdimitrov]

I've moved the random package out of utils to a standalone as I had some import cycle errors. It's a fairly simple change, but I realise it might make the PR difficult to review. Let me know if you want to have that moved to a separate PR 👍

[rdimitrov]

I rebased again. Added more artifact tests and did the same thing for jsonyaml in order to solve import cycle errors. Let me know if the helper function about collecting the artifact versions has a better place so I can move it there 👍

[JAORMX]

LGTM, I like how the reconciler looks way cleaner thanks to this.

[jhrozek]

LGTM!

Sorry I was so slow with reviews lately, I was trying to get some implementation of the remediations going. This is a really nice work though!

No longer applicable since the code changed since then. Nevertheless, if there's any feedback I'll address it in a separate PR.