-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Cleanup of CodeQL workflow #10308
Cleanup of CodeQL workflow #10308
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I'd like to have one more review from @stephengroat-dd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, slight ask to add the go modules cache if were making a change (sorry for the multiple user names, was performing some testing)
@@ -20,47 +20,41 @@ jobs: | |||
uses: actions/checkout@v2 | |||
with: | |||
fetch-depth: 0 | |||
path: go/src/github.com/DataDog/datadog-agent | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello ! I think this is a good idea and should absolutely be done (even for Python deps). But currently the go dependencies are downloaded between the codeql init and the codeql analyse steps which means that there is a possibility that they are analysed by CodeQL. I believe this warrants a separate PR to ensure that we are still getting the same amount of security findings.
Sadly this doesn't seem to fix the issue with the default branch. Happy to revert if you preferred the old way |
What does this PR do?
The CodeQL jobs are generating a lot of noise especially when you list all commits from the main branch (https://github.com/DataDog/datadog-agent/commits/main).
Here for example we only see failing commits because of the CodeQL results that are failing to understand that
main
is the default branch and tries to compare withmaster
which is not evaluated.Looking at the logs, I saw:
I believe codeql is confused by the complexity of the path used in the workflow and fails to fetch the current/base commit refs.
This PR tries to reduce this complexity by removing references to useless
GOPATH
orgo/src/github.com/...
. This is possible since the agent is go 1.11 module aware and thus doesn't require this setup.It is of course impossible to guarantee that this will do what it should, in the pure tradition of github actions.
Motivation
Additional Notes
Possible Drawbacks / Trade-offs
Describe how to test/QA your changes
Reviewer's Checklist
Triage
milestone is set.team/..
label has been applied, if known.changelog/no-changelog
label has been applied.qa/skip-qa
label is not applied.need-change/operator
andneed-change/helm
labels have been applied.