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.

Sign up for GitHub

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

Jump to bottom

Remove tasks and items on mutation delete #1090

Merged
merged 14 commits into from
Jun 9, 2023
Merged

Remove tasks and items on mutation delete #1090

merged 14 commits into from
Jun 9, 2023

Conversation

jpbruinsslot
Copy link
Contributor

Changes

  • When an ooi is deleted, the scheduler gets a message from the message queue from octopoes, the associated tasks are found and updated to 'cancelled'. Additionally items on the queue are then deleted
  • When an ooi is deleted, no additional boefjes should be scheduled.

Issue link

#1042

@jpbruinsslot jpbruinsslot added the mula Issues related to the scheduler label Jun 1, 2023
@jpbruinsslot jpbruinsslot self-assigned this Jun 1, 2023
@jpbruinsslot jpbruinsslot linked an issue Jun 1, 2023 that may be closed by this pull request
On scan profile mutations update the queue in the scheduler #1042
Closed
@jpbruinsslot jpbruinsslot mentioned this pull request Jun 1, 2023
Closed
@jpbruinsslot jpbruinsslot marked this pull request as ready for review June 1, 2023 12:16
@jpbruinsslot jpbruinsslot requested a review from a team as a code owner June 1, 2023 12:16
dekkers
dekkers previously approved these changes Jun 5, 2023
View reviewed changes
mula/scheduler/schedulers/boefje.py Outdated Show resolved Hide resolved
@Darwinkel
Copy link
Contributor

Checklist for QA:

  • I have checked out this branch, and successfully ran a fresh make reset.
  • I confirmed that there are no unintended functional regressions in this branch:
    • I have managed to pass the onboarding flow
    • Objects and Findings are created properly
    • Tasks are created and completed properly
  • I confirmed that the PR's advertised feature or hotfix works as intended.

What works:

  • Queued tasks are cancelled when the related OOI is deleted
  • No new boefjes are scheduled when an OOI is deleted

Bug or feature?:

  • When a long-running boefje is in progress (such as nmap), deleting the OOI does not cancel the job. It is completed normally, and the OOI is re-created. Desired behavior or no?

Donnype
Donnype approved these changes Jun 7, 2023
View reviewed changes
mula/scheduler/repositories/sqlalchemy/task_store.py
@@ -144,3 +96,69 @@ def create_task(self, task: models.Task) -> Optional[models.Task]:
def update_task(self, task: models.Task) -> None:
with self.datastore.session.begin() as session:
(session.query(models.TaskORM).filter(models.TaskORM.id == task.id).update(task.dict()))

@retry()
def api_list_tasks(
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is called only by the API, or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes exactly

mula/tests/integration/test_boefje_scheduler.py
self.assertEqual(boefje.id, task_pq.boefje.id)

task_db = self.mock_ctx.task_store.get_task_by_id(task_pq.id)
self.assertEqual(task_db.status, models.TaskStatus.CANCELLED)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@Donnype
Copy link
Contributor

Donnype commented Jun 7, 2023

@Darwinkel If that's desired behavior I'd suggest this turns into a new ticket because our workers do not support such functionality (yet). This also ties a bit into the timeout discussion we had recently.

@dekkers dekkers mentioned this pull request Jun 9, 2023
Open
@dekkers dekkers merged commit c0e41a7 into main Jun 9, 2023
@dekkers dekkers deleted the feature/mula/queue-update-rebase branch June 9, 2023 08:26
jpbruinsslot added a commit that referenced this pull request Jun 12, 2023
@jpbruinsslot
Merge branch 'main' into feature/mula/improve-ranking
6a013bc 
* main:
  Add bit to set default values for FindingType risk levels in Octopoes (#1075)
  Fix thread termination in Mula (#1003)
  test(boefjes): snyk (#1116)
  Add endpoints in Octopoes for bulk operations in the object list page (#1067)
  Remove tasks and items on mutation delete (#1090)
  Persist impact, recommendation and source fields in FindingType objects in XTDB (#1126)
  Handle an empty plugin.consumes field for the plugin detail page (#1104)
  Add script to automatically backport PR to release branch (#1097)
  Fix typos in 'no organizations found' message (#1123)
  Finding Types Boefjes (#1056)
  add findingtype files (#1117)
  Remove containers after `docker-compose run` (#1112)
  Bump cryptography from 39.0.1 to 41.0.0 in /boefjes/boefjes/plugins/kat_ssl_certificates (#1099)
  Bump cryptography from 40.0.2 to 41.0.1 (#1108)
  Bump cryptography from 40.0.2 to 41.0.0 in /bytes (#1100)
  Fix failing test-debian-install in CI (#1111)
  Remove unused boefje fields when creating a BoefjeTask object to send to the scheduler (#1103)
  add 'ideas' as a category in project guidelines (#1105)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Donnype Donnype Donnype approved these changes

@dekkers dekkers dekkers approved these changes

Assignees

@jpbruinsslot jpbruinsslot

Labels
mula Issues related to the scheduler
Projects
KAT
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

On scan profile mutations update the queue in the scheduler
5 participants
@jpbruinsslot @Darwinkel @Donnype @dekkers @praseodym