Cloud analysis of github stopped working

SonarCloud
29

I’m not sure we can say this with certainty.

Hold tight @mark1011 – I think we have to have some discussion internally to resolve where we stand here.

30

I’m not sure what you mean by “external Pull Requests”, or what could have changed for PRs against this repo to now be considered as “external”.

What do we have to do to get PRs against this repo being analysed again?

31

I’m also still running into issue. I also have a PR here. It was created by our own team members. Wouldn’t see how it would be considered external. My dependabot PRs seem to run Code Analysis, but any PRs created by members are not having code analysis run.

32

Here is a test PR is specifically submitted myself that shows no analysis being run.

33

We have been using SonarCloud with great success both with internal & external PRs up until a couple weeks ago. Before this, SonarCloud was analysing PRs that didn’t originate from our repo as well as ones that do.

For example:

while:

Weirdly enough, we can see that SonarCloud still does these checks on other repositories (Fix NuGet credential provider using wrong role by vchikoti1998 · Pull Request #4646 · aws/aws-toolkit-jetbrains · GitHub from earlier this week).

We’d like to understand the behaviour change – one of our main reasons for picking SonarCloud was it was an approved tool for SAST by OpenSSF but without the check on Pull Requests from external contributors, the use of the tool is some what limited for us.

34

Hey all.

It looks like we didn’t fully understand the scope and impact of a change that we made several weeks ago.

Our (internal) understanding was that the analysis of “external” PRs (PRs coming from the fork of a repository) was not accurate. Honestly, this is not a workflow we use at Sonar. I suppose now is as good a time as any to ask – were the analyses always accurate (correct determination of changed lines being the most important aspect)?

We’ll get back to you by Wednesday at the latest with an update. In the meantime, yes, these “external” PRs will not trigger Automatic Analysis. We’re sorry for the disruption and appreciate your patience.

35

Hi Colin, at least in our experience the code scanning of external PRs was as accurate as the one for internal ones, or at least we didn’t notice any major inconsistency over a year of usage and hundreds of PRs.

In terms of workflow, I’d say that for open source repositories that accept external contributions, the external PR analysis is even more important than the one for internal PRs, since it helps both maintainers and contributors have a clear and shared baseline.

Thank you for the quick answer, looking forward to hear from you on Wednesday, and ideally continue using SonarCloud in our organization.

36

Hey again!

We’re still investigating the behavior on our side. We’re turning back on the analysis of external PRs in a lower environment so we can run our own tests.

@dreamorosi Looking at your repos, here’s an example of an analyzed PR that doesn’t meet our expectations.

The associated pull request in Github shows four different python files changed, but 0 new lines are reported in SonarCloud. In SonarCloud, issues are only raised on lines that are detected as having changed.

This PR is showing 32k changed lines while Github is showing ~200 lines changed.

This inconsistency in results was noted several years ago, a ticket was made to turn off the analysis of External PRs since it was poorly supported… and now here we are.

So hopefully I’ll have an environment to investigate in soon :pray:

37

Hey all (specifically @dreamorosi)

After some consideration, we are going to:

  • revert the change we did in production to come back to the situation as it was before. We did, on closer inspection, find situations where it worked well (specifically if the external PR came from a non-main branch of the fork)
  • document the limitations we’re aware of when analyzing external PRs
  • implement some quick wins
  • look closer at what we need to do to be fully confident in supporting external PRs. this whole ordeal helped us kickstart the conversation again for the first time in a long time.

I don’t have an ETA on step 1 yet, so I’ll keep you posted.

38

Hey there! @dreamorosi @mark1011 @moorec-aws

Really happy to let you know that the analysis of external PRs has been turned back on, and we are now confident that the analysis results accurately reflect what has changed in the PR. Kudos to @Julien_HENRY for his work on this.

Please give it a try and let us know if you notice anything out of place.

1 Like
40

Thank you, this is great news!

We’ll be monitoring the integration over the coming weeks and see if it’s indeed behaving as it’s supposed to.

Thank you again for the work on this to you and the team!