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

Fix errors encountered by CodeQL #5049

Merged
merged 2 commits into from
May 1, 2024
Merged

Conversation

thaJeztah
Copy link
Member

gha: CodeQL: move go.mod/go.sum symlink earlier to help caching

actions/setup-go was trying to use caching, and produced a warning
because it expects a go.mod / go.sum;

Run actions/setup-go@v5
  with:
    go-version: 1.21
    check-latest: false
    token: ***
    cache: true
  env:
    DISABLE_WARN_OUTSIDE_CONTAINER: 1
Setup go version spec 1.21
Found in cache @ /opt/hostedtoolcache/go/1.21.9/x64
Added go to the path
Successfully set up Go version 1.21
/opt/hostedtoolcache/go/1.21.9/x64/bin/go env GOMODCACHE
/opt/hostedtoolcache/go/1.21.9/x64/bin/go env GOCACHE
/home/runner/go/pkg/mod
/home/runner/.cache/go-build
Warning: Restore cache failed: Dependencies file is not found in /home/runner/work/cli/cli. Supported file pattern: go.sum
go version go1.21.9 linux/amd64

While our regular builds would use a containerised flow, CodeQL's autobuild
does not, and also doesn't seem to use our vendor directory (?) so for this
one it's probably fine to let it use some caching.

go.mod: use SemVer format for go version to assist CodeQL AutoBuild

CodeQL AutoBuild started to produce errors if the go.mod does not use
SemVer for the go version; https://github.com/github/codeql/blob/3a2b0a2feba58c05706c88b5589cacf6094f7f9d/go/extractor/diagnostics/diagnostics.go#L512-L525

Let's give it one.

actions/setup-go was trying to use caching, and produced a warning
because it expects a `go.mod` / `go.sum`;

    Run actions/setup-go@v5
      with:
        go-version: 1.21
        check-latest: false
        token: ***
        cache: true
      env:
        DISABLE_WARN_OUTSIDE_CONTAINER: 1
    Setup go version spec 1.21
    Found in cache @ /opt/hostedtoolcache/go/1.21.9/x64
    Added go to the path
    Successfully set up Go version 1.21
    /opt/hostedtoolcache/go/1.21.9/x64/bin/go env GOMODCACHE
    /opt/hostedtoolcache/go/1.21.9/x64/bin/go env GOCACHE
    /home/runner/go/pkg/mod
    /home/runner/.cache/go-build
    Warning: Restore cache failed: Dependencies file is not found in /home/runner/work/cli/cli. Supported file pattern: go.sum
    go version go1.21.9 linux/amd64

While our regular builds would use a containerised flow, CodeQL's autobuild
does not, and also doesn't seem to use our vendor directory (?) so for this
one it's probably fine to let it use some caching.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
CodeQL AutoBuild started to produce errors if the `go.mod` does not use
SemVer for the go version; https://github.com/github/codeql/blob/3a2b0a2feba58c05706c88b5589cacf6094f7f9d/go/extractor/diagnostics/diagnostics.go#L512-L525

Let's give it one.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.07%. Comparing base (e81b835) to head (e3216ca).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5049   +/-   ##
=======================================
  Coverage   61.07%   61.07%           
=======================================
  Files         295      295           
  Lines       20667    20667           
=======================================
  Hits        12623    12623           
  Misses       7146     7146           
  Partials      898      898           

@thaJeztah
Copy link
Member Author

Thx! FWIW; not 100% sure this will fix the issues, but it started to show errors / warnings, and looking at the logs, these were the ones that I found. It didn't fail though, so 🤷‍♂️

Screenshot 2024-04-30 at 20 06 32 Screenshot 2024-04-30 at 20 06 48

@thaJeztah thaJeztah merged commit 2c8a5f7 into docker:master May 1, 2024
97 checks passed
@thaJeztah thaJeztah deleted the codeql_cache branch May 1, 2024 07:35
@thaJeztah thaJeztah added the kind/bugfix PR's that fix bugs label May 1, 2024
@thaJeztah thaJeztah added this to the 27.0.0 milestone May 1, 2024
@thaJeztah thaJeztah self-assigned this May 1, 2024
@thaJeztah
Copy link
Member Author

Looks like there's still some warnings / errors; one of them looks to be related to #4624, which renamed the workflow file; I can try deleting the workflow from that page;

Screenshot 2024-05-06 at 11 10 41

The other one is that it's still complaining about go files that it didn't analyse. Wondering if that's mac <--> windows, or something else;

Screenshot 2024-05-06 at 11 02 46

@thaJeztah
Copy link
Member Author

Hmm... right, so it shows more now on that page;

Screenshot 2024-05-06 at 11 14 56 Screenshot 2024-05-06 at 11 15 49

@thaJeztah
Copy link
Member Author

thaJeztah commented May 6, 2024

And it provides a CSV to show what files were analysed and what ones weren't...
code-scanning-files-extracted.csv

And from the looks of it;

  • it skips _test.go files
  • it skips windows files

So not sure if (at least) _test.go files could be enabled for it (but it's funny that it's effectively imposing a penalty for having tests 😂), and don't know if it supports windows;

Screenshot 2024-05-06 at 11 14 23 Screenshot 2024-05-06 at 11 19 41 Screenshot 2024-05-06 at 11 19 56

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants