Merge branch 'main' into kegan/client-api

Author: kegsay

ONBOARDING.md

@@ -185,10 +185,6 @@ To conditionally skip an entire *file* based on the homeserver being run, add a
 ```go
 // +build !dendrite_blacklist
 ```
-You can also do this based on features for MSC tests (which means you must run Complement *with* this tag for these tests *to run*):
-```go
-// +build msc_2836
-```
 See [GH Actions](https://github.com/matrix-org/complement/blob/master/.github/workflows/ci.yaml) for an example of how this is used for different homeservers in practice.
 
 ### Why do we use `t.Errorf` sometimes and `t.Fatalf` other times?
@@ -210,11 +206,11 @@ For Goland:
  * Under "Run"->"Edit Configurations..."->"Edit Configuration Templates..."->"Go Test", and add `COMPLEMENT_BASE_IMAGE=complement-dendrite:latest` to "Environment"
  * Then you can right-click on any test file or test case and "Run <test name>".
 
-	
+
 ### How do I make the linter checks pass?
 
 Use [`goimports`](https://pkg.go.dev/golang.org/x/tools/cmd/goimports) to sort imports and format in the style of `gofmt`.
-	
+
 Set this up to run on save in VSCode as follows:
 - File -> Preferences -> Settings.
   - Search for "Format On Save" and enable it.

README.md

@@ -140,24 +140,87 @@ for an example of how to do this correctly.
 
 To get started developing Complement tests, see [the onboarding documentation](ONBOARDING.md).
 
-### Build tags
+### Build tags (test blacklisting)
+
+Complement uses build tags to exclude tests for each homeserver implementation.
+Build tags are comments at the top of the file that look like:
 
-Complement uses build tags to include or exclude tests for each homeserver. Build tags are comments at the top of the file that look
-like:
 ```go
-// +build msc2403
+// +build !dendrite_blacklist
 ```
-We have tags for MSCs (the above is in `msc2403_test.go`) as well as general blacklists for a homeserver implementation e.g Dendrite,
-which has the name `dendrite_blacklist`. These are implemented as inverted tags such that specifying the tag results in the file not
-being picked up by `go test`. For example, `apidoc_presence_test.go` has:
+
+These are implemented as inverted tags, such that specifying the tag results in
+the file not being picked up by `go test`. This serves as a way to exclude
+known-broken tests per implementation.
+
+For example, `apidoc_presence_test.go` has:
+
 ```go
 // +build !dendrite_blacklist
 ```
-and all Dendrite tests run with `-tags="dendrite_blacklist"` to cause this file to be skipped. You can run tests with build tags like this:
+
+and all Dendrite tests run with `-tags="dendrite_blacklist"` to cause this file
+to be skipped. You can run tests with build tags like this:
+
 ```
-COMPLEMENT_BASE_IMAGE=complement-synapse:latest go test -v -tags="synapse_blacklist,msc2403" ./tests/...
+COMPLEMENT_BASE_IMAGE=complement-synapse:latest go test -v -tags="synapse_blacklist" ./tests/...
 ```
-This runs Complement with a Synapse HS and ignores tests which Synapse doesn't implement, and includes tests for MSC2403.
+
+This runs Complement with a Synapse HS and ignores tests which Synapse doesn't implement.
+
+The currently known blacklist tags are:
+
+* `synapse_blacklist`
+* `dendrite_blacklist`
+* `conduit_blacklist`
+* `conduwuit_blacklist`
+
+### Writing tests for unstable MSCs
+
+Complement is frequently used to test homeserver implementations of unstable
+MSCs. As these features/changes often become stable eventually and for
+convenience, this repo accepts such tests.
+
+Tests for a given MSC should be placed in a new directory under `tests/`. For
+example, to write tests for MSC9999, create a directory at `tests/msc9999`.
+
+This creates a new go "package", and tests contained within will not be run
+unless explicitly noted. A package directory should contain the following
+files:
+
+```
+tests/msc9999
+├── main_test.go
+└── msc9999_test.go
+```
+
+where `main_test.go` sets up Complement and indicates that this is a package
+containing tests:
+
+```go
+package tests
+
+import (
+    "testing"
+
+    "github.com/matrix-org/complement"
+)
+
+func TestMain(m *testing.M) {
+    complement.TestMain(m, "msc9999")
+}
+```
+
+and `msc9999_test.go` contains your actual tests. See existing `tests/msc*`
+directories for examples.
+
+You can create additional files to separate and organise logical chunks of
+tests. Just be sure each file is named `*_test.go` for `go test` to find it.
+
+Once an MSC is accepted, the tests should be migrated out of the `msc*`
+directory, as the MSC is now considered stable. Consider adding the tests to
+the blacklist of other homeserver implementations (see above section) if they
+don't yet implement the new changes described by the MSC.
 
 ## Why 'Complement'?
 

client/client.go

@@ -18,6 +18,7 @@ import (
 	"time"
 
 	"github.com/matrix-org/gomatrixserverlib"
+	"github.com/matrix-org/gomatrixserverlib/spec"
 	"github.com/tidwall/gjson"
 	"golang.org/x/crypto/curve25519"
 
@@ -139,7 +140,11 @@ func (c *CSAPI) CreateRoom(t ct.TestLike, body map[string]interface{}) *http.Res
 }
 
 // MustJoinRoom joins the room ID or alias given, else fails the test. Returns the room ID.
-func (c *CSAPI) MustJoinRoom(t ct.TestLike, roomIDOrAlias string, serverNames []string) string {
+//
+// Args:
+//   - `serverNames`: The list of servers to attempt to join the room through.
+//     These should be a resolvable addresses within the deployment network.
+func (c *CSAPI) MustJoinRoom(t ct.TestLike, roomIDOrAlias string, serverNames []spec.ServerName) string {
 	t.Helper()
 	res := c.JoinRoom(t, roomIDOrAlias, serverNames)
 	mustRespond2xx(t, res)
@@ -153,12 +158,19 @@ func (c *CSAPI) MustJoinRoom(t ct.TestLike, roomIDOrAlias string, serverNames []
 }
 
 // JoinRoom joins the room ID or alias given. Returns the raw http response
-func (c *CSAPI) JoinRoom(t ct.TestLike, roomIDOrAlias string, serverNames []string) *http.Response {
+//
+// Args:
+//   - `serverNames`: The list of servers to attempt to join the room through.
+//     These should be a resolvable addresses within the deployment network.
+func (c *CSAPI) JoinRoom(t ct.TestLike, roomIDOrAlias string, serverNames []spec.ServerName) *http.Response {
 	t.Helper()
 	// construct URL query parameters
-	query := make(url.Values, len(serverNames))
-	for _, serverName := range serverNames {
-		query.Add("server_name", serverName)
+	serverNameStrings := make([]string, len(serverNames))
+	for i, serverName := range serverNames {
+		serverNameStrings[i] = string(serverName)
+	}
+	query := url.Values{
+		"server_name": serverNameStrings,
 	}
 	// join the room
 	return c.Do(

cmd/perftest/snapshot.go

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"time"
 
-	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
 	"github.com/matrix-org/complement/internal/docker"
 )
 
@@ -30,7 +30,7 @@ func snapshotStats(spanName, desc string, deployment *docker.Deployment, absDura
 		if err != nil {
 			return nil
 		}
-		var sj types.StatsJSON
+		var sj container.StatsResponse
 		err = json.NewDecoder(stats.Body).Decode(&sj)
 		stats.Body.Close()
 		if err != nil {

config/config.go

@@ -106,6 +106,14 @@ type Complement struct {
 	// disable this behaviour being added later, once this has stablised.
 	EnableDirtyRuns bool
 
+	// The IP that is used to connect to the running homeserver from the host.
+	//
+	// For Complement tests, this is always configured as `127.0.0.1` but can be
+	// overridden by homerunner to allow binding to a different IP address
+	// (`HOMERUNNER_HS_PORTBINDING_IP`).
+	//
+	// This field is used for the host-accessible homeserver URLs (as the hostname)
+	// so clients in your tests can access the homeserver.
 	HSPortBindingIP string
 
 	// Name: COMPLEMENT_POST_TEST_SCRIPT

federation/handle.go

@@ -117,7 +117,7 @@ func MakeRespMakeKnock(s *Server, room *ServerRoom, userID string) (resp fclient
 // the current server is returned to the joining server.
 func SendJoinRequestsHandler(s *Server, w http.ResponseWriter, req *http.Request, expectPartialState bool, omitServersInRoom bool) {
 	fedReq, errResp := fclient.VerifyHTTPRequest(
-		req, time.Now(), spec.ServerName(s.serverName), nil, s.keyRing,
+		req, time.Now(), s.serverName, nil, s.keyRing,
 	)
 	if fedReq == nil {
 		w.WriteHeader(errResp.Code)
@@ -208,7 +208,7 @@ func HandleInviteRequests(inviteCallback func(gomatrixserverlib.PDU)) func(*Serv
 		// https://matrix.org/docs/spec/server_server/r0.1.4#put-matrix-federation-v2-invite-roomid-eventid
 		s.mux.Handle("/_matrix/federation/v2/invite/{roomID}/{eventID}", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 			fedReq, errResp := fclient.VerifyHTTPRequest(
-				req, time.Now(), spec.ServerName(s.serverName), nil, s.keyRing,
+				req, time.Now(), s.serverName, nil, s.keyRing,
 			)
 			if fedReq == nil {
 				w.WriteHeader(errResp.Code)
@@ -236,7 +236,7 @@ func HandleInviteRequests(inviteCallback func(gomatrixserverlib.PDU)) func(*Serv
 			}
 
 			// Sign the event before we send it back
-			signedEvent := inviteRequest.Event().Sign(s.serverName, s.KeyID, s.Priv)
+			signedEvent := inviteRequest.Event().Sign(string(s.serverName), s.KeyID, s.Priv)
 
 			// Send the response
 			res := map[string]interface{}{
@@ -263,7 +263,7 @@ func HandleDirectoryLookups() func(*Server) {
 				b, err := json.Marshal(fclient.RespDirectory{
 					RoomID: roomID,
 					Servers: []spec.ServerName{
-						spec.ServerName(s.serverName),
+						s.serverName,
 					},
 				})
 				if err != nil {
@@ -432,7 +432,7 @@ func HandleMediaRequests(mediaIds map[string]func(w http.ResponseWriter)) func(*
 			origin := vars["origin"]
 			mediaId := vars["mediaId"]
 
-			if origin != srv.serverName {
+			if origin != string(srv.serverName) {
 				w.WriteHeader(400)
 				w.Write([]byte("complement: Invalid Origin; Expected " + srv.serverName))
 				return
@@ -471,7 +471,7 @@ func HandleTransactionRequests(pduCallback func(gomatrixserverlib.PDU), eduCallb
 
 			// Check federation signature
 			fedReq, errResp := fclient.VerifyHTTPRequest(
-				req, time.Now(), spec.ServerName(srv.serverName), nil, srv.keyRing,
+				req, time.Now(), srv.serverName, nil, srv.keyRing,
 			)
 			if fedReq == nil {
 				log.Printf(

federation/server.go

@@ -48,9 +48,10 @@ type Server struct {
 	// Default: true
 	UnexpectedRequestsAreErrors bool
 
-	Priv       ed25519.PrivateKey
-	KeyID      gomatrixserverlib.KeyID
-	serverName string
+	Priv  ed25519.PrivateKey
+	KeyID gomatrixserverlib.KeyID
+	// The homeserver name. This should be a resolvable address in the deployment network
+	serverName spec.ServerName
 	listening  bool
 
 	certPath string
@@ -80,7 +81,7 @@ func NewServer(t ct.TestLike, deployment FederationDeployment, opts ...func(*Ser
 		mux:   mux.NewRouter(),
 		// The server name will be updated when the caller calls Listen() to include the port number
 		// of the HTTP server e.g "host.docker.internal:56353"
-		serverName:                  deployment.GetConfig().HostnameRunningComplement,
+		serverName:                  spec.ServerName(deployment.GetConfig().HostnameRunningComplement),
 		rooms:                       make(map[string]*ServerRoom),
 		aliases:                     make(map[string]string),
 		UnexpectedRequestsAreErrors: true,
@@ -142,7 +143,7 @@ func NewServer(t ct.TestLike, deployment FederationDeployment, opts ...func(*Ser
 // It is not supported to call ServerName() before Listen() because Listen() modifies the server name.
 // Listen() will select a random OS-provided high-numbered port to listen on, which then needs to be
 // retrofitted into the server name so containers know how to route to it.
-func (s *Server) ServerName() string {
+func (s *Server) ServerName() spec.ServerName {
 	if !s.listening {
 		ct.Fatalf(s.t, "ServerName() called before Listen() - this is not supported because Listen() chooses a high-numbered port and thus changes the server name. Ensure you Listen() first!")
 	}
@@ -205,28 +206,31 @@ func (s *Server) FederationClient(deployment FederationDeployment) fclient.Feder
 		ct.Fatalf(s.t, "FederationClient() called before Listen() - this is not supported because Listen() chooses a high-numbered port and thus changes the server name and thus changes the way federation requests are signed. Ensure you Listen() first!")
 	}
 	identity := fclient.SigningIdentity{
-		ServerName: spec.ServerName(s.ServerName()),
+		ServerName: s.ServerName(),
 		KeyID:      s.KeyID,
 		PrivateKey: s.Priv,
 	}
-	f := fclient.NewFederationClient(
+	fedClient := fclient.NewFederationClient(
 		[]*fclient.SigningIdentity{&identity},
 		fclient.WithTransport(deployment.RoundTripper()),
 	)
-	return f
+	return fedClient
 }
 
 // MustSendTransaction sends the given PDUs/EDUs to the target destination, returning an error if the /send fails or if the response contains an error
 // for any sent PDUs. Times out after 10 seconds.
-func (s *Server) MustSendTransaction(t ct.TestLike, deployment FederationDeployment, destination string, pdus []json.RawMessage, edus []gomatrixserverlib.EDU) {
+//
+// Args:
+//   - `destination`: This should be a resolvable addresses within the deployment network.
+func (s *Server) MustSendTransaction(t ct.TestLike, deployment FederationDeployment, destination spec.ServerName, pdus []json.RawMessage, edus []gomatrixserverlib.EDU) {
 	t.Helper()
-	cli := s.FederationClient(deployment)
+	fedClient := s.FederationClient(deployment)
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
 	defer cancel()
-	resp, err := cli.SendTransaction(ctx, gomatrixserverlib.Transaction{
+	resp, err := fedClient.SendTransaction(ctx, gomatrixserverlib.Transaction{
 		TransactionID: gomatrixserverlib.TransactionID(fmt.Sprintf("complement-%d", time.Now().Nanosecond())),
 		Origin:        spec.ServerName(s.ServerName()),
-		Destination:   spec.ServerName(destination),
+		Destination:   destination,
 		PDUs:          pdus,
 		EDUs:          edus,
 	})
@@ -319,6 +323,9 @@ func (s *Server) MustCreateEvent(t ct.TestLike, room *ServerRoom, ev Event) goma
 
 // MustJoinRoom will make the server send a make_join and a send_join to join a room
 // It returns the resultant room.
+//
+// Args:
+//   - `remoteServer`: This should be a resolvable addresses within the deployment network.
 func (s *Server) MustJoinRoom(t ct.TestLike, deployment FederationDeployment, remoteServer spec.ServerName, roomID string, userID string, opts ...JoinRoomOpt) *ServerRoom {
 	t.Helper()
 	var jr joinRoom
@@ -401,6 +408,9 @@ func (s *Server) MustJoinRoom(t ct.TestLike, deployment FederationDeployment, re
 }
 
 // Leaves a room. If this is rejecting an invite then a make_leave request is made first, before send_leave.
+//
+// Args:
+//   - `remoteServer`: This should be a resolvable addresses within the deployment network.
 func (s *Server) MustLeaveRoom(t ct.TestLike, deployment FederationDeployment, remoteServer spec.ServerName, roomID string, userID string) {
 	t.Helper()
 	origin := spec.ServerName(s.serverName)
@@ -449,7 +459,7 @@ func (s *Server) ValidFederationRequest(t ct.TestLike, handler func(fr *fclient.
 	return func(w http.ResponseWriter, req *http.Request) {
 		// Check federation signature
 		fedReq, errResp := fclient.VerifyHTTPRequest(
-			req, time.Now(), spec.ServerName(s.serverName), nil, s.keyRing,
+			req, time.Now(), s.serverName, nil, s.keyRing,
 		)
 		if fedReq == nil {
 			ct.Errorf(t,
@@ -495,7 +505,7 @@ func (s *Server) Listen() (cancel func()) {
 		ct.Fatalf(s.t, "ListenFederationServer: net.Listen failed: %s", err)
 	}
 	port := ln.Addr().(*net.TCPAddr).Port
-	s.serverName += fmt.Sprintf(":%d", port)
+	s.serverName = spec.ServerName(fmt.Sprintf("%s:%d", s.serverName, port))
 	s.listening = true
 
 	go func() {
@@ -647,7 +657,7 @@ func (f *basicKeyFetcher) FetchKeys(
 ) {
 	result := make(map[gomatrixserverlib.PublicKeyLookupRequest]gomatrixserverlib.PublicKeyLookupResult, len(requests))
 	for req := range requests {
-		if string(req.ServerName) == f.srv.serverName && req.KeyID == f.srv.KeyID {
+		if req.ServerName == f.srv.serverName && req.KeyID == f.srv.KeyID {
 			publicKey := f.srv.Priv.Public().(ed25519.PublicKey)
 			result[req] = gomatrixserverlib.PublicKeyLookupResult{
 				ValidUntilTS: spec.AsTimestamp(time.Now().Add(24 * time.Hour)),