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

Create codeql.yml #1880

Draft
wants to merge 9 commits into
base: 0.10
Choose a base branch
from
Draft

Create codeql.yml #1880

wants to merge 9 commits into from

Conversation

peternewman
Copy link
Member

No description provided.

@peternewman peternewman added this to the 0.10.10 milestone Jul 16, 2023
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@DaAwesomeP
Copy link
Member

DaAwesomeP commented Jul 16, 2023

For easier maintainability (since it is just a normal build with an extra step or two at the end) should it go in lint.yaml? We already have a pipline for building once and then running the last step in separate actions. That way we aren't maintaining build steps in more places.

I think this should be as simple as:

codeql:
  name: CodeQL
  if: github.ref == 'refs/heads/master' || github.ref == 'refs/heads/0.10'
  runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }}
  timeout-minutes: ${{ (matrix.language == 'swift' && 120) || 360 }}
  permissions:
    actions: read
    contents: read
    security-events: write
  strategy:
    fail-fast: false
    matrix:
      language: [ 'cpp', 'java', 'javascript', 'python' ]
  needs: build
  steps:
    - name: Download built source tree archive
      uses: actions/download-artifact@v3
      with:
        name: ola-debian-stable-built-source-tree
        path: .
    - name: SHA256 artifact archive
      run: sha256sum ola-debian-stable-built-source-tree.tar.gz
    - name: Unarchive artifacts and delete archive
      shell: bash
      run: |
        tar -xvzf ola-debian-stable-built-source-tree.tar.gz .
        rm ola-debian-stable-built-source-tree.tar.gz
    - name: Display structure of extracted files
      if: env.ACTIONS_STEP_DEBUG == 'true'
      run: ls -alR
    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v2
      with:
        category: "/language:${{matrix.language}}"

Also related to #1862, but the race condition there is caused by the test suite, so running lint on a schedule should be fine.

@DaAwesomeP
Copy link
Member

DaAwesomeP commented Jul 16, 2023

Ah, I missed the github/codeql-action/init@v2 step, so maybe this isn't possible. Or maybe it can be initialized no matter what in the initial lint build step.

@peternewman
Copy link
Member Author

For easier maintainability (since it is just a normal build with an extra step or two at the end) should it go in lint.yaml? We already have a pipline for building once and then running the last step in separate actions. That way we aren't maintaining build steps in more places.

I think this should be as simple as:

Yeah I was just trying hacking something in when I came across it, initially just by taking their template, then trying to make minimal fixes to it...

Merging it into an existing workflow would probably be my intention if it worked. Currently Javascript works fine, Java/Maven throws an error claiming it's too old (see #1881 ) and C++ compiles fine by says it didn't find any compiled code.

Feel free to have a go yourself with some workflows if you want, they're fairly easy as you can see...

Ah, I missed the github/codeql-action/init@v2 step, so maybe this isn't possible. Or maybe it can be initialized no matter what in the initial lint build step.

What do you think would be the issue with initing beforehand?

From a bit of reading of their docs it just looks at all the compilation that happens between init and analyse, so if they're split into a pre-build/cached compilation and the analysis then indeed that probably won't work. 😢

@peternewman
Copy link
Member Author

Well it works fine with no containers involved, despite them saying they shouldn't be an issue:
#1882

It found a small handful of problems but nothing too exciting IMHO. Most should be fairly easily fixed though too, which is good.

@DaAwesomeP
Copy link
Member

DaAwesomeP commented Jul 19, 2023

From a bit of reading of their docs it just looks at all the compilation that happens between init and analyse, so if they're split into a pre-build/cached compilation and the analysis then indeed that probably won't work. 😢

Yeah, exactly this is the issue. It doesn't seem possible to break up the jobs because of this. Maybe we can open an issue on their end about saving state (i.e. init CodeQL > run job > save CodeQL state > new job > restore CodeQL state > run job > finish CodeQL). The other issue is that it seems you can only init one language at a time which does not work for a build-once scenario.

On my list eventually is making the general build process into a shell script with options/steps or a GitHub Actions template, but that is a long way off for now unfortunately.

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 this pull request may close these issues.

2 participants