CI: Add CLA check, Linting and security scanning

[rebornplusplus]

• CLA Check ensures that contributors sign the Canonical Contributor Licence Agreement (https://ubuntu.com/legal/contributors).
  • Uses canonical/has-signed-canonical-cla.
• Various Linting Support has been added, with the help of golangci-lint. The various linters' specification is mentioned here: https://golangci-lint.run/usage/linters/
  • Check bugs and unused code using errcheck, unused and staticcheck.
  • Check formatting with go fmt ./....
• Security: scan for known vulnerabilities using Trivy.

[niemeyer]

Thanks Rafid, here is a first pass.

[niemeyer]

Thanks Rafid, here is a first pass.

[rebornplusplus]

Please note that the "Lint" tests are failing because some of the files we currently have, seem not to be appropriately formatted.
go fmt ./... produces the diff here: link to GH workflow run log.

Additionally, I ran errcheck, staticcheck and unused on my workstation in the repo(main) and found the following warnings:

Toggle to see log

$ golangci-lint run --disable-all --enable errcheck,staticcheck,unused
internal/strdist/log.go:47:22: Error return value of `globalLogger.Output` is not checked (errcheck)
		globalLogger.Output(2, fmt.Sprintf(format, args...))
		                   ^
internal/strdist/log.go:58:22: Error return value of `globalLogger.Output` is not checked (errcheck)
		globalLogger.Output(2, fmt.Sprintf(format, args...))
		                   ^
internal/strdist/log.go:43:6: func `logf` is unused (unused)
func logf(format string, args ...interface{}) {
     ^
internal/strdist/strdist.go:24:2: field `private` is unused (unused)
	private struct{}
	^
internal/control/control_test.go:91:22: Error return value of `control.ParseString` is not checked (errcheck)
		control.ParseString("Package", content)
		                   ^
internal/fsutil/log.go:40:22: Error return value of `globalLogger.Output` is not checked (errcheck)
		globalLogger.Output(2, fmt.Sprintf(format, args...))
		                   ^
internal/setup/fetch.go:47:25: Error return value of `lockFile.Unlock` is not checked (errcheck)
			defer lockFile.Unlock()
			                     ^
internal/archive/archive_test.go:117:16: Error return value of `release.Render` is not checked (errcheck)
	release.Render(base.Path, s.responses)
	              ^
internal/archive/archive_test.go:232:15: Error return value of `release.Walk` is not checked (errcheck)
		release.Walk(func(item testarchive.Item) error {
		            ^
internal/archive/archive_test.go:239:17: Error return value of `release.Render` is not checked (errcheck)
		release.Render("/ubuntu", s.responses)
		              ^
cmd/chisel/cmd_help.go:105:9: Error return value of `io.Copy` is not checked (errcheck)
	io.Copy(Stdout, strings.NewReader(str))
	       ^
cmd/chisel/main.go:167:16: Error return value is not checked (errcheck)
		printVersions()
		             ^
cmd/chisel/main.go:181:9: Error return value is not checked (errcheck)
	addHelp(parser)
	       ^
internal/fsutil/log.go:36:6: func `logf` is unused (unused)
func logf(format string, args ...interface{}) {
     ^
internal/deb/log.go:17:5: var `globalDebug` is unused (unused)
var globalDebug bool
    ^
internal/deb/log.go:47:6: func `debugf` is unused (unused)
func debugf(format string, args ...interface{}) {
     ^
internal/deb/version.go:86:6: func `matchEpoch` is unused (unused)
func matchEpoch(a string) bool {
     ^
internal/deb/version.go:99:6: func `atMostOneDash` is unused (unused)
func atMostOneDash(a string) bool {
     ^
internal/scripts/log.go:16:5: var `globalLogger` is unused (unused)
var globalLogger log_Logger
    ^
internal/scripts/log.go:17:5: var `globalDebug` is unused (unused)
var globalDebug bool
    ^
internal/scripts/log.go:36:6: func `logf` is unused (unused)
func logf(format string, args ...interface{}) {
     ^
internal/scripts/log.go:47:6: func `debugf` is unused (unused)
func debugf(format string, args ...interface{}) {
     ^
internal/jsonwall/log.go:16:5: var `globalLogger` is unused (unused)
var globalLogger log_Logger
    ^
internal/jsonwall/log.go:17:5: var `globalDebug` is unused (unused)
var globalDebug bool
    ^
internal/jsonwall/log.go:47:6: func `debugf` is unused (unused)
func debugf(format string, args ...interface{}) {
     ^
internal/setup/setup_test.go:17:2: field `slices` is unused (unused)
	slices    map[string]*setup.Slice
	^
internal/slicer/log.go:16:5: var `globalLogger` is unused (unused)
var globalLogger log_Logger
    ^
cmd/chisel/main.go:40:7: const `defaultChiselDir` is unused (unused)
const defaultChiselDir = "/var/lib/chisel/default"
      ^
cmd/chisel/main.go:91:6: func `addDebugCommand` is unused (unused)
func addDebugCommand(name, shortHelp, longHelp string, builder func() flags.Commander, optDescs map[string]string, argDescs []argDesc) *cmdInfo {
     ^
cmd/chisel/main_test.go:73:6: func `fakeArgs` is unused (unused)
func fakeArgs(args ...string) (restore func()) {
     ^
internal/testutil/pkgdata.go:229:2: SA4006: this value of `err` is never used (staticcheck)
	writer, err := zstd.NewWriter(&buf)
	^
internal/testutil/pkgdata_test.go:346:2: SA4006: this value of `err` is never used (staticcheck)
	zstdReader, err := zstd.NewReader(&tarZstdBuf)
	^
cmd/chisel/main.go:15:2: SA1019: "golang.org/x/crypto/ssh/terminal" is deprecated: this package moved to golang.org/x/term. (staticcheck)
	"golang.org/x/crypto/ssh/terminal"
	^
cmd/chisel/main_test.go:8:2: SA1019: "golang.org/x/crypto/ssh/terminal" is deprecated: this package moved to golang.org/x/term. (staticcheck)
	"golang.org/x/crypto/ssh/terminal"
	^

We should probably refactor these files for the tests to pass.

[jnsgruk]

Given that the job fails due to formatting - can you add another commit that actually fixes those changes? That way we should have one commit introducing everything you have so far, and another to address the linting differences which means we don't need to merge a broken PR :)

[rebornplusplus]

This PR being very old, I have rebased the commits onto the current main and later squashed the commits into one.

Given that the job fails due to formatting - can you add another commit that actually fixes those changes? That way we should have one commit introducing everything you have so far, and another to address the linting differences which means we don't need to merge a broken PR :)

I have fixed the formatting issues in 020f62b. However, the lint workflow will still fail due to the linters we are running (errcheck, unused, staticcheck). Fixing this will need some (minor I am hoping) refactoring. I will add another commit addressing those issues.

[cjdcordeiro]

Thanks @rebornplusplus ;) if you feel that refactoring will require significant changes, feel free to leave it for a future pulse so that we can plan it properly

[rebornplusplus]

if you feel that refactoring will require significant changes, feel free to leave it for a future pulse so that we can plan it properly

A'ight. I will leave it for next pulse then.

[cjdcordeiro]

Thanks for the update. This should be ready to review now. The failure on the spread tests is related to Kinetic reaching EOL (which is being addressed elsewhere)

[rebornplusplus]

Indeed, it is ready for review. #97 is merged already, tests are passing. Please take a look.

[jnsgruk]

LGTM. Changes requested by Gustavo seem to have been addressed, and the other code changes are all stylistic or of very low consequence.