feat: new custom linters system

[ldez]

We know the complaints about custom linter with the Go plugin system.

Today if you want to use a plugin, you have to:

• build golangci-lint from sources with CGO_ENABLED=1
• build the plugins with the same stack (Go, dependencies, etc.) as golangci-lint
• copy your plugins somewhere

The problems are:

• the requirement to have golangci-lint and the plugins build with the same stack.
• the requirement to store and rebuild the plugins when you need to share them (with other devs or CI).
• limited OS support

And I will cite the Go documentation:

Together, these restrictions mean that, in practice, the application and its plugins must all be built together by a single person or component of a system.
https://pkg.go.dev/plugin#hdr-Warnings

For me, it's clear that the Go plugin system is not adapted to extend golangci-lint.

In that case, it may be simpler for that person or component to generate Go source files that blank-import the desired set of plugins and then compile a static executable in the usual way.
https://pkg.go.dev/plugin#hdr-Warnings

I propose to use a different approach that follows the Go recommendation, the principle is similar to SQL drivers in Go:

• the plugins are simple Go dependencies that follow a contract and are imported with a blank-import.
• a plugin uses init to register itself.
• inside golangci-lint we read the register to load the plugins.

Description of the previous POC

This PR is a POC to discuss the custom linters.

The idea comes during the rewrite of the commands and the Manager.

We know the complaints about custom linter with the Go plugin system.

Today if you want to use a plugin, you have to:

• build golangci-lint from sources with CGO_ENABLED=1
• build the plugins with the same stack (Go, dependencies, etc.) as golangci-lint
• copy your plugins somewhere

The problems are:

• the requirement to have golangci-lint and the plugins build with the same stack.
• the requirement to store and rebuild the plugins when you need to share them (with other devs or CI).
• limited OS support

And I will cite the Go documentation:

Together, these restrictions mean that, in practice, the application and its plugins must all be built together by a single person or component of a system.
https://pkg.go.dev/plugin#hdr-Warnings

For me, it's clear that Go plugin system is not adapted to extend golangci-lint.

In that case, it may be simpler for that person or component to generate Go source files that blank-import the desired set of plugins and then compile a static executable in the usual way.
https://pkg.go.dev/plugin#hdr-Warnings

I propose to use a different approach that follows the Go recommendation, the principle is similar to SQL drivers in Go:

• the plugins are simple Go dependencies that follow a contract and are imported with a blank-import.
• a plugin uses init to register itself.
• inside golangci-lint we read the register to load the plugins.

FYI I already created something similar on https://github.com/kvtools/valkeyrie.

I put how-to.md and readme.md inside the directory pkg/experimental and its subdirectories.

pkg/experimental
├── commands
│   ├── custom                    # The command to build a custom version of golangci-lint with custom linters.
│   │   ├── custom.go
│   │   └── internal
│   │       ├── builder.go
│   │       ├── configuration.go
│   │       └── imports.go
│   ├── .mygcl.reference.yml      # The configuration reference.
│   └── mygcl.schema.json         # The JSONSchema of the configuration.
├── db
│   ├── builder_plugin_module.go  # the new plugin loader.
│   ├── builder_plugin_wrapper.go # temporary: it's just because it was easy to dev with that.
│   └── readme.md
├── how-to.md                     # How to use this POC.
├── modules
│   ├── myplugin                  # A plugin example (should be seen as an external module).
│   │   ├── myplugin.go
│   │   └── readme.md
│   └── register                  # the register library (should be seen as an external module).
│       ├── readme.md
│       └── register.go
└── todo.md                       # Just personal notes.

There is a new command: golangci-lint custom

If we want to follow this way we can either keep the command inside golangci-lint or use it as an external tool (inside the org).

The idea is to place the configuration file .mygcl.yml into a project that requires a plugins and then just run golangci-lint custom.
A binary gcl-custom will be automatically built and placed where the command has been run.

