Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions internal/controlplane/handlers_evalstatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,29 +630,14 @@ func (s *Server) buildRuleEvaluationStatusFromDBEvaluation(
return nil, fmt.Errorf("error fetching properties for entity: %s: %w", efp.Entity.ID.String(), err)
}

entityInfo := map[string]string{}
entityInfo["entity_id"] = eval.EntityID.String()
entityInfo := getRuleEvalEntityInfo(eval, efp)

if uid := efp.Properties.GetProperty(properties.PropertyUpstreamID); uid != nil {
entityInfo["upstream_id"] = uid.GetString()
}

remediationURL := ""
if eval.EntityType == db.EntitiesRepository {
// If any fields are missing, just leave them empty in the response
entityInfo["provider"] = eval.Provider
// TODO: We'll probably remove these fields in the future as we
// introduce more providers
if owner := efp.Properties.GetProperty(ghprop.RepoPropertyOwner); owner != nil {
entityInfo["repo_owner"] = owner.GetString()
}
if name := efp.Properties.GetProperty(ghprop.RepoPropertyName); name != nil {
entityInfo["repo_name"] = name.GetString()
}

// TODO: This will be removed in favor of entity_id
entityInfo["repository_id"] = efp.Entity.ID.String()

remediationURL, err = getRemediationURLFromMetadata(
eval.RemMetadata, efp.Entity.Name,
)
Expand Down Expand Up @@ -764,6 +749,49 @@ func buildEvalResultAlertFromLRERow(
return era
}

func getRuleEvalEntityInfo(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

rs db.ListRuleEvaluationsByProfileIdRow,
efp *entmodels.EntityWithProperties,
) map[string]string {
entityInfo := map[string]string{}

if name := efp.Properties.GetProperty(properties.PropertyName); name != nil {
entityInfo["name"] = name.GetString()
}

if owner := efp.Properties.GetProperty(ghprop.RepoPropertyOwner); owner != nil {
entityInfo["repo_owner"] = owner.GetString()
}
if name := efp.Properties.GetProperty(ghprop.RepoPropertyName); name != nil {
entityInfo["repo_name"] = name.GetString()
}

if artName := efp.Properties.GetProperty(ghprop.ArtifactPropertyName); artName != nil {
entityInfo["artifact_name"] = artName.GetString()
}

if artType := efp.Properties.GetProperty(ghprop.ArtifactPropertyType); artType != nil {
entityInfo["artifact_type"] = artType.GetString()
}

entityInfo["provider"] = rs.Provider
entityInfo["entity_type"] = efp.Entity.Type.ToString()
entityInfo["entity_id"] = rs.EntityID.String()

// temporary: These will be replaced by entity_id
//nolint:exhaustive
switch rs.EntityType {
case db.EntitiesRepository:
entityInfo["repository_id"] = efp.Entity.ID.String()
case db.EntitiesArtifact:
entityInfo["artifact_id"] = efp.Entity.ID.String()
default:
// We only need to handle the above two types specially for historical compatibility.
}

return entityInfo
}

func dbEntityToEntity(dbEnt db.Entities) minderv1.Entity {
switch dbEnt {
case db.EntitiesPullRequest:
Expand Down
127 changes: 127 additions & 0 deletions internal/controlplane/handlers_evalstatus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
mockpropssvc "github.com/mindersec/minder/internal/entities/properties/service/mock"
"github.com/mindersec/minder/internal/history"
mockhistory "github.com/mindersec/minder/internal/history/mock"
ghprop "github.com/mindersec/minder/internal/providers/github/properties"
minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
"github.com/mindersec/minder/pkg/entities/properties"
)

func TestBuildEvalResultAlertFromLRERow(t *testing.T) {
Expand Down Expand Up @@ -844,3 +846,128 @@
})
}
}

func TestListEvaluationResultsEntityInfo(t *testing.T) {
t.Parallel()

projectID := uuid.New()
profileID := uuid.New()
entityID := uuid.New()
ruleTypeID := uuid.New()

tests := []struct {
name string
entityType db.Entities
expectKeys []string
}{
{
name: "artifact entity info",
entityType: db.EntitiesArtifact,
expectKeys: []string{"artifact_name", "artifact_type", "entity_id", "entity_type", "provider", "artifact_id"},
},
{
name: "pull request entity info",
entityType: db.EntitiesPullRequest,
expectKeys: []string{"entity_id", "entity_type", "provider"}, // PR doesn't have specialized keys in getRuleEvalEntityInfo yet besides name
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockStore := mockdb.NewMockStore(ctrl)
mockProps := mockpropssvc.NewMockPropertiesService(ctrl)

minderEntityType := minderv1.Entity_ENTITY_REPOSITORIES
if tt.entityType == db.EntitiesArtifact {

Check failure on line 886 in internal/controlplane/handlers_evalstatus_test.go

View workflow job for this annotation

GitHub Actions / lint / Run golangci-lint

QF1003: could use tagged switch on tt.entityType (staticcheck)
minderEntityType = minderv1.Entity_ENTITY_ARTIFACTS
} else if tt.entityType == db.EntitiesPullRequest {
minderEntityType = minderv1.Entity_ENTITY_PULL_REQUESTS
}
Comment on lines +885 to +890

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
}


props := map[string]any{
properties.PropertyName: "test-entity",
}
if tt.entityType == db.EntitiesArtifact {
props[ghprop.ArtifactPropertyName] = "test-artifact"
props[ghprop.ArtifactPropertyType] = "container"
}

psObj := properties.NewProperties(props)
efp := entmodels.NewEntityWithPropertiesFromInstance(
entmodels.EntityInstance{
ID: entityID,
Type: minderEntityType,
Name: "test-entity",
}, psObj)

mockProps.EXPECT().
EntityWithPropertiesByID(gomock.Any(), entityID, gomock.Any()).
Return(efp, nil)
mockProps.EXPECT().
RetrieveAllPropertiesForEntity(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(nil)
mockStore.EXPECT().
GetProfileStatusByProject(gomock.Any(), projectID).
Return([]db.GetProfileStatusByProjectRow{
{ID: profileID, ProfileStatus: db.EvalStatusTypesSuccess},
}, nil)
mockStore.EXPECT().
ListProfilesByProjectIDAndLabel(gomock.Any(), gomock.Any()).
Return([]db.ListProfilesByProjectIDAndLabelRow{
{Profile: db.Profile{ID: profileID, Name: "test-profile", UpdatedAt: time.Now()}},
}, nil)
mockStore.EXPECT().
ListRuleEvaluationsByProfileId(gomock.Any(), gomock.Any()).
Return([]db.ListRuleEvaluationsByProfileIdRow{
{
RuleEvaluationID: uuid.New(),
EntityType: tt.entityType,
EntityID: entityID,
EntityName: "test-entity",
ProjectID: projectID,
RuleTypeID: ruleTypeID,
RuleName: "my_rule",
RuleTypeName: "rule_type_a",
RuleTypeReleasePhase: db.ReleaseStatusAlpha,
EvalStatus: db.EvalStatusTypesFailure,
EvalLastUpdated: time.Now(),
RemStatus: db.RemediationStatusTypesSkipped,
RemLastUpdated: time.Now(),
AlertStatus: db.AlertStatusTypesOff,
AlertLastUpdated: time.Now(),
RuleTypeSeverityValue: db.SeverityMedium,
Provider: "github",
},
}, nil)

server := Server{store: mockStore, props: mockProps}
ctx := engcontext.WithEntityContext(context.Background(), &engcontext.EntityContext{
Project: engcontext.Project{ID: projectID},
})

resp, err := server.ListEvaluationResults(ctx, &minderv1.ListEvaluationResultsRequest{})

require.NoError(t, err)
require.NotNil(t, resp)

found := false
for _, ent := range resp.Entities {
for _, prof := range ent.Profiles {
for _, res := range prof.Results {
found = true
for _, key := range tt.expectKeys {
_, ok := res.EntityInfo[key]
require.True(t, ok, "key %s not found in EntityInfo", key)
}
}
}
}
require.True(t, found, "no results found in response")
})
}
}
45 changes: 0 additions & 45 deletions internal/controlplane/handlers_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ import (
"github.com/mindersec/minder/internal/db"
"github.com/mindersec/minder/internal/engine/engcontext"
"github.com/mindersec/minder/internal/engine/entities"
entmodels "github.com/mindersec/minder/internal/entities/models"
propSvc "github.com/mindersec/minder/internal/entities/properties/service"
"github.com/mindersec/minder/internal/logger"
ghprop "github.com/mindersec/minder/internal/providers/github/properties"
"github.com/mindersec/minder/internal/util"
minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
"github.com/mindersec/minder/pkg/entities/properties"
prof "github.com/mindersec/minder/pkg/profiles"
"github.com/mindersec/minder/pkg/ruletypes"
)
Expand Down Expand Up @@ -248,49 +246,6 @@ func getProfilePBFromDB(

// TODO: We need to replace this with a more generic method that can be used for all entities
// probably coming from the properties.
func getRuleEvalEntityInfo(
rs db.ListRuleEvaluationsByProfileIdRow,
efp *entmodels.EntityWithProperties,
) map[string]string {
entityInfo := map[string]string{}

if name := efp.Properties.GetProperty(properties.PropertyName); name != nil {
entityInfo["name"] = name.GetString()
}

if owner := efp.Properties.GetProperty(ghprop.RepoPropertyOwner); owner != nil {
entityInfo["repo_owner"] = owner.GetString()
}
if name := efp.Properties.GetProperty(ghprop.RepoPropertyName); name != nil {
entityInfo["repo_name"] = name.GetString()
}

if artName := efp.Properties.GetProperty(ghprop.ArtifactPropertyName); artName != nil {
entityInfo["artifact_name"] = artName.GetString()
}

if artType := efp.Properties.GetProperty(ghprop.ArtifactPropertyType); artType != nil {
entityInfo["artifact_type"] = artType.GetString()
}

entityInfo["provider"] = rs.Provider
entityInfo["entity_type"] = efp.Entity.Type.ToString()
entityInfo["entity_id"] = rs.EntityID.String()

// temporary: These will be replaced by entity_id
//nolint:exhaustive
switch rs.EntityType {
case db.EntitiesRepository:
entityInfo["repository_id"] = efp.Entity.ID.String()
case db.EntitiesArtifact:
entityInfo["artifact_id"] = efp.Entity.ID.String()
default:
// We only need to handle the above two types specially for historical compatibility.
}

return entityInfo
}

func (s *Server) getRuleEvaluationStatuses(
ctx context.Context,
dbRuleEvaluationStatuses []db.ListRuleEvaluationsByProfileIdRow,
Expand Down
Loading