diff --git a/cmd/dev/app/rule_type/rttst.go b/cmd/dev/app/rule_type/rttst.go index 9215f41927..7ebd39fff5 100644 --- a/cmd/dev/app/rule_type/rttst.go +++ b/cmd/dev/app/rule_type/rttst.go @@ -298,7 +298,7 @@ func selectAndEval( evalStatus *engif.EvalStatusParams, profileSelectors selectors.Selection, ) error { - selEnt := provsel.EntityToSelectorEntity(ctx, inf.Type, ewp) + selEnt := provsel.EntityToSelectorEntity(ctx, nil, inf.Type, ewp) if selEnt == nil { return fmt.Errorf("error converting entity to selector entity") } diff --git a/internal/engine/executor.go b/internal/engine/executor.go index d38af095cd..e798824795 100644 --- a/internal/engine/executor.go +++ b/internal/engine/executor.go @@ -240,7 +240,7 @@ func (e *executor) profileEvalStatus( return fmt.Errorf("error getting entity with properties: %w", err) } - selEnt := provsel.EntityToSelectorEntity(ctx, eiw.Type, ewp) + selEnt := provsel.EntityToSelectorEntity(ctx, e.querier, eiw.Type, ewp) if selEnt == nil { return fmt.Errorf("error converting entity to selector entity") } diff --git a/internal/providers/selectors/selector_entity.go b/internal/providers/selectors/selector_entity.go index cc435ef60f..892dd9c99b 100644 --- a/internal/providers/selectors/selector_entity.go +++ b/internal/providers/selectors/selector_entity.go @@ -18,10 +18,12 @@ package selectors import ( "context" + "fmt" "github.com/rs/zerolog" "google.golang.org/protobuf/proto" + "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/entities/models" "github.com/stacklok/minder/internal/entities/properties" internalpb "github.com/stacklok/minder/internal/proto" @@ -29,14 +31,22 @@ import ( minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" ) -type toSelectorEntity func(ctx context.Context, entityWithProps *models.EntityWithProperties) *internalpb.SelectorEntity - -func repoToSelectorEntity( - _ context.Context, entityWithProps *models.EntityWithProperties) *internalpb.SelectorEntity { - if entityWithProps.Entity.Type != minderv1.Entity_ENTITY_REPOSITORIES { - return nil +func buildBaseSelectorEntity( + entityWithProps *models.EntityWithProperties, selProv *internalpb.SelectorProvider) *internalpb.SelectorEntity { + return &internalpb.SelectorEntity{ + EntityType: entityWithProps.Entity.Type, + Name: entityWithProps.Entity.Name, + Provider: selProv, } +} + +type toSelectorEntity func( + entityWithProps *models.EntityWithProperties, selProv *internalpb.SelectorProvider, +) *internalpb.SelectorEntity +func repoToSelectorEntity( + entityWithProps *models.EntityWithProperties, selProv *internalpb.SelectorProvider, +) *internalpb.SelectorEntity { var isFork *bool if propIsFork, err := entityWithProps.Properties.GetProperty(properties.RepoPropertyIsFork).AsBool(); err == nil { isFork = proto.Bool(propIsFork) @@ -47,26 +57,22 @@ func repoToSelectorEntity( isPrivate = proto.Bool(propIsPrivate) } - return &internalpb.SelectorEntity{ - EntityType: minderv1.Entity_ENTITY_REPOSITORIES, - Name: entityWithProps.Entity.Name, - Entity: &internalpb.SelectorEntity_Repository{ - Repository: &internalpb.SelectorRepository{ - Name: entityWithProps.Entity.Name, - IsFork: isFork, - IsPrivate: isPrivate, - Properties: entityWithProps.Properties.ToProtoStruct(), - }, + selEnt := buildBaseSelectorEntity(entityWithProps, selProv) + selEnt.Entity = &internalpb.SelectorEntity_Repository{ + Repository: &internalpb.SelectorRepository{ + Name: entityWithProps.Entity.Name, + IsFork: isFork, + IsPrivate: isPrivate, + Properties: entityWithProps.Properties.ToProtoStruct(), + Provider: selProv, }, } + return selEnt } func artifactToSelectorEntity( - ctx context.Context, entityWithProps *models.EntityWithProperties) *internalpb.SelectorEntity { - if entityWithProps.Entity.Type != minderv1.Entity_ENTITY_ARTIFACTS { - return nil - } - + entityWithProps *models.EntityWithProperties, selProv *internalpb.SelectorProvider, +) *internalpb.SelectorEntity { var artifactType string var err error artifactType, err = entityWithProps.Properties.GetProperty(properties.ArtifactPropertyType).AsString() @@ -74,35 +80,30 @@ func artifactToSelectorEntity( artifactType = entityWithProps.Properties.GetProperty(ghprop.ArtifactPropertyType).GetString() } - return &internalpb.SelectorEntity{ - EntityType: minderv1.Entity_ENTITY_ARTIFACTS, - Name: entityWithProps.Entity.Name, - Entity: &internalpb.SelectorEntity_Artifact{ - Artifact: &internalpb.SelectorArtifact{ - Name: entityWithProps.Entity.Name, - Type: artifactType, - Properties: entityWithProps.Properties.ToProtoStruct(), - }, + selEnt := buildBaseSelectorEntity(entityWithProps, selProv) + selEnt.Entity = &internalpb.SelectorEntity_Artifact{ + Artifact: &internalpb.SelectorArtifact{ + Name: entityWithProps.Entity.Name, + Type: artifactType, + Properties: entityWithProps.Properties.ToProtoStruct(), + Provider: selProv, }, } + return selEnt } func pullRequestToSelectorEntity( - _ context.Context, entityWithProps *models.EntityWithProperties) *internalpb.SelectorEntity { - if entityWithProps.Entity.Type != minderv1.Entity_ENTITY_PULL_REQUESTS { - return nil - } - - return &internalpb.SelectorEntity{ - EntityType: minderv1.Entity_ENTITY_PULL_REQUESTS, - Name: entityWithProps.Entity.Name, - Entity: &internalpb.SelectorEntity_PullRequest{ - PullRequest: &internalpb.SelectorPullRequest{ - Name: entityWithProps.Entity.Name, - Properties: entityWithProps.Properties.ToProtoStruct(), - }, + entityWithProps *models.EntityWithProperties, selProv *internalpb.SelectorProvider, +) *internalpb.SelectorEntity { + selEnt := buildBaseSelectorEntity(entityWithProps, selProv) + selEnt.Entity = &internalpb.SelectorEntity_PullRequest{ + PullRequest: &internalpb.SelectorPullRequest{ + Name: entityWithProps.Entity.Name, + Properties: entityWithProps.Properties.ToProtoStruct(), + Provider: selProv, }, } + return selEnt } // newConverterFactory creates a new converterFactory with the default converters for each entity type @@ -118,9 +119,31 @@ func newConverter(entType minderv1.Entity) toSelectorEntity { return nil } +func fillProviderInfo( + ctx context.Context, + querier db.Store, + entityWithProps *models.EntityWithProperties, +) (*internalpb.SelectorProvider, error) { + if querier == nil { + zerolog.Ctx(ctx).Warn().Msg("No querier, will not fill provider information") + return nil, nil + } + + dbProv, err := querier.GetProviderByID(ctx, entityWithProps.Entity.ProviderID) + if err != nil { + return nil, fmt.Errorf("failed to get provider %s by ID", entityWithProps.Entity.ProviderID.String()) + } + + return &internalpb.SelectorProvider{ + Name: dbProv.Name, + Class: string(dbProv.Class), + }, nil +} + // EntityToSelectorEntity converts an entity to a SelectorEntity func EntityToSelectorEntity( ctx context.Context, + querier db.Store, entType minderv1.Entity, entityWithProps *models.EntityWithProperties, ) *internalpb.SelectorEntity { @@ -129,5 +152,15 @@ func EntityToSelectorEntity( zerolog.Ctx(ctx).Error().Str("entType", entType.ToString()).Msg("No converter available") return nil } - return converter(ctx, entityWithProps) + + selProv, err := fillProviderInfo(ctx, querier, entityWithProps) + if err != nil { + zerolog.Ctx(ctx).Error(). + Str("providerID", entityWithProps.Entity.ProviderID.String()). + Err(err). + Msg("Cannot fill provider information") + return nil + } + selEnt := converter(entityWithProps, selProv) + return selEnt } diff --git a/internal/providers/selectors/selector_entity_test.go b/internal/providers/selectors/selector_entity_test.go index 7daa1a5b9e..6e7bf5b917 100644 --- a/internal/providers/selectors/selector_entity_test.go +++ b/internal/providers/selectors/selector_entity_test.go @@ -17,13 +17,17 @@ package selectors import ( "context" + "database/sql" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" + "github.com/stacklok/minder/internal/db" + dbf "github.com/stacklok/minder/internal/db/fixtures" "github.com/stacklok/minder/internal/entities/models" "github.com/stacklok/minder/internal/entities/properties" internalpb "github.com/stacklok/minder/internal/proto" @@ -31,6 +35,25 @@ import ( minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" ) +var ( + githubProvider = db.Provider{ + Name: "github", + Class: db.ProviderClassGithubApp, + } + gitlabProvider = db.Provider{ + Name: "gitlab", + Class: db.ProviderClassGitlab, + } +) + +func withGetProviderByID(result db.Provider, err error) func(dbf.DBMock) { + return func(mock dbf.DBMock) { + mock.EXPECT(). + GetProviderByID(gomock.Any(), gomock.Any()). + Return(result, err) + } +} + func buildEntityWithProperties(entityType minderv1.Entity, name string, propMap map[string]any) *models.EntityWithProperties { props, err := properties.NewProperties(propMap) if err != nil { @@ -54,40 +77,46 @@ func checkProps(t *testing.T, got *structpb.Struct, expected map[string]any) { assert.Equal(t, gotMap, expected) } -func checkSelEntArtifact(t *testing.T, got, expected *internalpb.SelectorArtifact, propMap map[string]any) { +func checkSelEntArtifact(t *testing.T, got, expected *internalpb.SelectorArtifact, propMap map[string]any, expProvider *db.Provider) { t.Helper() assert.Equal(t, got.Name, expected.Name) assert.Equal(t, got.Type, expected.Type) + assert.Equal(t, got.GetProvider().GetName(), expProvider.Name) + assert.Equal(t, got.GetProvider().GetClass(), string(expProvider.Class)) checkProps(t, got.Properties, propMap) } -func checkSelEntRepo(t *testing.T, got, expected *internalpb.SelectorRepository, propMap map[string]any) { +func checkSelEntRepo(t *testing.T, got, expected *internalpb.SelectorRepository, propMap map[string]any, expProvider *db.Provider) { t.Helper() assert.Equal(t, got.Name, expected.Name) assert.Equal(t, got.IsFork, expected.IsFork) + assert.Equal(t, got.GetProvider().GetName(), expProvider.Name) + assert.Equal(t, got.GetProvider().GetClass(), string(expProvider.Class)) checkProps(t, got.Properties, propMap) } -func checkSelEntPullRequest(t *testing.T, got, expected *internalpb.SelectorPullRequest, propMap map[string]any) { +func checkSelEntPullRequest(t *testing.T, got, expected *internalpb.SelectorPullRequest, propMap map[string]any, expProvider *db.Provider) { t.Helper() assert.Equal(t, got.Name, expected.Name) + assert.Equal(t, got.GetProvider().GetName(), expProvider.Name) + assert.Equal(t, got.GetProvider().GetClass(), string(expProvider.Class)) checkProps(t, got.Properties, propMap) } -func checkSelEnt(t *testing.T, got, expected *internalpb.SelectorEntity, propMap map[string]any) { +func checkSelEnt(t *testing.T, got, expected *internalpb.SelectorEntity, propMap map[string]any, expProvider *db.Provider) { t.Helper() assert.Equal(t, got.EntityType, expected.EntityType) switch got.EntityType { // nolint:exhaustive case minderv1.Entity_ENTITY_REPOSITORIES: - checkSelEntRepo(t, got.GetRepository(), expected.GetRepository(), propMap) + checkSelEntRepo(t, got.GetRepository(), expected.GetRepository(), propMap, expProvider) case minderv1.Entity_ENTITY_ARTIFACTS: - checkSelEntArtifact(t, got.GetArtifact(), expected.GetArtifact(), propMap) + checkSelEntArtifact(t, got.GetArtifact(), expected.GetArtifact(), propMap, expProvider) case minderv1.Entity_ENTITY_PULL_REQUESTS: - checkSelEntPullRequest(t, got.GetPullRequest(), expected.GetPullRequest(), propMap) + checkSelEntPullRequest(t, got.GetPullRequest(), expected.GetPullRequest(), propMap, expProvider) } } @@ -103,6 +132,8 @@ func TestEntityToSelectorEntity(t *testing.T) { entityProps map[string]any expSelEnt *internalpb.SelectorEntity checkSelEnt func(proto.Message) + expDbProv *db.Provider + dbSetup dbf.DBMockBuilder success bool }{ { @@ -128,7 +159,11 @@ func TestEntityToSelectorEntity(t *testing.T) { }, }, }, - success: true, + dbSetup: dbf.NewDBMock( + withGetProviderByID(githubProvider, nil), + ), + expDbProv: &githubProvider, + success: true, }, { name: "Artifact", @@ -152,7 +187,11 @@ func TestEntityToSelectorEntity(t *testing.T) { }, }, }, - success: true, + dbSetup: dbf.NewDBMock( + withGetProviderByID(githubProvider, nil), + ), + expDbProv: &githubProvider, + success: true, }, { name: "Pull Request", @@ -177,7 +216,54 @@ func TestEntityToSelectorEntity(t *testing.T) { }, }, }, - success: true, + dbSetup: dbf.NewDBMock( + withGetProviderByID(gitlabProvider, nil), + ), + expDbProv: &gitlabProvider, + success: true, + }, + { + name: "Repository but no querier provided", + entityType: minderv1.Entity_ENTITY_REPOSITORIES, + entityName: "testorg/testrepo", + entityProps: map[string]any{ + properties.PropertyUpstreamID: "12345", + properties.RepoPropertyIsFork: true, + properties.RepoPropertyIsPrivate: true, + ghprops.RepoPropertyId: "12345", + ghprops.RepoPropertyName: "testrepo", + ghprops.RepoPropertyOwner: "testorg", + }, + expSelEnt: &internalpb.SelectorEntity{ + EntityType: minderv1.Entity_ENTITY_REPOSITORIES, + Name: "testorg/testrepo", + Entity: &internalpb.SelectorEntity_Repository{ + Repository: &internalpb.SelectorRepository{ + Name: "testorg/testrepo", + IsFork: &trueBool, + IsPrivate: &trueBool, + }, + }, + }, + expDbProv: &db.Provider{}, + success: true, + }, + { + name: "Invalid Entity Type", + entityType: minderv1.Entity_ENTITY_BUILD, + entityName: "testorg/testbuild", + dbSetup: dbf.NewDBMock(), + success: false, + }, + { + name: "Invalid Provider", + entityType: minderv1.Entity_ENTITY_REPOSITORIES, + entityName: "testorg/testrepo", + dbSetup: dbf.NewDBMock( + withGetProviderByID(db.Provider{}, sql.ErrNoRows), + ), + expDbProv: &githubProvider, + success: false, }, } @@ -187,10 +273,20 @@ func TestEntityToSelectorEntity(t *testing.T) { t.Run(scenario.name, func(t *testing.T) { t.Parallel() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + ctx := context.Background() + + var mockQuerier db.Store + if scenario.dbSetup != nil { + mockQuerier = scenario.dbSetup(ctrl) + } + entity := buildEntityWithProperties(scenario.entityType, scenario.entityName, scenario.entityProps) selEnt := EntityToSelectorEntity( - context.Background(), + ctx, + mockQuerier, scenario.entityType, entity, ) @@ -199,7 +295,9 @@ func TestEntityToSelectorEntity(t *testing.T) { require.NotNil(t, selEnt) require.Equal(t, selEnt.GetEntityType(), scenario.entityType) require.Equal(t, selEnt.GetName(), scenario.entityName) - checkSelEnt(t, selEnt, scenario.expSelEnt, scenario.entityProps) + require.Equal(t, selEnt.GetProvider().GetName(), scenario.expDbProv.Name) + require.Equal(t, selEnt.GetProvider().GetClass(), string(scenario.expDbProv.Class)) + checkSelEnt(t, selEnt, scenario.expSelEnt, scenario.entityProps, scenario.expDbProv) } else { require.Nil(t, selEnt) }