You should read the file pkg/experimental/how-to.md for more details.

Fixes #4427
Fixes #2505
Fixes #1276

Maybe a fix for #3669, #3826, #2566

[ldez]

FYI The failure of the CI is expected because it's just a POC.

[ldez]

@pohly I'm interested in your feedback about this new way to use plugins.
I think it can simplify your workflow inside Kubernetes.

[firelizzard18]

I like this design. I've been using a custom binary (the manual way, more or less) that I hacked to load my own plugins. From reading this PR, I'm confident it will cover my current use case without having to resort to hacks.

It would be nice to have a way for golangci-lint run to automatically build and execute a custom binary. I have been using a custom binary and that works fine, but the vscode-go plugin likes rebuilding golangci-lint, which clobbers my custom binary. Of course I could (and probably should) rename my custom binary something else and configure vscode-go to use that instead, but it would be rather convenient if golangci-lint run worked (built and executed a custom binary) with just a bit of configuration.

[ldez]

With this POC you can generate a binary called golangci-lint and use it to generate another binary also called golangci-lint, etc.

I prefer to keep the thing separated: one command to run, one command to build.
Custom plugins are a niche, I think it's not worth the cost of creating an "auto-magical" run command.

Maybe the vscode-go plugin can offer an option to handle the binary (name, path, etc.)

[firelizzard18]

That makes sense. Once this change lands I can submit a PR to vscode-go to detect .mygcl.yml and prompt the user to set a setting that will automatically build with golangci-lint custom. That will allow users with custom plugins to opt-in to using custom-built binary mode.

[pohly]

This looks good to me. I've been proposing a similar mechanism myself elsewhere, without knowing about this SQL driver precedent.

I'm not sure whether we would use this in Kubernetes, though: we already rebuild in the CI from source, so building the Go plugin in the same environment is not a burden. Because we build with go install, adding a file to the build would be more complicated than what we do now.

I'm aware that go install is not recommended. It has been working so far.

[ldez]

@bombsimon do you have an opinion about this PR?
Should I start to work on the cleaning of this PR?

[bombsimon]

I don't have much experience with how a plugin interface would look like without re-building so with that in mind this looks fine to me. But it also makes me feel, if shelling out to git and go replacing directives to then re-build golangci-lint from source is the proposed solution, it's not too far from adding my linter as a regular git patch and apply them and re-build golangci-lint. Obviously this would be a more stable interface and better user experience but yeah...

I'm also curious about the decision to bootstrap and build golangci-lint. I know there's been requested to use a specific version of golangci-lint based on the configuration and one of the reasons we didn't want to do that was to avoid external network calls to fetch and build/install golangci-lint. Has that changed or is there a difference in your opinion?

Either way, the current plugin system is crap and this is a great initiative! I don't have any better suggestion and it seems to satisfy users who requested this! 👍

I'm also not sure it solves #3669 because that requests to run plugins without re-building golangci-lint binary but that's exactly what this proposal does. You worked a lot with yaegi that the issue links to. Is something like that not suitable at all here?

[ldez]

it's not too far from adding my linter as a regular git patch and apply them and re-build golangci-lint.

The goal of an official way to build plugins is just to simplify the usage and provide something stable, but you can do the same manually with different approaches.

Has that changed or is there a difference in your opinion?

I don't remember the "check the version" topic 🤔

From my memory, the call to external resources was to get something not controlled by golangci-lint or a public source of trust, and updatable at runtime.

Here, we rely on GitHub (only for the golangci-lint repo) and Go proxies, they are not perfect sources of trust but at least it's manageable from the point of view of the security.
Also, this cannot be changed at runtime and the resources are called only on running explicitly a specific command.

At least, this approach is more secure than the current Go plugin system https://twitter.com/dominikhonnef/status/1394766501157167112

You worked a lot with yaegi that the issue links to. Is something like that not suitable at all here?

