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

Go: Basic Go 1.21 support #13867

Merged
merged 17 commits into from
Aug 18, 2023
Merged

Go: Basic Go 1.21 support #13867

merged 17 commits into from
Aug 18, 2023

Conversation

mbg
Copy link
Member

@mbg mbg commented Aug 2, 2023

Go 1.21 was released recently. This PR updates our CI to use Go 1.21 and makes the Go extractor aware of Go 1.21. It also adds support for the new built-in functions: min, max, and clear.

@mbg mbg self-assigned this Aug 2, 2023
@github-actions github-actions bot added the Go label Aug 2, 2023
@mbg mbg force-pushed the mbg/go/1.21-support branch from 59d52aa to f7a91ca Compare August 2, 2023 12:00
@mbg mbg force-pushed the mbg/go/1.21-support branch from f7a91ca to 7de85cb Compare August 8, 2023 23:55
@marcusthelin
Copy link

Status on this? :)

@mbg
Copy link
Member Author

mbg commented Aug 9, 2023

@marcusthelin our existing CodeQL release should already be compatible with Go 1.21 and you should be able to build Go 1.21 projects with it. This PR is mostly about modelling standard library change, updating our CI, etc.

Is there anything in particular that doesn't work for you with Go 1.21?

@mbg mbg force-pushed the mbg/go/1.21-support branch from 7de85cb to 4fb7d29 Compare August 9, 2023 19:02
@mbg mbg force-pushed the mbg/go/1.21-support branch from 4fb7d29 to a623733 Compare August 11, 2023 10:10
@mbg mbg force-pushed the mbg/go/1.21-support branch from 84f4697 to bcb96da Compare August 11, 2023 10:47
@mbg mbg force-pushed the mbg/go/1.21-support branch from bcb96da to 513da82 Compare August 11, 2023 10:51
@mbg mbg changed the title Go: Support Go 1.21 Go: Basic Go 1.21 support Aug 11, 2023
@mbg mbg marked this pull request as ready for review August 11, 2023 11:30
@mbg mbg requested a review from a team as a code owner August 11, 2023 11:30
@mbg mbg requested a review from owen-mc August 11, 2023 11:30
.github/workflows/go-tests-other-os.yml Outdated Show resolved Hide resolved
go/ql/lib/semmle/go/Scopes.qll Show resolved Hide resolved
@marcusthelin
Copy link

@marcusthelin our existing CodeQL release should already be compatible with Go 1.21 and you should be able to build Go 1.21 projects with it. This PR is mostly about modelling standard library change, updating our CI, etc.

Is there anything in particular that doesn't work for you with Go 1.21?

@mbg we keep getting the below error. Our go.mod file has version 1.21.

image

Our workflow:

name: CodeQL

on:
  workflow_call:
    inputs:
      working-directory:
        type: string
        required: true
      languages:
        type: string
        default: go
    secrets:
      githubToken:
        required: true

jobs:
  analyze:
    name: Analyze
    runs-on: ubuntu-latest
    timeout-minutes: 360
    permissions:
      actions: read
      contents: read
      security-events: write

    steps:
    - name: Checkout repository
      uses: actions/checkout@v3

    # Initializes the CodeQL tools for scanning.
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v2
      with:
        languages: ${{ inputs.languages }}
        # If you wish to specify custom queries, you can do so here or in a config file.
        # By default, queries listed here will override any specified in a config file.
        # Prefix the list here with "+" to use these queries and those in the config file.

        # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
        # queries: security-extended,security-and-quality

    - name: Autobuild
      uses: github/codeql-action/autobuild@v2
      with:
        working-directory: ${{ inputs.working-directory }}

    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v2
      with:
        category: "/${{ inputs.working-directory }}"

@mbg
Copy link
Member Author

mbg commented Aug 15, 2023

Ah thanks for sharing that extra information @marcusthelin. The problem you run into is that the GitHub Actions runner image has Go 1.20 installed by default and so CodeQL gives you an error to tell you that there is a mismatch between the Go version in the environment and the one you expect for your project.

To remedy this, add a step to your workflow that uses the setup-go action like so:

Updated workflow
name: CodeQL

on:
  workflow_call:
    inputs:
      working-directory:
        type: string
        required: true
      languages:
        type: string
        default: go
    secrets:
      githubToken:
        required: true

jobs:
  analyze:
    name: Analyze
    runs-on: ubuntu-latest
    timeout-minutes: 360
    permissions:
      actions: read
      contents: read
      security-events: write

    steps:
    - name: Checkout repository
      uses: actions/checkout@v3

    - name: Install Go
      uses: actions/setup-go@v4
      with:
        go-version-file: go.mod # adjust this path if needed

    # Initializes the CodeQL tools for scanning.
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v2
      with:
        languages: ${{ inputs.languages }}
        # If you wish to specify custom queries, you can do so here or in a config file.
        # By default, queries listed here will override any specified in a config file.
        # Prefix the list here with "+" to use these queries and those in the config file.

        # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
        # queries: security-extended,security-and-quality

    - name: Autobuild
      uses: github/codeql-action/autobuild@v2
      with:
        working-directory: ${{ inputs.working-directory }}

    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v2
      with:
        category: "/${{ inputs.working-directory }}"

@marcusthelin
Copy link

Ah thanks for sharing that extra information @marcusthelin. The problem you run into is that the GitHub Actions runner image has Go 1.20 installed by default and so CodeQL gives you an error to tell you that there is a mismatch between the Go version in the environment and the one you expect for your project.

To remedy this, add a step to your workflow that uses the setup-go action like so:

Updated workflow

name: CodeQL

on:
  workflow_call:
    inputs:
      working-directory:
        type: string
        required: true
      languages:
        type: string
        default: go
    secrets:
      githubToken:
        required: true

jobs:
  analyze:
    name: Analyze
    runs-on: ubuntu-latest
    timeout-minutes: 360
    permissions:
      actions: read
      contents: read
      security-events: write

    steps:
    - name: Checkout repository
      uses: actions/checkout@v3

    - name: Install Go
      uses: actions/setup-go@v4
      with:
        go-version-file: go.mod # adjust this path if needed

    # Initializes the CodeQL tools for scanning.
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v2
      with:
        languages: ${{ inputs.languages }}
        # If you wish to specify custom queries, you can do so here or in a config file.
        # By default, queries listed here will override any specified in a config file.
        # Prefix the list here with "+" to use these queries and those in the config file.

        # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
        # queries: security-extended,security-and-quality

    - name: Autobuild
      uses: github/codeql-action/autobuild@v2
      with:
        working-directory: ${{ inputs.working-directory }}

    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v2
      with:
        category: "/${{ inputs.working-directory }}"

@mbg I've tried this now and I still see the same error. Is the CodeQL action overriding the Go version?

@mbg
Copy link
Member Author

mbg commented Aug 15, 2023

@marcusthelin I have just put together a small test repo to validate this for myself with a slightly simplified version of your workflow. It seems to work fine for me: https://github.com/mbg/go-test/actions/runs/5867141929/job/15907302196

Could you verify that the output of the "Install Go" step in your workflow is able to find the go.mod file and that it installs Go 1.21 as expected / seen in my workflow run: https://github.com/mbg/go-test/actions/runs/5867141929/job/15907302196#step:3:8

If not, you may need to adjust the argument for go-version-file. Perhaps something like this would work better in your workflow since you seem to have a workflow input for the working directory which is used to set the working directory for other steps:

      with:
        go-version-file: ${{ inputs.working-directory }}/go.mod

@marcusthelin
Copy link

marcusthelin commented Aug 15, 2023

@mbg I see the log in autobuilder:

Autobuilder was built with go1.20.5, environment has go1.21.0

I also see errors:

  Error: 2023/08/15 13:49:18   /opt/hostedtoolcache/go/1.21.0/x64/src/slices/sort.go:67:7: undefined: min
  Error: 2023/08/15 13:49:18   /opt/hostedtoolcache/go/1.21.0/x64/src/slices/sort.go:97:7: undefined: max

which indicates that it does not in fact run 1.21, since those are new functions in 1.21. What could be the cause?

@mbg
Copy link
Member Author

mbg commented Aug 15, 2023

I also see errors:

  Error: 2023/08/15 13:49:18   /opt/hostedtoolcache/go/1.21.0/x64/src/slices/sort.go:67:7: undefined: min
  Error: 2023/08/15 13:49:18   /opt/hostedtoolcache/go/1.21.0/x64/src/slices/sort.go:97:7: undefined: max

which indicates that it does not in fact run 1.21, since those are new functions in 1.21. What could be the cause?

Thank you for posting these error messages. In short, there are multiple versions of Go at play: the version of the Go toolchain and standard library that is installed on the runner (which is indeed 1.21 now as reported), and the version of the Go language libraries that the CodeQL Go extractor is built against / the version of the Go compiler that the extractor is built with (which is still for 1.20 in the latest CodeQL release).

This is largely expected, however, until we support the new built-ins. Once both #13923 and this PR are merged and a new CodeQL CLI release is out with those changes, those errors should go away.

In the meantime, I would expect your CodeQL analysis to still be working successfully as before (the errors in the log shouldn't stop it from proceeding). The only limitation here is that data flow / taint flow through the new built-ins isn't modelled yet until the updated models are released.

@marcusthelin
Copy link

@mbg Ah, okay! So just because I see an error under the "Security" tab (according to the screenshot I've posted previously), the scans should still work.

Thank you for helping me out on this 🙏🏼

@mbg
Copy link
Member Author

mbg commented Aug 15, 2023

Yes, sorry about that error. I can see that we are reporting an error-level diagnostic in this particular scenario that's probably not helpful. I will raise an internal issue about this to see if we can improve the reporting here. Go 1.21 is the first new Go release since we introduced the tool status page, so this is the first time we have run into the particular scenario where the system version of Go is newer than the one our tooling is built with! Thank you for your feedback here!

- `clear` isn't pure because it modifies a data structure in place
- `clear` may not be used correctly, but this is determined statically
@mbg mbg requested a review from smowton August 15, 2023 19:24
@mbg mbg added the no-change-note-required This PR does not need a change note label Aug 15, 2023
For `os.dirEntry` and `os.unixDirent` which are only available
on unix and Windows respectively.
@mbg mbg force-pushed the mbg/go/1.21-support branch from 69f259a to c981fd7 Compare August 15, 2023 19:32
@mbg mbg mentioned this pull request Aug 17, 2023
@mbg mbg force-pushed the mbg/go/1.21-support branch from be6c90e to 109b96f Compare August 17, 2023 16:50
@mbg mbg requested a review from smowton August 17, 2023 17:39
Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@mbg mbg dismissed owen-mc’s stale review August 18, 2023 10:51

Approved by other member of the team

@mbg mbg merged commit a1c9dee into main Aug 18, 2023
@mbg mbg deleted the mbg/go/1.21-support branch August 18, 2023 13:37
@mbg mbg mentioned this pull request Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Go no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants