Populate entityInfo for all entity types in ListEvaluationResults#6361
Open
krrish175-byte wants to merge 1 commit into
Open
Conversation
This addresses issue mindersec#3116 by reusing the getRuleEvalEntityInfo helper across ListEvaluationResults so that entityInfo (like artifact_name, artifact_type, etc.) is correctly populated for non-repository entities such as Artifacts and Pull Requests. Signed-off-by: krrish175-byte <krrishbiswas175@gmail.com>
5bd69ad to
b81f8b7
Compare
Contributor
Author
|
@evankanderson PTAL |
evankanderson
requested changes
Apr 28, 2026
Member
evankanderson
left a comment
There was a problem hiding this comment.
This needs to appease the linter; otherwise, it looks good.
| return era | ||
| } | ||
|
|
||
| func getRuleEvalEntityInfo( |
Member
There was a problem hiding this comment.
This looks like a straight move from handlers_profile.go (fine, but this adds a "type 2 - refactoring" change to a "type 3 - behavior" change, which makes it a little bit harder to review).
Comment on lines
+885
to
+890
| minderEntityType := minderv1.Entity_ENTITY_REPOSITORIES | ||
| if tt.entityType == db.EntitiesArtifact { | ||
| minderEntityType = minderv1.Entity_ENTITY_ARTIFACTS | ||
| } else if tt.entityType == db.EntitiesPullRequest { | ||
| minderEntityType = minderv1.Entity_ENTITY_PULL_REQUESTS | ||
| } |
Member
There was a problem hiding this comment.
Lint wants you to use a tagged switch here. If you disagree, you'll need to add an appropriately scoped nolint directive, e.g. //nolint:staticcheck // tagged switch is ...
Suggested change
| minderEntityType := minderv1.Entity_ENTITY_REPOSITORIES | |
| if tt.entityType == db.EntitiesArtifact { | |
| minderEntityType = minderv1.Entity_ENTITY_ARTIFACTS | |
| } else if tt.entityType == db.EntitiesPullRequest { | |
| minderEntityType = minderv1.Entity_ENTITY_PULL_REQUESTS | |
| } | |
| minderEntityType := minderv1.Entity_ENTITY_REPOSITORIES | |
| switch tt.EntityType { | |
| case db.EntitiesArtifact: | |
| minderEntityType = minderv1.Entity_ENTITY_ARTIFACTS | |
| case db.EntitiesPullRequest: | |
| minderEntityType = minderv1.Entity_ENTITY_PULL_REQUESTS | |
| } |
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
This PR addresses an issue where the
ListEvaluationResultsRPC only populatedentityInfofor repository entities, leaving it empty for other entity types such as Pull Requests and Artifacts.To fix this, the
getRuleEvalEntityInfohelper function (previously scoped tohandlers_profile.go) was moved tohandlers_evalstatus.go. The mapping logic inbuildRuleEvaluationStatusFromDBEvaluationwas then updated to utilize this shared helper, ensuring consistent population ofentityInfoattributes (e.g.,artifact_name,artifact_type, etc.) across all supported entity types without duplicating code. Unused imports inhandlers_profile.gowere also cleaned up.Fixes #3116
Testing
TestListEvaluationResultsEntityInfoto specifically verify thatentityInfofields are correctly populated when formatting the evaluation results for bothArtifactandPullRequestentities.internal/controlplanepackage (go test -v ./internal/controlplane/...) to ensure no regressions were introduced. All tests passed successfully.