Add policy init and reconcile policy support for artifacts #844
Conversation
This file contains 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
jhrozek
commented
Sep 4, 2023
| @@ -44,14 +44,14 @@ SELECT * FROM artifact_versions WHERE artifact_id = $1 AND sha = $2; | |||
| SELECT * FROM artifact_versions | |||
| WHERE artifact_id = $1 | |||
| ORDER BY created_at DESC | |||
| LIMIT $2; | |||
| LIMIT COALESCE(sqlc.narg('limit')::int, 2147483647); | |||
There was a problem hiding this comment.
@yrobla I wasn't able to force sqlc to accept an NULL argument here without the COALESCE. I was trying to just use LIMIT sqlc::narg('limit') here because Postgres uses NULL value in limit as "no limit", but sqlc wasn't just generating the query parameter at all unless I coalesced it. Have you run into a similar issue earlier?
jhrozek
commented
Sep 4, 2023
jhrozek
commented
Sep 4, 2023
jhrozek
commented
Sep 4, 2023
|
This needs to be rebased atop #841 (already done locally, the rebase just needs to be pushed) |
In some cases, like evaluating all results, we want to really list all the versions, but our API expected a limit. Rather than letting the user pass in a large enough number, let's let them pass NULL and get all the artifacts.
The field was not used anywhere and I'm not sure if it's actually useful for the clients. Let's remove it.
On artifact reconcile, call the policy evaluation so that artifacts would get evaluated against policies right when a repo is registered with mediator.
Call policy evaluation on artifacts, too, so that when an artifact policy is created after a repo is enrolled and after the initial reconcile, we still get the policy evaluation result. Fixes: #599
jhrozek
force-pushed
the
artifact_init_reconcile
branch
from
8e0e027
to
7acc708
Compare
…tect bugs The code was mixing up artifactr ID coming from GH with the primary key of the database rows. Let's rename the artifact to avoid such bugs in the future.
jhrozek
force-pushed
the
artifact_init_reconcile
branch
from
7acc708
to
5b8b28a
Compare
JAORMX
approved these changes
Sep 5, 2023
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.
The PR contains some fixes, removing one unused field from the Artifact protobuf
struct or renaming an ambiguous field in the same struct. The core of the PR
is evaluating policies on policy init for artifacts and on reconcile so that we
get a policy result at the time when the policy is instantiated or when the repo
is registered.
Also some small fixes that I found while testing the latest refactor.
Fixes: #804