During the creation of Yaegi, I experimented with multiple interpreters.
I know the current limitations and problems related to interpreters (ex: the dependencies).

Currently, we already embedded an interpreter: ruleguard.

Also we (the ruleguard author, Marc (the author of Yaegi), and I) already privately discussed the topic of Go interpreters.
There is initiative but the subject is still open.

We can also talk about WASM (I also worked on a WASM plugin integration in Go), but IMHO it's not ready to be used inside something like golangci-lint.

[bombsimon]

Thanks for elaborating, it sounds like you've given this a lot of though and considered the alternatives!

I'm mostly asking because once this lands I guess it will be a part of golangci-lint for quite some time, even if there are progress with an alternative solution based on interpreters or wasm.

I think a wasm solution could be interesting to explore as well. I've seen projects like https://github.com/knqyf263/go-plugin which seems pretty cool although I didn't dig much deeper to see how relevant it is. But also speaking of the linked inspiration by HashiCorp that seems pretty well tested, have you had any look to see if something like https://github.com/hashicorp/go-plugin would fit? It looks pretty nice to run the plugin in a subprocess and use rpc communication.

But looking at this poc implementation there doesn't seem to be a lot to maintain and I don't have any concrete alternative or relevant experience so don't interpret my questions as if I'm not for this suggestion - I'm all for it and just want to bounce some alternatives.

Btw do you plan to deprecate the current plugin system closely after or will they live in parallel?

[ldez]

But also speaking of the linked inspiration by HashiCorp that seems pretty well tested

This kind of plugin is extremely slow (from the POV of golangci-lint) but it's not the main problem.
It's RPC communication, then the communication is based on network calls and so requires that everything be serializable, I'm not sure if it's possible or if I want to create and maintain a binding for the goanalysis package.

Like the other ways to create plugins, the pros and cons depend on the usage context, there is no silver bullet.

golangci-lint needs a high-performance extension system and easy maintenance.

Btw do you plan to deprecate the current plugin system closely after or will they live in parallel?

The 2 plugin systems can live together, and they don't require a lot of maintenance.
But this also doesn't mean that we will never deprecate one of those systems: we don't know the future 😄

[ldez]

The PR is now ready for review.

The 2 first commits are the POC and the revert of the POC commit, then you can ignore them.

The other commits contain one focused change by commit (I hope easy to review).

The doc is far from perfect but I think it's a good start.

The new module plugin-module-register will be released at the end of the review phase before the merge.
(It's a dedicated repo because sub-modules have been created to generate a git hell 😸, so I prefer to don't use them 🛑)

To test the PR

1. modify the file pkg/commands/internal/builder.go: replace "https://github.com/golangci/golangci-lint.git", by "https://github.com/ldez/golangci-lint.git",
2. define the file .custom-gcl.yml:

version: feat/new-plugin-module
name: custom-golangci-lint
plugins:
  - module: 'github.com/golangci/example-plugin-module-linter'
    version: v0.1.0

3. run: go run ./cmd/golangci-lint custom -v
4. use this configuration:

linters-settings:
  custom:
    example:
      type: module
      description: Example
      original-url: github.com/golangci/example-plugin-module-linter
      settings:
        one: Foo
        two:
          - name: Bar
        three:
          name: Bar

linters:
  disable-all: true
  enable:
    - example

5. run: ./custom-golangci-lint run

[bombsimon]

⭐

[ldez]

It's time to merge this PR 🎉

[Antonboom]

Sorry for noise 🙏
Cool job.

[ccoVeille]

I just found you implemented this @ldez, it's amazing

it will change the way we had to work with the home-made plugin.

We might have to rewrite a lot of things, but at least we will avoid the iterative fix we had

You make my day @ldez

Thanks !!!

I will take a look at https://github.com/golangci/example-plugin-module-linter

More to come here https://github.com/synthesio/zconfigcheck/ for our https://github.com/synthesio/zconfig lib