diff --git a/server/Makefile b/server/Makefile index 198373a7b5..7ffe02170d 100644 --- a/server/Makefile +++ b/server/Makefile @@ -112,6 +112,7 @@ GO_MINOR_VERSION = $(shell $(GO) version | cut -c 14- | cut -d' ' -f1 | cut -d'. MINIMUM_SUPPORTED_GO_MAJOR_VERSION = 1 MINIMUM_SUPPORTED_GO_MINOR_VERSION = 15 GO_VERSION_VALIDATION_ERR_MSG = Golang version is not supported, please update to at least $(MINIMUM_SUPPORTED_GO_MAJOR_VERSION).$(MINIMUM_SUPPORTED_GO_MINOR_VERSION) +GO_COMPATIBILITY_TEST_VERSIONS := 1.22.7 1.23.6 # GOOS/GOARCH of the build host, used to determine whether we're cross-compiling or not BUILDER_GOOS_GOARCH="$(shell $(GO) env GOOS)_$(shell $(GO) env GOARCH)" @@ -283,6 +284,13 @@ else $(MAKE) mmctl-build endif +golang-versions: ## Install Golang versions used for compatibility testing (e.g. plugins) + @for version in $(GO_COMPATIBILITY_TEST_VERSIONS); do \ + $(GO) install golang.org/dl/go$$version@latest && \ + $(GOBIN)/go$$version download; \ + done + export GO_COMPATIBILITY_TEST_VERSIONS="${GO_COMPATIBILITY_TEST_VERSIONS}" + golangci-lint: ## Run golangci-lint on codebase $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.57.1 @@ -412,7 +420,7 @@ modules-tidy: ## Tidy Go modules -cd public && $(GO) mod tidy mv enterprise/external_imports.go.orig enterprise/external_imports.go -test-server-pre: check-prereqs-enterprise start-docker gotestsum ## Runs tests. +test-server-pre: check-prereqs-enterprise start-docker gotestsum golang-versions ## Runs tests. ifeq ($(BUILD_ENTERPRISE_READY),true) @echo Running all tests else diff --git a/server/channels/app/plugin_api_test.go b/server/channels/app/plugin_api_test.go index 8ed8034bf2..e259ff5025 100644 --- a/server/channels/app/plugin_api_test.go +++ b/server/channels/app/plugin_api_test.go @@ -1614,7 +1614,8 @@ func TestInterpluginPluginHTTP(t *testing.T) { defer th.TearDown() setupMultiPluginAPITest(t, - []string{` + []string{ + ` package main import ( @@ -1759,8 +1760,7 @@ func TestAPIMetrics(t *testing.T) { pluginID := model.NewId() backend := filepath.Join(pluginDir, pluginID, "backend.exe") - code := - ` + code := ` package main import ( @@ -1896,6 +1896,23 @@ func TestPluginHTTPConnHijack(t *testing.T) { require.Equal(t, "OK", string(body)) } +func makePluginHTTPRequest(t *testing.T, pluginID string, port int, token string) string { + t.Helper() + client := &http.Client{} + reqURL := fmt.Sprintf("http://localhost:%d/plugins/%s", port, pluginID) + req, err := http.NewRequest("GET", reqURL, nil) + require.NoError(t, err) + req.Header.Set(model.HeaderAuth, model.HeaderToken+" "+token) + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return string(body) +} + func TestPluginMFAEnforcement(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -1942,22 +1959,6 @@ func TestPluginMFAEnforcement(t *testing.T) { }) require.Nil(t, appErr) - client := &http.Client{} - makeRequest := func() string { - reqURL := fmt.Sprintf("http://localhost:%d/plugins/%s", th.Server.ListenAddr.Port, pluginID) - req, err := http.NewRequest("GET", reqURL, nil) - require.NoError(t, err) - req.Header.Set(model.HeaderAuth, model.HeaderToken+" "+session.Token) - - resp, err := client.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - require.NoError(t, err) - return string(body) - } - t.Run("MFA not enforced", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableMultifactorAuthentication = true @@ -1965,7 +1966,7 @@ func TestPluginMFAEnforcement(t *testing.T) { }) // Should return user ID since MFA is not enforced - userID := makeRequest() + userID := makePluginHTTPRequest(t, pluginID, th.Server.ListenAddr.Port, session.Token) assert.Equal(t, user.Id, userID) }) @@ -1976,7 +1977,7 @@ func TestPluginMFAEnforcement(t *testing.T) { }) // Should return empty string since MFA is enforced but not active - userID := makeRequest() + userID := makePluginHTTPRequest(t, pluginID, th.Server.ListenAddr.Port, session.Token) assert.Empty(t, userID) }) } @@ -2806,3 +2807,42 @@ func TestPluginPatchChannelMembersNotifications(t *testing.T) { assert.Equal(t, "", updated.NotifyProps["test_field"]) }) } + +func TestPluginServeHTTPCompatibility(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + pluginCode := ` + package main + + import ( + "net/http" + "github.com/mattermost/mattermost/server/public/plugin" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) { + w.Write([]byte("plugin response")) + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + ` + + for _, goVersion := range strings.Fields(os.Getenv("GO_COMPATIBILITY_TEST_VERSIONS")) { + t.Run(goVersion, func(t *testing.T) { + tearDown, ids, errs := SetAppEnvironmentWithPluginsGoVersion(t, []string{pluginCode}, th.App, th.NewPluginAPI, goVersion) + defer tearDown() + require.NoError(t, errs[0]) + require.Len(t, ids, 1) + pluginID := ids[0] + + res := makePluginHTTPRequest(t, pluginID, th.Server.ListenAddr.Port, "") + assert.Equal(t, "plugin response", res) + }) + } +} diff --git a/server/channels/app/plugin_hooks_test.go b/server/channels/app/plugin_hooks_test.go index 4d4e92b65f..65d4874f9e 100644 --- a/server/channels/app/plugin_hooks_test.go +++ b/server/channels/app/plugin_hooks_test.go @@ -31,6 +31,14 @@ import ( ) func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, apiFunc func(*model.Manifest) plugin.API) (func(), []string, []error) { + return setAppEnvironmentWithPlugins(t, pluginCode, app, apiFunc, "") +} + +func SetAppEnvironmentWithPluginsGoVersion(t *testing.T, pluginCode []string, app *App, apiFunc func(*model.Manifest) plugin.API, goVersion string) (func(), []string, []error) { + return setAppEnvironmentWithPlugins(t, pluginCode, app, apiFunc, goVersion) +} + +func setAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, apiFunc func(*model.Manifest) plugin.API, goVersion string) (func(), []string, []error) { pluginDir, err := os.MkdirTemp("", "") require.NoError(t, err) webappPluginDir, err := os.MkdirTemp("", "") @@ -45,7 +53,7 @@ func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, a for _, code := range pluginCode { pluginID := model.NewId() backend := filepath.Join(pluginDir, pluginID, "backend.exe") - utils.CompileGo(t, code, backend) + utils.CompileGoVersion(t, goVersion, code, backend) err = os.WriteFile(filepath.Join(pluginDir, pluginID, "plugin.json"), []byte(`{"id": "`+pluginID+`", "server": {"executable": "backend.exe"}}`), 0600) require.NoError(t, err) @@ -327,7 +335,8 @@ func TestHookMessageHasBeenPosted(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + `, + }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) defer tearDown() post := &model.Post{ @@ -366,7 +375,8 @@ func TestHookMessageWillBeUpdated(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() post := &model.Post{ @@ -414,7 +424,8 @@ func TestHookMessageHasBeenUpdated(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + `, + }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) defer tearDown() post := &model.Post{ @@ -460,7 +471,8 @@ func TestHookMessageHasBeenDeleted(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + `, + }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) defer tearDown() post := &model.Post{ @@ -726,7 +738,8 @@ func TestUserWillLogIn_Blocked(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() r := &http.Request{} @@ -766,7 +779,8 @@ func TestUserWillLogInIn_Passed(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() r := &http.Request{} @@ -808,7 +822,8 @@ func TestUserHasLoggedIn(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() r := &http.Request{} @@ -850,7 +865,8 @@ func TestUserHasBeenDeactivated(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() user := &model.User{ @@ -898,7 +914,8 @@ func TestUserHasBeenCreated(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() user := &model.User{ @@ -943,7 +960,8 @@ func TestErrorString(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() require.Len(t, activationErrors, 1) @@ -973,7 +991,8 @@ func TestErrorString(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() require.Len(t, activationErrors, 1) @@ -1029,7 +1048,8 @@ func TestHookContext(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + `, + }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) defer tearDown() post := &model.Post{ @@ -1077,7 +1097,8 @@ func TestActiveHooks(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() require.Len(t, pluginIDs, 1) @@ -1133,8 +1154,7 @@ func TestHookMetrics(t *testing.T) { pluginID := model.NewId() backend := filepath.Join(pluginDir, pluginID, "backend.exe") - code := - ` + code := ` package main import ( @@ -1241,7 +1261,8 @@ func TestHookReactionHasBeenAdded(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + `, + }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) defer tearDown() reaction := &model.Reaction{ @@ -1283,7 +1304,8 @@ func TestHookReactionHasBeenRemoved(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + `, + }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) defer tearDown() reaction := &model.Reaction{ @@ -1326,7 +1348,8 @@ func TestHookRunDataRetention(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() require.Len(t, pluginIDs, 1) @@ -1370,7 +1393,8 @@ func TestHookOnSendDailyTelemetry(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() require.Len(t, pluginIDs, 1) @@ -1414,7 +1438,8 @@ func TestHookOnCloudLimitsUpdated(t *testing.T) { func main() { plugin.ClientMain(&MyPlugin{}) } - `}, th.App, th.NewPluginAPI) + `, + }, th.App, th.NewPluginAPI) defer tearDown() require.Len(t, pluginIDs, 1) diff --git a/server/public/plugin/client_rpc.go b/server/public/plugin/client_rpc.go index ffe849b844..a62366927e 100644 --- a/server/public/plugin/client_rpc.go +++ b/server/public/plugin/client_rpc.go @@ -16,6 +16,7 @@ import ( "log" "net/http" "net/rpc" + "net/url" "os" "reflect" "sync" @@ -59,7 +60,8 @@ func (p *hooksPlugin) Server(b *plugin.MuxBroker) (any, error) { } func (p *hooksPlugin) Client(b *plugin.MuxBroker, client *rpc.Client) (any, error) { - return &hooksRPCClient{client: client, + return &hooksRPCClient{ + client: client, log: p.log, muxBroker: b, apiImpl: p.apiImpl, @@ -171,8 +173,10 @@ func init() { // These enforce compile time checks to make sure types implement the interface // If you are getting an error here, you probably need to run `make pluginapi` to // autogenerate RPC glue code -var _ plugin.Plugin = &hooksPlugin{} -var _ Hooks = &hooksRPCClient{} +var ( + _ plugin.Plugin = &hooksPlugin{} + _ Hooks = &hooksRPCClient{} +) // // Below are special cases for hooks or APIs that can not be auto generated @@ -318,8 +322,7 @@ func (s *hooksRPCServer) OnActivate(args *Z_OnActivateArgs, returns *Z_OnActivat return nil } -type Z_LoadPluginConfigurationArgsArgs struct { -} +type Z_LoadPluginConfigurationArgsArgs struct{} type Z_LoadPluginConfigurationArgsReturns struct { A []byte @@ -358,9 +361,39 @@ func init() { hookNameToId["ServeHTTP"] = ServeHTTPID } +// Using a subset of http.Request prevents a known incompatibility when decoding Go v1.23+ gob-encoded x509.Certificate +// structs from Go v1.22 compiled plugins. These come from http.Request.TLS field (*tls.ConnectionState). +type HTTPRequestSubset struct { + Method string + URL *url.URL + Proto string + ProtoMajor int + ProtoMinor int + Header http.Header + Host string + RemoteAddr string + RequestURI string + Body io.ReadCloser +} + +func (r *HTTPRequestSubset) GetHTTPRequest() *http.Request { + return &http.Request{ + Method: r.Method, + URL: r.URL, + Proto: r.Proto, + ProtoMajor: r.ProtoMajor, + ProtoMinor: r.ProtoMinor, + Header: r.Header, + Host: r.Host, + RemoteAddr: r.RemoteAddr, + RequestURI: r.RequestURI, + Body: r.Body, + } +} + type Z_ServeHTTPArgs struct { ResponseWriterStream uint32 - Request *http.Request + Request *HTTPRequestSubset Context *Context RequestBodyStream uint32 } @@ -402,7 +435,7 @@ func (g *hooksRPCClient) ServeHTTP(c *Context, w http.ResponseWriter, r *http.Re }() } - forwardedRequest := &http.Request{ + forwardedRequest := &HTTPRequestSubset{ Method: r.Method, URL: r.URL, Proto: r.Proto, @@ -447,19 +480,21 @@ func (s *hooksRPCServer) ServeHTTP(args *Z_ServeHTTPArgs, returns *struct{}) err } defer r.Body.Close() + httpReq := r.GetHTTPRequest() + if hook, ok := s.impl.(interface { ServeHTTP(c *Context, w http.ResponseWriter, r *http.Request) }); ok { - hook.ServeHTTP(args.Context, w, r) + hook.ServeHTTP(args.Context, w, httpReq) } else { - http.NotFound(w, r) + http.NotFound(w, httpReq) } return nil } type Z_PluginHTTPArgs struct { - Request *http.Request + Request *HTTPRequestSubset RequestBody []byte } @@ -469,7 +504,7 @@ type Z_PluginHTTPReturns struct { } func (g *apiRPCClient) PluginHTTP(request *http.Request) *http.Response { - forwardedRequest := &http.Request{ + forwardedRequest := &HTTPRequestSubset{ Method: request.Method, URL: request.URL, Proto: request.Proto, @@ -514,7 +549,7 @@ func (s *apiRPCServer) PluginHTTP(args *Z_PluginHTTPArgs, returns *Z_PluginHTTPR if hook, ok := s.impl.(interface { PluginHTTP(request *http.Request) *http.Response }); ok { - response := hook.PluginHTTP(args.Request) + response := hook.PluginHTTP(args.Request.GetHTTPRequest()) responseBody, err := io.ReadAll(response.Body) if err != nil { @@ -743,8 +778,7 @@ type Z_LogDebugArgs struct { B []any } -type Z_LogDebugReturns struct { -} +type Z_LogDebugReturns struct{} func (g *apiRPCClient) LogDebug(msg string, keyValuePairs ...any) { stringifiedPairs := stringifyToObjects(keyValuePairs) @@ -771,8 +805,7 @@ type Z_LogInfoArgs struct { B []any } -type Z_LogInfoReturns struct { -} +type Z_LogInfoReturns struct{} func (g *apiRPCClient) LogInfo(msg string, keyValuePairs ...any) { stringifiedPairs := stringifyToObjects(keyValuePairs) @@ -799,8 +832,7 @@ type Z_LogWarnArgs struct { B []any } -type Z_LogWarnReturns struct { -} +type Z_LogWarnReturns struct{} func (g *apiRPCClient) LogWarn(msg string, keyValuePairs ...any) { stringifiedPairs := stringifyToObjects(keyValuePairs) @@ -827,8 +859,7 @@ type Z_LogErrorArgs struct { B []any } -type Z_LogErrorReturns struct { -} +type Z_LogErrorReturns struct{} func (g *apiRPCClient) LogError(msg string, keyValuePairs ...any) { stringifiedPairs := stringifyToObjects(keyValuePairs) @@ -960,7 +991,7 @@ func init() { type Z_ServeMetricsArgs struct { ResponseWriterStream uint32 - Request *http.Request + Request *HTTPRequestSubset Context *Context RequestBodyStream uint32 } @@ -1002,7 +1033,7 @@ func (g *hooksRPCClient) ServeMetrics(c *Context, w http.ResponseWriter, r *http }() } - forwardedRequest := &http.Request{ + forwardedRequest := &HTTPRequestSubset{ Method: r.Method, URL: r.URL, Proto: r.Proto, @@ -1047,12 +1078,14 @@ func (s *hooksRPCServer) ServeMetrics(args *Z_ServeMetricsArgs, returns *struct{ } defer r.Body.Close() + httpReq := r.GetHTTPRequest() + if hook, ok := s.impl.(interface { ServeMetrics(c *Context, w http.ResponseWriter, r *http.Request) }); ok { - hook.ServeMetrics(args.Context, w, r) + hook.ServeMetrics(args.Context, w, httpReq) } else { - http.NotFound(w, r) + http.NotFound(w, httpReq) } return nil diff --git a/server/public/plugin/utils/test_files_compiler.go b/server/public/plugin/utils/test_files_compiler.go index 904cb4b6e9..626a0a4ce2 100644 --- a/server/public/plugin/utils/test_files_compiler.go +++ b/server/public/plugin/utils/test_files_compiler.go @@ -15,6 +15,18 @@ import ( ) func CompileGo(t *testing.T, sourceCode, outputPath string) { + compileGo(t, "go", sourceCode, outputPath) +} + +func CompileGoVersion(t *testing.T, goVersion, sourceCode, outputPath string) { + var goBin string + if goVersion != "" { + goBin = os.Getenv("GOBIN") + } + compileGo(t, filepath.Join(goBin, "go"+goVersion), sourceCode, outputPath) +} + +func compileGo(t *testing.T, goBin, sourceCode, outputPath string) { dir, err := os.MkdirTemp(".", "") require.NoError(t, err) defer os.RemoveAll(dir) @@ -32,7 +44,7 @@ func CompileGo(t *testing.T, sourceCode, outputPath string) { serverPath := filepath.Dir(filepath.Dir(sourceFile)) out := &bytes.Buffer{} - cmd := exec.Command("go", "build", "-o", outputPath, main) + cmd := exec.Command(goBin, "build", "-o", outputPath, main) cmd.Dir = serverPath cmd.Stdout = out cmd.Stderr = out