Integrate GolangCI-Lint

[hanzei]

Mattermost is currently in the process of using golangci-lint as the default linter for all go projects. mattermost-community/mattermost-plugin-autolink#108 serves as an example of how the migration should look like:

1. Copy .golangci.yml from https://github.com/mattermost/mattermost-plugin-autolink/ into this repository.
2. Copy Makefile from https://github.com/mattermost/mattermost-plugin-autolink/ into this repository.
3. Copy build/ from https://github.com/mattermost/mattermost-plugin-autolink/ into this repository.
4. Run go mod tidy and make dist to ensure the build system works fine.
5. Update linters-settings.goimports.local-prefixes to the go module path i.e. to github.com/mattermost/mattermost-plugin-aws-SNS.
6. Run make golangci-lint and fix all issues that arise.

Fixing an linter issue is sometimes trivial but can also be quite complicated. There might be cases, where ignoring an issue using issues.exclude-rules is needed. If you don't know how to resolve an error, please reach out to @hanzei on the Mattermost Community Server or ask the question here.

[arshchimni]

I would like to pick this up and give it a go 😄

[arshchimni]

@hanzei
After going through the above process. Most of the issues generated by the linter have been solved. Following are the remaining issues which I want to discuss (wether to ignore them or to fix them)

golangci-lint run ./...
build/manifest/main.go:106:13: G306: Expect WriteFile permissions to be 0600 or less (gosec)
                if err := ioutil.WriteFile(
                        "server/manifest.go",
                        []byte(fmt.Sprintf(pluginIDGoFileTemplate, manifest.Id, manifest.Version)),
                        0644,
                ); err != nil {
build/manifest/main.go:116:13: G306: Expect WriteFile permissions to be 0600 or less (gosec)
                if err := ioutil.WriteFile(
                        "webapp/src/manifest.js",
                        []byte(fmt.Sprintf(pluginIDJSFileTemplate, manifest.Id, manifest.Version)),
                        0644,
                ); err != nil {

Since the permissions are hardcoded, should those be changed?

server/webex/client.go:133:28: response body must be closed (bodyclose)
        rp, err := c.httpClient.Do(rq)
                                  ^

The body is closed in this function

server/store.go:4:2: G501: Blacklisted import `crypto/md5`: weak cryptographic primitive (gosec)
        "crypto/md5"
        ^

server/store.go:29:7: G401: Use of weak cryptographic primitive (gosec)
        h := md5.New()
             ^

These from my understanding can be ignored as we are using the packages for user-hash generation and not encryption(i might be wrong)

server/http.go:56:37: `w` can be `io.Writer` (interfacer)
func (p *Plugin) handleStartMeeting(w http.ResponseWriter, r *http.Request) (int, error) {

On this error I am unsure wether to change the signature or not

[hanzei]

Thanks for jumping on this issue @arshchimni 👍

1.Yes, please change the permission to 0600 👍
2. That is indeed a false positive from bodyclose. The easiest fix might be to disable bodyclose entirely for now by commenting it out.
3. Using md5 is fine here, but there should be a comment explaining why. Please add something like:
"crypto/md5" //nolint:gosec // md5 is used for user-hash generation and not encryption.
4. Using io.Writer is fine here 👍