* short-circuit plugin activation steps if executable is not found
* improve error message by removing redundant ref to plugin and mor consise language about server environment
* MM-37165: Fix improper plugin shutdown
This was caught from a race test failure. While the failure manifested due to a log being
written from a test after the test exited, the real reason was hidden further deeper.
What was happening is that the server would always listen for plugin requests in a
separate goroutine via `g.muxBroker.AcceptAndServe` in the `OnActivate` hook. But the
plugin shutdown process would just close the plugin connections and move on, leading
to the classic case of improperly shut down goroutines.
When this happened, an opportunity opens up in a way that the server
would still be executing a request whereas the main goroutine and therefore the parent
test has already finished. This would lead to an error like
```
{"level":"error","ts":1626451258.4141135,"caller":"mlog/sugar.go:25","msg":"pluginAPI scheduleOnce poller encountered an error but is still polling","plugin_id":"com.mattermost.plugin-incident-management","error":"ListPluginKeys: Unable to list all the plugin keys., failed to get PluginKeyValues with pluginId=com.mattermost.plugin-incident-management: sql: database is closed
```
And now, this finally calls `mlog.Error`, which finally triggers our race condition :)
To fix this, we use some basic synchronization via waitgroups and just wait for it
to finish after closing the plugin process.
https://mattermost.atlassian.net/browse/MM-37165
```release-note
NONE
```
* gofmt
```release-note
NONE
```
* split waitgroup additions
```release-note
NONE
```
This PR builds up on the pass-through DB driver to a fully functioning DB driver implementation via our RPC layer.
To keep things separate from the plugin RPC API, and have the ability to move fast with changes, a separate field Driver is added to MattermostPlugin. Typically the field which is required to be compatible are the API and Helpers. It would be well-documented that Driver is purely for internal use by Mattermost plugins.
A new Driver interface was created which would have a client and server implementation. Every object (connection, statement, etc.) is created and added to a map on the server side. On the client side, the wrapper structs hold the object id, and communicate via the RPC API using this id.
When the server gets the object id, it picks up the appropriate object from its map and performs the operation, and sends back the data.
Some things that need to be handled are errors. Typical error types like pq.Error and mysql.MySQLError are registered with encoding/gob. But for error variables like sql.ErrNoRows, a special integer is encoded with the ErrorString struct. And on the cilent side, the integer is checked, and the appropriate error variable is returned.
Some pending things:
- Context support. This is tricky. Since context.Context is an interface, it's not possible to marshal it. We have to find a way to get the timeout value from the context and pass it.
- RowsColumnScanType(rowsID string, index int) reflect.Type API. Again, reflect.Type is an interface.
- Master/Replica API support.
* format using `goimports -local github.com/mattermost/mattermost-server/v5 -w`
* added goimports lint check to .golangci.yml
* format using `goimports -local github.com/mattermost/mattermost-server/v5 -w` for a corner case
* make app-layers, *-mocks and store-layers for ci check
Co-authored-by: Mahmudul Haque <mahmudulhaque@protonmail.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
* MM-31063: Change constants to use CamelCase
* store package
* change allcaps to camel case (#16615)
* New tools.mod
Co-authored-by: Ibrahim Serdar Acikgoz <serdaracikgoz86@gmail.com>
* explicitly assert panic as error log
* Revert "[MM-18150] plugin panic trace should not be lost (#13559)"
This reverts commit 5d928b4f94, while leaving the unit tests intact
and now asserting debug logs instead.
* missing license header
* Transit panic from debug to error
* Parse plugin's StdErr and output panic to the mlog.Error
* Add unit tests
* Change log test
* Remove buffer from logger
* Remove 'panic' string filter
* Change *Buffer to io.Writer
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* MM-21019 - Fix race in (*Environment).SetPluginState()
- We change from passing pointers to registeredPlugin to passing
the struct by value.
- We also add a mutex to the supervisor struct to protect
from racy data access.
* move the immutability comment to the godoc of the Active method
* Changing mut to lock
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* introduce plugin health check
* implement plugin health check job
* add support for checking pid of plugin process and RPC ping, to determine a plugin's health
* implement restart policy with back-offs
* support "EnableHealthCheck" boolean from config file.
* add tests for supervisor.PerformHealthCheck() and shouldDeactivatePlugin()
* improve error handling. clean up if blocks to be more concise
* Fix shadowed variables in cmd package
* Fix shadowed variables in plugin package
* Fix shadowed variables in store package
* Fix shadowed variables in web package
* Changes as requested
Signed-off-by: Hanzei <hanzei@mailbox.org>
* Fix build
* Remove unnessary statements
* Use require all the time
* Fix build
* Rename variables according to feedback
* Fix NPE
* Changes as requested
Fix code issues in channel_test.go
Fix Channel Test Issues detected by Megacheck
Fix API Emoji Test Issues detected by Megacheck
Fixed API Issues Reported by Megacheck
Fixed App issues reported by megacheck
Remaining fixes
removed test added by mistake from old HEAD
gofmt
Store Fixes
simplified returns
Fix test for multi member channel delete
revert to delete unused function
* ensure plugin is always shutdown
Once we call `.client.Client()` the plugin has started, and must be shut
down. `newSupervisor` sometimes returned with an error (and without a
reference to the supervisor), leaving the client running indefinitely.
* Clarify the documentation to explain that plugin hooks will not trigger until `OnActivate` returns successfully, and will stop triggering just before `OnDeactivate` is called.
* test for plugin deadlock
* plugin/environment.go: switch to sync.Map
From: https://golang.org/pkg/sync/#Map
> If a goroutine holds a RWMutex for reading and another goroutine might call Lock, no goroutine should expect to be able to acquire a read lock until the initial read lock is released. In particular, this prohibits recursive read locking. This is to ensure that the lock eventually becomes available; a blocked Lock call excludes new readers from acquiring the lock.
The previous `RWMutex` was not safe given that we effectively acquired read locks recursively (hook -> api -> hook). This worked up until we activated or deactivated plugins, tried to acquire a write lock, and the plugin used the API to effectively trigger another hook.
Switching to sync.Map avoids this by divesting the need to lock at all, avoiding the potential for a recursive lock in the first place.
* clean up plugins GoDoc:
- eliminate plugin.NewBlankContext() as unnecessary
- export ValidIdRegex as a string vs. the less readable var
- add/update various documentation strings
- hide everything by default, except where used by client plugins or the mattermost-server. The exception to this rule are the `*(Args|Returns)` structs which must be public for go-plugin, but are now prefixed with `Z_` with a warning not to use.
- include a top-level example to get plugin authors started
This is not a breaking change for existing plugins compiled against
plugins-v2.
* remove commented out ServeHTTPResponseWriter
* update examples to match developer docs
* add missing plugin/doc.go license header
* allow `Wait()`ing on the supervisor
In the event the plugin supervisor shuts down a plugin for crashing too
many times, the new `Wait()` interface allows the `ActivatePlugin` to
accept a callback function to trigger when `supervisor.Wait()` returns.
If the supervisor shuts down normally, this callback is invoked with
a nil error, otherwise any error reported by the supervisor is passed
along.
* improve plugin activation/deactivation logic
Avoid triggering activation of previously failed-to-start plugins just
becase something in the configuration changed. Now, intelligently
compare the global enable bit as well as the each individual plugin's
enabled bit.
* expose store to manipulate PluginStatuses
* expose API to fetch plugin statuses
* keep track of whether or not plugin sandboxing is supported
* transition plugin statuses
* restore error on plugin activation if already active
* don't initialize test plugins until successfully loaded
* emit websocket events when plugin statuses change
* skip pruning if already initialized
* MM-8622: maintain plugin statuses in memory
Switch away from persisting plugin statuses to the database, and
maintain in memory instead. This will be followed by a cluster interface
to query the in-memory status of plugin statuses from all cluster nodes.
At the same time, rename `cluster_discovery_id` on the `PluginStatus`
model object to `cluster_id`.
* MM-8622: aggregate plugin statuses across cluster
* fetch cluster plugin statuses when emitting websocket notification
* address unit test fixes after rebasing
* relax (poor) racey unit test re: supervisor.Wait()
* make store-mocks
* begin backend plugin wip
* flesh out rpcplugin. everything done except for minor supervisor stubs
* done with basic plugin infrastructure
* simplify tests
* remove unused test lines
