[MM-63345] Address Go v1.23 incompatibility issues with plugins (#30386)

* Address Go v1.23 incompatibility issues with plugins

* Install multiple Go versions for compatibility tests

* Rename

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

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)
+		})
+	}
+}

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)

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 (
-var _ Hooks = &hooksRPCClient{}
+	_ 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

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