feat: A reconciler to actually keep our records in check#4991
feat: A reconciler to actually keep our records in check#4991another-rex wants to merge 5 commits intogoogle:masterfrom
Conversation
michaelkedar
left a comment
michaelkedar
left a comment
There was a problem hiding this comment.
can we s/datastore/database (or similar) throughout (since the underlying database is abstracted away)
| _ = wt.Checkout(&git.CheckoutOptions{ | ||
| Branch: plumbing.NewRemoteReferenceName("origin", sourceRepo.Git.Branch), | ||
| Force: true, | ||
| }) |
There was a problem hiding this comment.
Have you verified that this actually checks it out on-disk?
I wouldn't be surprised if this was in-memory only.
| // "oss-fuzz", | ||
| // "debian-cve", | ||
| // "cve-osv", |
There was a problem hiding this comment.
why are these commented out.
Also FYI, "oss-fuzz" is explicitly filtered out of the SourceRepositoryStore implementation, so it shouldn't even be surfaced here.
| if sourceRepo.Type == models.SourceRepositoryTypeGit && sourceRepo.Git != nil { | ||
| if existingBranch, ok := gitBranches[sourceRepo.Git.URL]; ok { | ||
| if existingBranch != sourceRepo.Git.Branch { | ||
| return fmt.Errorf("multiple sources with same git url %q but different branches (%q and %q) is not supported", |
There was a problem hiding this comment.
Should this stop the entire reconciler?
If not, we could just combine this loop with the one afterwards, and not store the sourceRepos
There was a problem hiding this comment.
You have this in base, but no patch in the production environment, meaning it'll get deployed to prod without the PersistentVolume?
Either add the prod one (if you want it), or remove this from base, and add it as a resource (instead of a patch) to the test instance
| // - parsed: A pointer to a pre-parsed gjson.Result if already available. If nil, sourceRecord is opened and parsed dynamically. | ||
| // - sourceRecord: The interface used to lazily retrieve file contents (from a bucket, git, or REST). | ||
| // - format: RecordFormat representing the format we expect to decode (JSON, YAML, etc.). | ||
| func checkReconcile( |
There was a problem hiding this comment.
This blocks the SourceRepository walking to download the record from GCS/REST (or open the file when in git), then sends it to the importerWorker which downloads the file again to do validation/send to worker.
IMO most of this function should be in the pool of workers for the reconciler.
2 participants
Wrote a reconciler so that we catch any missing records within 30 hours of it happening.