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

new jira type trackers #608

Merged
merged 6 commits into from
Aug 29, 2024
Merged

new jira type trackers #608

merged 6 commits into from
Aug 29, 2024

Conversation

jsvob
Copy link
Contributor

@jsvob jsvob commented Jun 26, 2024

Related to OSIDB-2980

@jsvob jsvob force-pushed the new_jira_type_tracker_wip_tmp_notes branch from 66cff9a to 13445b6 Compare August 6, 2024 10:17
@jsvob jsvob force-pushed the new_jira_type_tracker_wip_tmp_notes branch 2 times, most recently from c0f6fbd to 9e29bde Compare August 19, 2024 18:55
@jsvob jsvob force-pushed the new_jira_type_tracker_wip_tmp_notes branch 2 times, most recently from 4eadde8 to b43453d Compare August 21, 2024 19:47
@jsvob jsvob changed the title WIP notes for new jira type trackers WIP: new jira type trackers Aug 21, 2024
@jsvob jsvob requested a review from a team August 21, 2024 19:48
@jsvob
Copy link
Contributor Author

jsvob commented Aug 21, 2024

@RedHatProductSecurity/osidb-devs Please pre-review if you have spare cycles on Thursday Aug 22 or Monday Aug 26. OSIDB-2980 contains a comment with a link to a relevant slack thread. Next week I will rebase it to modern version of master, will create the appropriate file in ps-constants and try to write a few smoke tests. My intention is to have it merged after Aug 26 and write the rest of the tests in parallel so that it can be tested with a few brave projects, while the rest stays with the old and tested Bug issuetype (if that's not OK, please provide adequate reasoning so that delay reasons are clear for management). Thank you!

@jsvob
Copy link
Contributor Author

jsvob commented Aug 21, 2024

Reviewing commit-by-commit is probably more pleasant than the whole diff.

Copy link
Contributor

@osoukup osoukup left a comment

Choose a reason for hiding this comment

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

Mostly LGTM and I think this goes in the right direction. I have a number of comments but most of them are regarding the multi-flaw trackers and rare cases so they do not have to be necessarily addressed in the short term.

apps/trackers/jira/query.py Show resolved Hide resolved
apps/trackers/jira/query.py Outdated Show resolved Hide resolved
apps/trackers/jira/query.py Outdated Show resolved Hide resolved
apps/trackers/jira/query.py Outdated Show resolved Hide resolved
apps/trackers/jira/query.py Show resolved Hide resolved
apps/trackers/jira/query.py Show resolved Hide resolved
apps/trackers/jira/query.py Outdated Show resolved Hide resolved
apps/trackers/jira/query.py Show resolved Hide resolved
apps/trackers/jira/query.py Outdated Show resolved Hide resolved
osidb/serializer.py Outdated Show resolved Hide resolved
Copy link
Contributor

@JakubFrejlach JakubFrejlach left a comment

Choose a reason for hiding this comment

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

Couple of suggestions/observations.

collectors/ps_constants/tests/test_core.py Outdated Show resolved Hide resolved
apps/trackers/jira/query.py Outdated Show resolved Hide resolved
osidb/serializer.py Outdated Show resolved Hide resolved
@jsvob
Copy link
Contributor Author

jsvob commented Aug 27, 2024

@RedHatProductSecurity/osidb-devs Ready for re-review with two caveats: The code that fetches jira_bug_issuetype.yml doesn't work yet for some reason and I yet have to test the two fields marked with # TODO test it with real jira with stage Jira. I'll fix these caveats Wednesday morning, and the fixes will be in separate (probably small) commits. Thank you!

@jsvob
Copy link
Contributor Author

jsvob commented Aug 28, 2024

The two methods marked with # TODO test it with real jira are currently untestable, because the stage jira still doesn't have the required fields. I'll deal with that in parallel.

@RedHatProductSecurity/osidb-devs please review

@jsvob jsvob requested a review from a team August 28, 2024 08:40
@jsvob jsvob changed the title WIP: new jira type trackers new jira type trackers Aug 28, 2024
@jsvob jsvob marked this pull request as ready for review August 28, 2024 08:40
@jsvob
Copy link
Contributor Author

jsvob commented Aug 28, 2024

I'll squash and rebase before merge, so that I don't make it even more confusing by changing history.

@jsvob
Copy link
Contributor Author

jsvob commented Aug 28, 2024

The two methods marked with # TODO test it with real jira finally manually tested with stage jira.

Copy link
Contributor

@osoukup osoukup left a comment

Choose a reason for hiding this comment

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

LGTM

osidb/models.py Outdated
@@ -3501,6 +3501,18 @@ class SpecialConsiderationPackage(models.Model):
name = models.CharField(max_length=255, unique=True)


class JiraBugIssuetype(models.Model):
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal but I think that logically this should rather belong to
https://github.com/RedHatProductSecurity/osidb/blob/master/apps/trackers/models.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved in a new commit. I'll fix the commit & migration history mess during squash and rebase.

name=self.affects.first().ps_module
).bts_key
).exists():
actual_jira_issuetype = "Bug"
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing to be careful about is that before the first PS constants collection after the release we are actually not going to default to the old behavior but to the new one. We could overcome it by checking whether there is actually at least some JiraBugIssuetype but that might that cause problems when all of them adapt and the field gets empty. Otherwise we could manually do the sync in console right after the release or do it eg. inside a migration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would we make it behave sanely once the last project switches to Vulnerability issuetype? If we add a check here and now, then in that distant (?) point in future, it will trigger confusing and already-forgotten behavior of switching all new trackers back to Bug. Maybe I'm missing something.

Doing the first sync manually sounds good to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I am OK with it too. Just keep it in mind before we go to production and we should be ready to just hit a command and run it as I think for the PS constants last time it was quite laborious.

Copy link
Contributor

@JakubFrejlach JakubFrejlach left a comment

Choose a reason for hiding this comment

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

LGTM as well!

apps/trackers/jira/query.py Outdated Show resolved Hide resolved
apps/trackers/jira/query.py Outdated Show resolved Hide resolved
Prepares for "new" Vulnerability issuetype-based TrackerJiraQueryBuilder.
Related to OSIDB-2980
Prepares for "new" Vulnerability issuetype-based TrackerJiraQueryBuilder.
Related to OSIDB-2980
Prepares for "new" Vulnerability issuetype-based TrackerJiraQueryBuilder.
Related to OSIDB-2980
Add switching between "new" Vulnerability issuetype and "old" Bug
issuetype using ps-constants.
Related to OSIDB-2980
even on debug-disabled deployments.
This is important because we expect to encounter bugs in Jira
configuration when creating trackers.
Related to OSIDB-2980
@jsvob jsvob force-pushed the new_jira_type_tracker_wip_tmp_notes branch from 29fd912 to 18ab156 Compare August 28, 2024 19:25
@jsvob jsvob enabled auto-merge August 29, 2024 08:43
@jsvob jsvob added this pull request to the merge queue Aug 29, 2024
Merged via the queue into master with commit df76915 Aug 29, 2024
11 checks passed
@jsvob jsvob deleted the new_jira_type_tracker_wip_tmp_notes branch August 29, 2024 09:00
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 this pull request may close these issues.

3 participants