Skip to content

Support jira 9 #7112

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

Merged
merged 3 commits into from
Nov 20, 2022
Merged

Support jira 9 #7112

merged 3 commits into from
Nov 20, 2022

Conversation

pmilosev
Copy link
Contributor

Fixes: #6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).

Tested manually against JIRA 7.13.8, 8.22.6 and 9.3.1.

@pmilosev
Copy link
Contributor Author

some unit tests are failing - i'm working on it

@pmilosev
Copy link
Contributor Author

pmilosev commented Nov 14, 2022

OK I finally think I know why the tests are failing ...

In dojo/jira_link/helper.py I have the following code:

        if jira._version < (9, 0, 0):
            try:
                meta = jira.createmeta(
...
        else:
            try:
                issuetypes = jira.createmeta_issuetypes(project_key)
            except JIRAError as e:
...

This code works properly without the patch I added to pycontribs/jira#1527, as long as the JIRA instance version is below 9, as the old createmeta API will be triggered.
I assume the test setup reports JIRA version above or equal to 9 which triggers the newly added API, but since that patch is missing in the current version of the pycontrib/jira client, it results in:

2022-11-14T19:37:46.2208524Z uwsgi_1         | [14/Nov/2022 19:37:46] ERROR [dojo.jira_link.helper:1396] 'JIRA' object has no attribute 'createmeta_issuetypes'

If I'm not mistaken, the mock JIRA responses are configured in unittests/vcr/jira, but there I see all response bodies are base64 encoded binary blobs, such as:

...
  response:
    body:
      string: !!binary |
        H4sIAAAAAAAAA1SPS2vDMBCE/8teazsr+SFXt9Ic2lLSgp1TCUW2JOIiS8aSCyHkv1emoY/bMPvN
        zu4ZOuHVfjbA4RjC5PlmI5VWfZDuw2UiGOH9IGxmVYAEPtXsB2cjTBBJhhmmze7utXl4aX+nu2Xs
        ogL+tkIJJnhIQKrJuNOobGhPk4oL7o1bZAx1y2DkdwR4DFAsr+ZWhBWkSGmKdUpYizVHxmmZIeJN
        JDHmvZpjbzuM/9jbFpHTnJN4ZFH+sP34aLWLYFUQpnOtdc5qSivMa4ycoBXpBcqqZ6xQmFd/C4JZ
        G56GWcD6jhaLCc+uF6t9BnNVoOz7voHL5QsAAP//AwDEYVdgWgEAAA==
...

Decoding the base64 indeed results in a binary blob.
Currently I don't know what the binary blob is / how to parse it in order to verify my hypothesis that some of these responses indeed returns JIRA version 9+.

If this is true the easiest solution would be reducing that version to below JIRA 9 so that the new API is not triggered.
Of course the even better approach would be to add additional tests to demonstrate the new support for Jira 9, but currently this is way out of my competence or capacity.

@Maffooch any hint on how to view / edit the mock responses ?

Update:
In meantime I learned about vcrpy and recording gzipped streams. After decoding I indeed see that the Jira version is ..."version":"1001.0.0-SNAPSHOT","versionNumbers":[1001,0,0]... which match the current cloud Jira instance of defectDojo: https://defectdojo.atlassian.net/rest/api/2/serverInfo

I don't think it's possible to freeze the could Jira instance to an older version and re-record the responses. Also this is a valid error case (Jira client does not support / match Jira API) and should not be quietly suppressed in code. And it affects most (if not all) Jira tests so it doesn't make sense to disable certain tests.
The only solution I see ATM is to update the VCR recordings with version set to lower than Jira 9.

@Maffooch
Copy link
Contributor

@pmilosev
Good analysis of the unit tests. You are right, there is not a good way to test this without having an on premise version < 9.x and then recording new tests against that.

The best idea I have is to standup jira in a docker container and test against that.

@pmilosev
Copy link
Contributor Author

pmilosev commented Nov 15, 2022

For the manual testing that's what I did. I tested both the pycontrib/jira patch as well as defectDojo against my own instances of Jira 7.13.8, 8.22.6 and 9.3.1.
However the existing unit tests are failing and the only way to fix them is what I proposed above - decode the current recordings, change the version to something below 9.0.0 and save the changed responses. From what I've seen this only needs to be done for https://defectdojo.atlassian.net/rest/api/2/serverInfo calls, but still there are 122 of those, meaning I need a script to do it :) ... i'm working on it

@pmilosev
Copy link
Contributor Author

The last commit fixed 20+ failing tests, only two more to go :)
For future reference this is the script I used for updating the VCR recordings: https://gist.github.com/pmilosev/9e398d8567bb1f03a817ae249d52e1ae#file-dd-update-jira-version-py

