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

Enable configuration to fail CI if new alerts are introduced #598

Closed
johnthagen opened this issue Jun 25, 2021 · 6 comments · Fixed by #599
Closed

Enable configuration to fail CI if new alerts are introduced #598

johnthagen opened this issue Jun 25, 2021 · 6 comments · Fixed by #599

Comments

@johnthagen
Copy link

We are currently trying to migrate from LGTM to codeql-action due to limitations in LGTM's infrastructure.

With LGTM, we could set it return a failing check any time new analysis alerts were introduced in an PR. This would allow us to prevent merging a PR if it introduced new alerts.

We set up the default GitHub Actions for codeql-action and it ran and found 6 alerts in our test suite, but the build did not put up a failing check that would have prevented merging.

This is our first PR that adds codeql-action, so perhaps this needs to be merged first so that future PRs have something to be compared against? We are hesitant to merge without knowing there is a way to configure the action to fail when new alerts are introduced.

@johnthagen johnthagen changed the title Enable configuration to fail CI if new errors are introduced Enable configuration to fail CI if new alerts are introduced Jun 25, 2021
@aeisenberg
Copy link
Contributor

Hi there, thanks for reporting an issue. You are correct that Code Scanning is not failing because it has nothing to compare the new alerts against. The message:

Warning: Code scanning cannot determine the alerts introduced or fixed by this pull request, because 1 analysis was not found.

means that it found some alerts in the PR, but it did not find any alerts in the base branch (master) so it doesn't know if it should fail on these new alerts.

See Reasons for the "missing analysis" message

You will need to run code scanning on the master branch before code scanning can properly calibrate the alerts. And there is a setting in Settings -> Security and Analysis -> Code Scanning to make sure there is a failing commit status if any alert is found.

@johnthagen
Copy link
Author

@aeisenberg Thanks for the quick feedback! Might I suggest a sentence or two be added to the README about this? I think it would help new users onboard faster.

@adityasharad
Copy link
Contributor

You may also find https://docs.github.com/en/code-security/secure-coding/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#defining-the-alert-severities-causing-pull-request-check-failure useful to determine the threshold for making the check fail. The default is that error-severity alerts (or a failing build) will block the PR, but you can adjust this in settings to a lower severity if you wish.

@johnthagen
Copy link
Author

@aeisenberg @adityasharad After migrating to codeql-actions it seems there is no way to disable only one of the languages that LGTM detects (we only want to use Javascript with it now). Is that something the codeql team can help with?

The project is: https://lgtm.com/projects/g/sillsdev/TheCombine/?mode=list

We'd like to disable C# but keep Javascript.

The problem we're hitting is that the codeql-action is running fine, but LGTM keeps reporting failed builds to our PRs, which GitHub then marks with a red X even though we don't require them to pass.

@adityasharad
Copy link
Contributor

Certainly. We have disabled C# on that project on LGTM. Please don't hesitate to let us know if we can be of more help as you migrate to code scanning on GitHub.

@johnthagen
Copy link
Author

@adityasharad Just wanted to report everything is working great now. Thank you!

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 a pull request may close this issue.

3 participants