Skip to content

Commit

Permalink
task sync manager preparations (#835)
Browse files Browse the repository at this point in the history
This PR is a move towards async task management. It aims to simplify the
stuff a bit and remove some more unnecessary Jira API calls. This time
we focus on the calls which sync back the task right after we updated
it. We will just assume that when the update is successful the Jira
issue will have the values we set. We also set the task last update
timestamp on our side which may be a little bit before the actual update
in Jira but will be synced by the collector in a short while so no big
deal. A number of test changes were necessary along the way.

I added some more parts. I reworked the decision logic regarding the
task management and moved some functionality so the execution parts are
at the end separated methods which can be called both synchronously and
async.

Then I run into an issue that the broken way serializers update the
flaws (and other and create but that was put aside) means that the
**save** and all the parameters are not used all at once but it is
called multiple times with different context - or **tasksync** used to
be called completely separately outside of the `save` call. It required
quite some serializer rework which is definitely not complete but
already brought what I needed. I think that the result is actually
simpler and more understandable then it was before but I am glad for any
feedback.
  • Loading branch information
osoukup authored Dec 10, 2024
2 parents 62ca7bd + a3a8689 commit 83ae2f4
Show file tree
Hide file tree
Showing 20 changed files with 4,658 additions and 12,694 deletions.
6 changes: 3 additions & 3 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
"filename": "apps/taskman/tests/test_flaw_model_integration.py",
"hashed_secret": "3c3b274d119ff5a5ec6c1e215c1cb794d9973ac1",
"is_verified": false,
"line_number": 156,
"line_number": 126,
"is_secret": false
}
],
Expand All @@ -184,7 +184,7 @@
"filename": "apps/workflows/tests/test_endpoints.py",
"hashed_secret": "3c3b274d119ff5a5ec6c1e215c1cb794d9973ac1",
"is_verified": false,
"line_number": 294,
"line_number": 283,
"is_secret": false
}
],
Expand Down Expand Up @@ -356,7 +356,7 @@
"filename": "osidb/tests/endpoints/flaws/test_package_versions.py",
"hashed_secret": "3c3b274d119ff5a5ec6c1e215c1cb794d9973ac1",
"is_verified": false,
"line_number": 138,
"line_number": 141,
"is_secret": false
}
],
Expand Down
3 changes: 3 additions & 0 deletions apps/bbsync/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,9 @@ def test_bz_id(self):


class TestFlawDraftBBSyncIntegration:
@freeze_time(
timezone.datetime(2020, 12, 12)
) # freeze against top of the second crossing
@pytest.mark.vcr
@pytest.mark.enable_signals
@pytest.mark.parametrize(
Expand Down
2 changes: 2 additions & 0 deletions apps/taskman/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,7 @@
"is_embargoed",
"owner",
"team_id",
]
TRANSITION_REQUIRED_FIELDS = [
"workflow_state",
]
10 changes: 8 additions & 2 deletions apps/taskman/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class JiraTaskSyncMixin(models.Model):
class Meta:
abstract = True

def save(self, *args, diff=None, jira_token=None, **kwargs):
def save(self, *args, diff=None, force_creation=False, jira_token=None, **kwargs):
"""
save the model and sync it to Jira
Expand All @@ -26,7 +26,13 @@ def save(self, *args, diff=None, jira_token=None, **kwargs):
# check taskman conditions are met
# and eventually perform the sync
if JIRA_TASKMAN_AUTO_SYNC_FLAW and jira_token is not None:
self.tasksync(*args, diff=diff, jira_token=jira_token, **kwargs)
self.tasksync(
*args,
diff=diff,
force_creation=force_creation,
jira_token=jira_token,
**kwargs
)

def tasksync(self, *args, jira_token, force_creation=False, **kwargs):
"""
Expand Down
72 changes: 43 additions & 29 deletions apps/taskman/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@
"""
import json
import logging
from typing import List, Tuple
from typing import List, Optional, Tuple

from django.db import models
from django.utils import timezone
from jira import Issue
from jira.exceptions import JIRAError
from rest_framework.response import Response

from apps.trackers.jira.query import JiraPriority
from collectors.jiraffe.core import JiraQuerier
Expand Down Expand Up @@ -108,9 +107,12 @@ def _check_token(self) -> None:
f"Token is valid for {JIRA_TASKMAN_URL} but user doesn't have write permission in {JIRA_TASKMAN_PROJECT_KEY} project."
)

def create_or_update_task(self, flaw: Flaw, check_token: bool = True) -> Response:
def create_or_update_task(
self, flaw: Flaw, check_token: bool = True
) -> Optional[str]:
"""
Creates or updates a task using Flaw data
returns the Jira task ID if newly created
by default the user tokens are being checked for validity which can
be turned off by parameter if not necessary to lower the Jira load
Expand All @@ -131,39 +133,15 @@ def create_or_update_task(self, flaw: Flaw, check_token: bool = True) -> Respons
)
flaw.task_key = issue.key
if flaw.team_id: # Jira does not allow setting team during creation
return self.create_or_update_task(
self.create_or_update_task(
flaw, check_token=False # no need to check the token again
)
return Response(data=issue.raw, status=201)
return flaw.task_key
else: # task exists; update
url = f"{self.jira_conn._get_url('issue')}/{flaw.task_key}"
if flaw.team_id:
data["fields"]["customfield_12313240"] = flaw.team_id
self.jira_conn._session.put(url, json.dumps(data))

status, resolution = flaw.jira_status()
issue = self.jira_conn.issue(flaw.task_key).raw
if (
(resolution and not issue["fields"]["resolution"])
or (
issue["fields"]["resolution"]
and issue["fields"]["resolution"]["name"] != resolution
)
or status != issue["fields"]["status"]["name"]
):
resolution_data = (
{"resolution": {"name": resolution}} if resolution else {}
)
self.jira_conn.transition_issue(
issue=flaw.task_key,
transition=status,
**resolution_data,
)
return Response(
data=self.jira_conn.issue(flaw.task_key).raw,
status=200,
)
return Response(data=issue, status=200)
except JIRAError as e:
creating = not flaw.task_key
creating_updating_word = "creating" if creating else "updating"
Expand All @@ -181,6 +159,42 @@ def create_or_update_task(self, flaw: Flaw, check_token: bool = True) -> Respons
# create_or_update_task is used.
raise JiraTaskErrorException(message)

def transition_task(self, flaw: Flaw, check_token: bool = True) -> None:
"""
transition a task through the Jira workflow using Flaw data
by default the user tokens are being checked for validity which can
be turned off by parameter if not necessary to lower the Jira load
"""
# check the token validity in case the user token is used
# assuming the service token is valid lowering Jira load
if check_token and not self.is_service_account():
self._check_token()

# when there is no task we assume that the caller
# made a mistake and simply refuse the operation
if not flaw.task_key:
raise JiraTaskErrorException(
f"Cannot promote flaw {flaw.cve_id or flaw.uuid} without an associated task."
)

try:
status, resolution = flaw.jira_status()
resolution_data = {"resolution": {"name": resolution}} if resolution else {}
self.jira_conn.transition_issue(
issue=flaw.task_key,
transition=status,
**resolution_data,
)
except JIRAError as e:
# raising so that the error from Jira is communicated to the client
raise JiraTaskErrorException(
f"Jira error when transitioning "
f"Task for Flaw UUID {flaw.uuid} cve_id {flaw.cve_id}. "
f"Jira HTTP status code {e.status_code}, "
f"Jira response {safe_get_response_content(e.response)}"
)

def _generate_task_data(self, flaw: Flaw):
modules = flaw.affects.values_list("ps_module", flat=True).distinct()
products = PsProduct.objects.filter(ps_modules__name__in=modules)
Expand Down
Loading

0 comments on commit 83ae2f4

Please sign in to comment.