@pmilosev pmilosev force-pushed the support-jira-9 branch 2 times, most recently from 22b4e5e to bac10c5 Compare November 15, 2022 13:35
@pmilosev
Copy link
Contributor Author

pmilosev commented Nov 15, 2022

@Maffooch (or some other maintainer) I need some help to understand what is going on.

I have the same unit test failing as in #7141 and #7130.
I synced the forks and rebased to make sure my commits are on top of the current dev branch.
Since I still got the same failure I tried to find a pull request that was merged and for which the unit tests passed and apply my commits on top of it. I did that on top of #7123.

Unfortunately the same unit test failure still persists which leads me to conflicting conclusions:

  • The same issue appears in other unrelated (even automated) PRs, which indicates the issue is not due to my changes.
  • The issue persists even after I rebase on top of a commit for which the unit tests were successful, which indicates it might be me who broke something in my branch. Unless Bump boto3 from 1.26.6 to 1.26.7 #7123 was based on a good commit, the unit tests were broken afterwards and it was merged on top of a commit that had failing tests ...

Anyway, I have spent several hours today on it and what I know is that:

  • A non empty list is expected but an empty query list is produced:
...
- 2022-11-15T14:46:24.8598105Z �[36muwsgi_1         |�[0m ======================================================================
2022-11-15T14:46:24.8598613Z �[36muwsgi_1         |�[0m FAIL: test_finding_queries (unittests.test_metrics_queries.FindingQueriesTest.test_finding_queries)
2022-11-15T14:46:24.8599123Z �[36muwsgi_1         |�[0m ----------------------------------------------------------------------
2022-11-15T14:46:24.8599528Z �[36muwsgi_1         |�[0m Traceback (most recent call last):
2022-11-15T14:46:24.8599965Z �[36muwsgi_1         |�[0m   File "/usr/local/lib/python3.11/unittest/mock.py", line 1359, in patched
2022-11-15T14:46:24.8600376Z �[36muwsgi_1         |�[0m     return func(*newargs, **newkeywargs)
2022-11-15T14:46:24.8600714Z �[36muwsgi_1         |�[0m            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2022-11-15T14:46:24.8601336Z �[36muwsgi_1         |�[0m   File "/app/unittests/test_metrics_queries.py", line 77, in test_finding_queries
2022-11-15T14:46:24.8601742Z �[36muwsgi_1         |�[0m     self.assertSequenceEqual(
2022-11-15T14:46:24.8602279Z �[36muwsgi_1         |�[0m AssertionError: Sequences differ: <TagulousCastTaggedQuerySet []> != [{'id': 226, 'title': 'Test Endpoint Miti[10454 chars]lse}]
...
  • It used to be the case that empty list was expected until few weeks ago, when this test was updated by @kiblik (git blame unittests/test_metrics_queries.py):
...
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    1                 [
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    2                     {'id': 226, 'title': 'Test Endpoint Mitigation - Finding F1 Without Endpoints', 'date': date(2022, 10, 15), 'sla_start_date': None, 'cwe': None, 'cve': None, 'cvssv3': None, 'cvssv3_score': None, 'url': None, 'sever
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    3                     {'id': 227, 'title': 'Test Endpoint Mitigation - Finding F2 With Many Endpoints', 'date': date(2022, 10, 15), 'sla_start_date': None, 'cwe': None, 'cve': None, 'cvssv3': None, 'cvssv3_score': None, 'url': None, 'sev
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    4                     {'id': 228, 'title': 'Test Endpoint Mitigation - Finding F3 EPS False Positive', 'date': date(2022, 10, 15), 'sla_start_date': None, 'cwe': None, 'cve': None, 'cvssv3': None, 'cvssv3_score': None, 'url': None, 'seve
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    5                     {'id': 229, 'title': 'Test Endpoint Mitigation - Finding F4 EPS Out of Scope', 'date': date(2022, 10, 15), 'sla_start_date': None, 'cwe': None, 'cve': None, 'cvssv3': None, 'cvssv3_score': None, 'url': None, 'severi
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    6                     {'id': 230, 'title': 'Test Endpoint Mitigation - Finding F5 EPS Risk Accepted', 'date': date(2022, 10, 15), 'sla_start_date': None, 'cwe': None, 'cve': None, 'cvssv3': None, 'cvssv3_score': None, 'url': None, 'sever
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    7                     {'id': 231, 'title': 'Test Endpoint Mitigation - Finding F6 Mitigated', 'date': date(2022, 10, 15), 'sla_start_date': None, 'cwe': None, 'cve': None, 'cvssv3': None, 'cvssv3_score': None, 'url': None, 'severity': 'I
e65694307e (kiblik              2022-10-26 16:46:26 +0200)│    8                 ]
...
  • The updated tests appear to be working for some time and then got broken.
  • The test data seem to match the assertion in the test (git blame dojo/fixtures/dojo_testdata.json):
...
e65694307e (kiblik              2022-10-26 16:46:26 +0200 1711)       "numerical_severity": "S4",
e65694307e (kiblik              2022-10-26 16:46:26 +0200 1712)       "hash_code": "a6dd6bd359ff0b504a21b8a7ae5e59f1b40dd0fa1715728bd58de8f688f01b19",
e65694307e (kiblik              2022-10-26 16:46:26 +0200 1713)       "static_finding": false,
e65694307e (kiblik              2022-10-26 16:46:26 +0200 1714)       "dynamic_finding": true,
...

I have very little experience with this technology stack and can not figure out who and how this data is loaded and why it ends up in an empty list. I initially thought that by modifying the VCR recordings I might have broken some hash codes etc. but this doesn't seem to be the case. Now I suspect that the issue is outside of my PR.

I'm running out of time to work on this pull request. I would appreciate any help to get it to a state where it can be merged.

TL;DR

  • Could my changes (jira client, jira tests, jira API recordings) cause this issue ?
  • If not, should I rebase to some older commit so that the cnahges could be merged and the failing test debugged independently ?

@kiblik
Copy link
Contributor

kiblik commented Nov 15, 2022

Hi guys (mainly @pmilosev),

my PR #6193 was quite a huge change. It dropped redundancy in finding-endpoint mapping but it also changed how some endpoint/findings are marked as vulnerable. Because of this, I was forced to update also unittests/test_metrics_queries.py. The area around metrics_queries is not my main field of expertise but these changes made sense to me at that time and I fixed them based on the test's expected results. There is chance I made mistake but I expected it will be discovered during peer-review.

I really would like to know this part better but I do not have time for deep dive to this part for now. Maybe empty list is right value in this place.

@kiblik
Copy link
Contributor

kiblik commented Nov 15, 2022

I'm just working on #7002 and I noticed the same error. So it is definitely a mistake outside of #7002 and #7112.

@Maffooch
Copy link
Contributor

I am looking into this. There are a few users that are experiencing this issue. Appears to be something wrong with the 2.16.2 release, but am not sure what exactly. All tests had passed on dev before the release, and now something is broken.

@kiblik
Copy link
Contributor

kiblik commented Nov 15, 2022

I just prepared #7142. It fixes that test. I rolled-back 1/2 of the file from #7142. Do not understand why there is this kind for chaos.

@Maffooch
Copy link
Contributor

#7142 has been merged. Please pull the latest dev branch to get that unit test to pass.

For the VCR unit tests, I do not think it changing all the unit tests to only test against versions < 9.x since that totally removes test coverage for the cloud offerings. I think it would be better to record a new test that specifically tests against version < 9 rather than changing all tests.

@pmilosev
Copy link
Contributor Author

Thanks a lot, I'll look into this tomorrow.

Regarding the VCR, I only changed the recorded Jira version which doesn't seem to be used in the unit tests for decision making, so all tests will work as they did before the PR. However this discussion is probably irrelevant as motivated by your comment and the odd cloud version 1001 I looked a little bit closer to see which Jira release version does this cloud version match, and I ran into this threat https://community.atlassian.com/t5/Jira-questions/How-can-I-tell-actual-Jira-version-of-a-cloud-instance-for/qaq-p/1483730.

Not only cloud Jira is a rolling release that rolls out in steps (two cloud accounts may have different features available), but also the cloud Jira API is independent and sometimes different from on-prem versions of Jira. This means all integrating apps that support both must account for these differences.
Luckily createmeta endpoint is the same (for now) between cloud JIRA and Jira DC prior 8.4. Therefore I will rollback the VCR changes and will have to account for the cloud API in my version checks on which API to be used.

renovate bot and others added 3 commits November 17, 2022 22:55
No change in behaviour (except perhaps in logging).
Fixes: DefectDojo#6963
Depends on: pycontribs/jira#1527

If you don't have the update in the mentioned dependency than behaves as it used to - works until Jira 9 and breaks afterwards.
If you update the dependency than it will continue working after Jira 9.
With the updated dependency it will throw deprecation warnings on Jira versions [8.4 - 9).
@pmilosev
Copy link
Contributor Author

It's a bit messy history, but I finally cleaned up everything:

  • Added a check if the Jira deployment type is Cloud
  • Reverted back the VCR recordings
  • Synced, updated, merged to latest dev
  • Squashed all the intermediary commits to the one where the support for Jira 9 was added.
  • Both unit and integration tests complete successfully

I can only see that due to the updating and merging commit b47e8f9 is part of my branch now that introduces Dcoker tag bump from 1.33.0 to 1.33.1 (this is already the case on dev). I'm not sure if I should let it stay or get rid of this commit here.

Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

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

Looks good @pmilosev

Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

@mtesauro mtesauro merged commit 7e25be9 into DefectDojo:dev Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants