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

The CodeQL analysis is broken due to the large content of the generated SARIF file #10203

Closed
abstractj opened this issue Feb 14, 2022 · 5 comments · Fixed by #10606
Closed

The CodeQL analysis is broken due to the large content of the generated SARIF file #10203

abstractj opened this issue Feb 14, 2022 · 5 comments · Fixed by #10606
Assignees
@abstractj
Labels
area/ci Indicates an issue on the CI kind/bug Categorizes a PR related to a bug
Milestone
18.0.0

Comments

@abstractj
Copy link
Contributor

abstractj commented Feb 14, 2022 •
edited
Loading

Describe the bug

The CodeQL analysis is broken due to the large content of the generated SARIF file.

Version

17.0.0

Anything else?

Ref: This is a follow up of #10108 (comment)
@abstractj abstractj added kind/bug Categorizes a PR related to a bug status/triage labels Feb 14, 2022
@abstractj abstractj self-assigned this Feb 14, 2022
@abstractj abstractj mentioned this issue Feb 14, 2022
Closed
@abstractj abstractj changed the title The CodeQL analysis is broken due to the large content of the generated SARIF file. The CodeQL analysis is broken due to the large content of the generated SARIF file Feb 14, 2022
@stianst stianst added the area/ci Indicates an issue on the CI label Feb 16, 2022
@andreaTP
Copy link
Contributor

Is it an option to temporary disable the codeql analysis until we have a solution?

Currently the CodeQL analysis job in GH Action is keeping busy 1 runner for 40 minutes for each PR (out of 20 total runners available), this is seriously impacting every PR validation performance.

@abstractj
Copy link
Contributor Author

@andreaTP please let's use this issue to discuss the solution. For requests like that, we can use other communication channels.

@DGuhr
Copy link
Contributor

DGuhr commented Feb 17, 2022

I just found github/codeql-action#820, seems to be kind of the same issue. perhaps we should also post a failing workflow run there and see if they provide some sugggestions? Also, there seems to be some suggestions inside (just scanned over it, though)

@abstractj
Copy link
Contributor Author

abstractj commented Feb 17, 2022 •
edited
Loading

Update:

The GitHub documentation states that paths-ignore is only available in the context of interpreted languages, for compiled languages like Java it's necessary to change the build steps to exclude the directories we need like: testsuite, examples, etc.

For that, it's necessary to remove autobuild from the CodeQL workflow and provide the custom build steps. The initial attempt was to skip all the tests, and also the adapters, but that did not reduce the size of the SARIF file, nor prevented the compilation of the testsuite:

- if: matrix.language == 'java' 
      name: Build Java
      run: |
        cd quarkus && mvn -f ../pom.xml clean install -Dmaven.test.skip -DskipAdapters -DskipTestsuite -DskipExamples -DskipTests

You can find the complete file here. @JoshuaMulliken may have other ideas. If there's anything that you would like to suggest as a fix, please let us know.

@abstractj
Copy link
Contributor Author

@andreaTP @DGuhr FYI #10354

@abstractj abstractj mentioned this issue Mar 4, 2022
Merged
stianst pushed a commit that referenced this issue Mar 11, 2022
@abstractj
The CodeQL analysis is broken due to the large content of the SARIF f…
f06ba05 
…ile (#10606)

The issue was originally caused by high number of flows paths per alert
generated by the LDAP federation module. That was identified taking the
SARIF file generated and running:

```
jq '.runs[0].results | map({query_id: .rule.id, numPaths: .codeFlows |
length})' java.sarif

```

Together we reduced the number of flows paths, adding optimizations to
skip some paths and avoid false alerts.

Co-authored-by: Bruno Oliveira da Silva <bruno@abstractj.com>

Closes #10203

Co-authored-by: Joshua Mulliken <joshua@mulliken.net>
@stianst stianst added this to the 18.0.0 milestone Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@abstractj abstractj

Labels
area/ci Indicates an issue on the CI kind/bug Categorizes a PR related to a bug
Projects
None yet
Milestone
18.0.0
Development

Successfully merging a pull request may close this issue.

The CodeQL analysis is broken due to the large content of the SARIF file
4 participants
@abstractj @stianst @andreaTP @DGuhr