docker / cli

The Docker CLI
Apache License 2.0
4.77k stars 1.89k forks source link

Fix errors encountered by CodeQL #5049

Closed thaJeztah closed 2 months ago

thaJeztah commented 2 months ago

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.

codecov-commenter commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files ```diff @@ 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 commented 2 months ago

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 commented 2 months ago

Looks like there's still some warnings / errors; one of them looks to be related to https://github.com/docker/cli/pull/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 commented 2 months ago

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 commented 2 months ago

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;

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