Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hook properties fetching in executor #4173

Closed
JAORMX opened this issue Aug 16, 2024 · 0 comments · Fixed by #4377
Closed

Hook properties fetching in executor #4173

JAORMX opened this issue Aug 16, 2024 · 0 comments · Fixed by #4377
Assignees

Comments

@JAORMX
Copy link
Contributor

JAORMX commented Aug 16, 2024

This will allow the executor to leverage properties for filtering.

jhrozek added a commit to jhrozek/minder that referenced this issue Aug 29, 2024
Changes fetchRepo to refresh the repository being processed and return
an EntityInstance model structure instead of a database repo

Related: stacklok#4173
jhrozek added a commit to jhrozek/minder that referenced this issue Aug 29, 2024
Changes fetchRepo to refresh the repository being processed and return
an EntityInstance model structure instead of a database repo

Related: stacklok#4173
jhrozek added a commit that referenced this issue Aug 29, 2024
…4299)

* Accept transaction from outside in the property service

If we had called these methods in a transaction, we would have
effectivelly created another parallel transaction which might cause
entities to not be referenced correctly.

* Add FilteredCopy and Merge to properties

These will be useful later.

* Add EntityInstance model

This is just a shortcut to avoid passing the properties and project ID
and entity etc around

* Decouple entities from fetching properties

This makes it easier to handle multiple entities inside the github
provider as all that is needed is to implement the PropertyFetcher
interface. It also makes moves the logic around formatting the name and
what properties are operational into the provider to code that is
per-entity.

* Handle operational attributes

minder saves attributes like hook ID which do not come from the upstream
entity. We need to avoid deleting them on updates.

* Add a method to refresh a repository + lazy migration into the github repository service

Related: #4179

* Switch over webhook handling to use property entities

Changes fetchRepo to refresh the repository being processed and return
an EntityInstance model structure instead of a database repo

Related: #4173

* Propagate 404 from the property wrappers as a special error code
jhrozek added a commit to jhrozek/minder that referenced this issue Aug 30, 2024
Refreshing the repositories ensures that when the requests get to the
executor, the properties will have been fetched.

Related: stacklok#4173
jhrozek added a commit that referenced this issue Aug 30, 2024
Refreshing the repositories ensures that when the requests get to the
executor, the properties will have been fetched.

Related: #4173
jhrozek added a commit to jhrozek/minder that referenced this issue Sep 5, 2024
We can simplify the GetEntityByID call to not accept providerID since
the call searches on the globally-unique and indexed entity ID.

Related: stacklok#4173
jhrozek added a commit to jhrozek/minder that referenced this issue Sep 5, 2024
Adds a convenience function to search an entity by ID with all its
properties. Also moves two utility functions form the property service
here.

Related: stacklok#4173
jhrozek added a commit to jhrozek/minder that referenced this issue Sep 5, 2024
This utility function will allow us to use properties both in selectors
that use the structupb.Struct and in the general entity protobuf message
that we added recently.

Fixes: stacklok#4173
jhrozek added a commit to jhrozek/minder that referenced this issue Sep 5, 2024
This is so unit tests can set int64 properties.

Related: stacklok#4173
jhrozek added a commit to jhrozek/minder that referenced this issue Sep 5, 2024
Changes the ruletype test command to not use an EntityInfoWrapper as
intput, but just properties. An example of a repo entity with properties
is:
```
name: jakubtestorg/bad-python
is_private: false
is_fork: false
is_archived: false
upstream_id: "706108924"
github/repo_id:
  minder.internal.type: int64
    minder.internal.value: "706108924"
github/repo_name: bad-python
github/repo_owner: jakubtestorg
github/default_branch: master
foo: bar
```

Adds a lookup of an entity with properties to the evaluation engine so
that the resulting selectorEntity is augmented with properties and
passed to the selector engine. Changes the conversion interfaces to
selector entities to only use the EntityWithProperties model.

As a result, the user can use properties in selectors such as:
```
  - description: This should not apply copyleft repos
    entity: repository
    selector: repository.properties['github/license'] != 'GPL'
```
And in future allow us to extend selectors based on any other properties
we add.

Fixes: stacklok#4173
jhrozek added a commit that referenced this issue Sep 5, 2024
* GetEntityByID only accepts an ID since it's unique

We can simplify the GetEntityByID call to not accept providerID since
the call searches on the globally-unique and indexed entity ID.

Related: #4173

* Add GetEntityWithPropertiesByID to our entity model package

Adds a convenience function to search an entity by ID with all its
properties. Also moves two utility functions form the property service
here.

Related: #4173

* Add ToProtoStruct to properties

This utility function will allow us to use properties both in selectors
that use the structupb.Struct and in the general entity protobuf message
that we added recently.

Fixes: #4173

* Add an option to NewProperties to skip checking the internal prefix

This is so unit tests can set int64 properties.

Related: #4173

* Take properties into use in selectors and ruletype test.

Changes the ruletype test command to not use an EntityInfoWrapper as
intput, but just properties. An example of a repo entity with properties
is:
```
name: jakubtestorg/bad-python
is_private: false
is_fork: false
is_archived: false
upstream_id: "706108924"
github/repo_id:
  minder.internal.type: int64
    minder.internal.value: "706108924"
github/repo_name: bad-python
github/repo_owner: jakubtestorg
github/default_branch: master
foo: bar
```

Adds a lookup of an entity with properties to the evaluation engine so
that the resulting selectorEntity is augmented with properties and
passed to the selector engine. Changes the conversion interfaces to
selector entities to only use the EntityWithProperties model.

As a result, the user can use properties in selectors such as:
```
  - description: This should not apply copyleft repos
    entity: repository
    selector: repository.properties['github/license'] != 'GPL'
```
And in future allow us to extend selectors based on any other properties
we add.

Fixes: #4173

* Move GetEntityWithPropertiesByID from models to service
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants