Skip to content

Commit

Permalink
Add PullRequest as available entity for selectors (#4000)
Browse files Browse the repository at this point in the history
This is more or less for completeness' sake

Fixes: #3988
  • Loading branch information
jhrozek authored Jul 25, 2024
1 parent da3c20b commit 9f15189
Show file tree
Hide file tree
Showing 4 changed files with 285 additions and 85 deletions.
19 changes: 15 additions & 4 deletions internal/engine/selectors/selectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ func artifactEnvFactory() (*cel.Env, error) {
"internal.SelectorArtifact")
}

// pullRequestEnvFactory is a factory for creating a CEL environment
// for the SelectorPullRequest type representing a pull request
func pullRequestEnvFactory() (*cel.Env, error) {
return newEnvForEntity(
"pull_request",
&internalpb.SelectorArtifact{},
"internal.SelectorPullRequest")
}

// newEnvForEntity creates a new CEL environment for an entity. All environments are allowed to
// use the generic "entity" variable plus the specific entity type is also declared as variable
// with the appropriate type.
Expand Down Expand Up @@ -155,10 +164,10 @@ type entityEnvCache struct {
// are used on first access to create the CEL environments for each entity type.
func NewEnv() *Env {
factoryMap := map[minderv1.Entity]celEnvFactory{
minderv1.Entity_ENTITY_UNSPECIFIED: genericEnvFactory,
minderv1.Entity_ENTITY_REPOSITORIES: repoEnvFactory,
minderv1.Entity_ENTITY_ARTIFACTS: artifactEnvFactory,
// TODO(jakub): Add pull requests when we add then to the selector protobuf
minderv1.Entity_ENTITY_UNSPECIFIED: genericEnvFactory,
minderv1.Entity_ENTITY_REPOSITORIES: repoEnvFactory,
minderv1.Entity_ENTITY_ARTIFACTS: artifactEnvFactory,
minderv1.Entity_ENTITY_PULL_REQUESTS: pullRequestEnvFactory,
}

entityEnvs := make(map[minderv1.Entity]*entityEnvCache, len(factoryMap))
Expand Down Expand Up @@ -364,6 +373,8 @@ func inputAsMap(se *internalpb.SelectorEntity) (map[string]any, error) {
value = se.GetRepository()
case minderv1.Entity_ENTITY_ARTIFACTS:
value = se.GetArtifact()
case minderv1.Entity_ENTITY_PULL_REQUESTS:
value = se.GetPullRequest()
default:
return nil, fmt.Errorf("unsupported entity type [%d]: %s", se.GetEntityType(), se.GetEntityType().ToString())
}
Expand Down
97 changes: 90 additions & 7 deletions internal/engine/selectors/selectors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestNewSelectorEngine(t *testing.T) {
type testSelectorEntityBuilder func() *internalpb.SelectorEntity
type testRepoOption func(selRepo *internalpb.SelectorRepository)
type testArtifactOption func(selArtifact *internalpb.SelectorArtifact)
type testPrOption func(selPr *internalpb.SelectorPullRequest)

func newTestArtifactSelectorEntity(artifactOpts ...testArtifactOption) testSelectorEntityBuilder {
return func() *internalpb.SelectorEntity {
Expand Down Expand Up @@ -86,7 +87,7 @@ func withIsFork(isFork bool) testRepoOption {
}
}

func withProperties(properties map[string]any) testRepoOption {
func repoWithProperties(properties map[string]any) testRepoOption {
return func(selRepo *internalpb.SelectorRepository) {
protoProperties, err := structpb.NewStruct(properties)
if err != nil {
Expand All @@ -96,6 +97,36 @@ func withProperties(properties map[string]any) testRepoOption {
}
}

func prWithProperties(properties map[string]any) testPrOption {
return func(selPr *internalpb.SelectorPullRequest) {
protoProperties, err := structpb.NewStruct(properties)
if err != nil {
panic(err)
}
selPr.Properties = protoProperties
}
}

func newTestPullRequestSelectorEntity(prOpts ...testPrOption) testSelectorEntityBuilder {
return func() *internalpb.SelectorEntity {
pr := &internalpb.SelectorEntity{
EntityType: minderv1.Entity_ENTITY_PULL_REQUESTS,
Name: "testorg/testrepo/123",
Entity: &internalpb.SelectorEntity_PullRequest{
PullRequest: &internalpb.SelectorPullRequest{
Name: "testorg/testrepo/123",
},
},
}

for _, opt := range prOpts {
opt(pr.Entity.(*internalpb.SelectorEntity_PullRequest).PullRequest)
}

return pr
}
}

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

Expand Down Expand Up @@ -158,6 +189,28 @@ func TestSelectSelectorEntity(t *testing.T) {
selectorEntityBld: newTestRepoSelectorEntity(),
selected: false,
},
{
name: "Simple true pull request expression",
exprs: []*minderv1.Profile_Selector{
{
Entity: minderv1.PullRequestEntity.String(),
Selector: "pull_request.name == 'testorg/testrepo/123'",
},
},
selectorEntityBld: newTestPullRequestSelectorEntity(),
selected: true,
},
{
name: "Simple false pull request expression",
exprs: []*minderv1.Profile_Selector{
{
Entity: minderv1.PullRequestEntity.String(),
Selector: "pull_request.name != 'testorg/testrepo/123'",
},
},
selectorEntityBld: newTestPullRequestSelectorEntity(),
selected: false,
},
{
name: "Simple true generic entity expression for repo entity type",
exprs: []*minderv1.Profile_Selector{
Expand Down Expand Up @@ -294,7 +347,7 @@ func TestSelectSelectorEntity(t *testing.T) {
},
},
selectorEntityBld: newTestRepoSelectorEntity(
withProperties(map[string]any{
repoWithProperties(map[string]any{
"github": map[string]any{"is_fork": false},
}),
),
Expand All @@ -309,7 +362,7 @@ func TestSelectSelectorEntity(t *testing.T) {
},
},
selectorEntityBld: newTestRepoSelectorEntity(
withProperties(map[string]any{
repoWithProperties(map[string]any{
"license": "MIT",
}),
),
Expand All @@ -324,7 +377,7 @@ func TestSelectSelectorEntity(t *testing.T) {
},
},
selectorEntityBld: newTestRepoSelectorEntity(
withProperties(map[string]any{
repoWithProperties(map[string]any{
"license": "BSD",
}),
),
Expand All @@ -339,7 +392,7 @@ func TestSelectSelectorEntity(t *testing.T) {
},
},
selectorEntityBld: newTestRepoSelectorEntity(
withProperties(map[string]any{
repoWithProperties(map[string]any{
"github": map[string]any{"is_fork": true},
}),
),
Expand All @@ -354,7 +407,7 @@ func TestSelectSelectorEntity(t *testing.T) {
},
},
selectorEntityBld: newTestRepoSelectorEntity(
withProperties(map[string]any{
repoWithProperties(map[string]any{
"github": map[string]any{"is_fork": true},
}),
),
Expand Down Expand Up @@ -396,13 +449,43 @@ func TestSelectSelectorEntity(t *testing.T) {
WithUnknownPaths("repository.properties"),
},
selectorEntityBld: newTestRepoSelectorEntity(
withProperties(map[string]any{
repoWithProperties(map[string]any{
"github": map[string]any{"is_fork": true},
}),
),
expectedSelectErr: ErrResultUnknown,
selected: false,
},
{
name: "Use a PR property that is defined and true result",
exprs: []*minderv1.Profile_Selector{
{
Entity: minderv1.PullRequestEntity.String(),
Selector: "pull_request.properties.github['is_draft'] == false",
},
},
selectorEntityBld: newTestPullRequestSelectorEntity(
prWithProperties(map[string]any{
"github": map[string]any{"is_draft": false},
}),
),
selected: true,
},
{
name: "Use a PR property that is defined and false result",
exprs: []*minderv1.Profile_Selector{
{
Entity: minderv1.PullRequestEntity.String(),
Selector: "pull_request.properties.github['is_draft'] == false",
},
},
selectorEntityBld: newTestPullRequestSelectorEntity(
prWithProperties(map[string]any{
"github": map[string]any{"is_draft": true},
}),
),
selected: false,
},
}

for _, scenario := range scenarios {
Expand Down
Loading

0 comments on commit 9f15189

Please sign in to comment.