ci: enable stylecheck linter, fix ST1005 warnings

[kolyshkin]

1. Enable stylecheck linter (with some checks disabled).

2. Enabled checks disabled above in golangci-extra.yml (used for new code).

3. Fix "ST1005: error strings should not be capitalized (stylecheck)" warnings.

[AkihiroSuda]

Maybe too strict?

[kolyshkin]

Maybe too strict?

Maybe. We wrap some errors, and those need to be non-capitalized, and we don't wrap some others, and those are actually better if capitalized.

@thaJeztah wdyt?

[kolyshkin]

Problem is, we don't know which errors are wrapped and which are not.

• anything that is returned from libcontainer may be wrapped (so needs to be lowercased)
• anything outside of libcontainer may or may not be wrapped

@AkihiroSuda what if we only apply the linter to libcontainer?

[kolyshkin]

Problem is, we don't know which errors are wrapped and which are not.

• anything that is returned from libcontainer may be wrapped (so needs to be lowercased)
• anything outside of libcontainer may or may not be wrapped

@AkihiroSuda what if we only apply the linter to libcontainer?

Nope; in fact, kubernetes, cri-o, containerd and other users also wrap errors printed by runc, so everything should be lowercased, so this PR makes total sense.

[AkihiroSuda]

Sorry, not sure how wrapping is relevant to capitalization

[kolyshkin]

Sorry, not sure how wrapping is relevant to capitalization

The idea here is something like "Error: can't start container: can't run init: permission denied" looks better than "Error: Can't start container: Can't run init: permission denied".

Here' a quote from Google Go style guide (https://google.github.io/styleguide/go/decisions#error-strings):

Error strings should not be capitalized (unless beginning with an exported name, a proper noun or an acronym) and should not end with punctuation. This is because error strings usually appear within other context before being printed to the user.

// Bad:
err := fmt.Errorf("Something bad happened.")
// Good:
err := fmt.Errorf("something bad happened")

On the other hand, the style for the full displayed message (logging, test failure, API response, or other UI) depends, but should typically be capitalized.

// Good:
log.Infof("Operation aborted: %v", err)
log.Errorf("Operation aborted: %v", err)
t.Errorf("Op(%q) failed unexpectedly; err=%v", args, err)

Another quote, from https://gist.github.com/adamveld12/c0d9f0d5f0e1fba1e551#error-strings:

Error strings should not be capitalized (unless beginning with proper nouns or acronyms) or end with punctuation, since they are usually printed following other context. That is, use fmt.Errorf("something bad") not fmt.Errorf("Something bad"), so that log.Print("Reading %s: %v", filename, err) formats without a spurious capital letter mid-message. This does not apply to logging, which is implicitly line-oriented and not combined inside other messages.

[thaJeztah]

overall I agree with these changes, but slightly concerned about changing the error-messages, as it's possible there's some string-matching in other codebases that handle the errors returned from runc (binary).

[thaJeztah]

For others reviewing (I was wondering if this should be CWD), but the property is lowercase in JSON, so probably makes sense to match that

runc/vendor/github.com/opencontainers/runtime-spec/specs-go/config.go

Lines 78 to 80 in e42446f

	// Cwd is the current working directory for the process and must be
	// relative to the container's root.
	Cwd string `json:"cwd"`

[kolyshkin]

Rebased

[cyphar]

but slightly concerned about changing the error-messages, as it's possible there's some string-matching in other codebases that handle the errors returned from runc (binary).

Maybe we should randomly change our error messages to ensure people don't do this? 😉

[kolyshkin]

Guess I'll close this one due to lack of interest.