From baf118b08cee29cf7435a8a871a7aab423baf779 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Fri, 3 Feb 2023 13:42:35 +0100 Subject: [PATCH] Add Sytest/Complement coverage to scheduled runs (#2962) This adds Sytest and Complement coverage reporting to the nightly scheduled CI runs. Fixes a few API mode related issues as well, since we seemingly never really ran them with Complement. Also fixes a bug related to device list changes: When we pass in an empty `newlyLeftRooms` slice, we got a list of all currently joined rooms with the corresponding members. When we then got the `newlyJoinedRooms`, we wouldn't update the `changed` slice, because we already got the user from the `newlyLeftRooms` query. This is fixed by simply ignoring empty `newlyLeftRooms`. --- .github/workflows/dendrite.yml | 3 +- .github/workflows/schedules.yaml | 176 ++++++++++++++++++- build/scripts/complement-cmd.sh | 2 +- clientapi/routing/leaveroom.go | 10 ++ clientapi/routing/register.go | 12 ++ clientapi/routing/routing.go | 23 ++- roomserver/internal/perform/perform_leave.go | 3 +- syncapi/internal/keychange.go | 64 +++---- 8 files changed, 253 insertions(+), 40 deletions(-) diff --git a/.github/workflows/dendrite.yml b/.github/workflows/dendrite.yml index 1de39850..2dc5f74c 100644 --- a/.github/workflows/dendrite.yml +++ b/.github/workflows/dendrite.yml @@ -460,7 +460,8 @@ jobs: name: Run Complement Tests env: COMPLEMENT_BASE_IMAGE: complement-dendrite:${{ matrix.postgres }}${{ matrix.api }}${{ matrix.cgo }} - API: ${{ matrix.api && 1 }} + COMPLEMENT_DENDRITE_API: ${{ matrix.api && 1 }} + COMPLEMENT_SHARE_ENV_PREFIX: COMPLEMENT_DENDRITE_ working-directory: complement integration-tests-done: diff --git a/.github/workflows/schedules.yaml b/.github/workflows/schedules.yaml index 5636c4cf..fa2304c1 100644 --- a/.github/workflows/schedules.yaml +++ b/.github/workflows/schedules.yaml @@ -19,11 +19,18 @@ jobs: fail-fast: false matrix: include: - - label: SQLite + - label: SQLite native - - label: SQLite, full HTTP APIs + - label: SQLite Cgo + cgo: 1 + + - label: SQLite native, full HTTP APIs api: full-http + - label: SQLite Cgo, full HTTP APIs + api: full-http + cgo: 1 + - label: PostgreSQL postgres: postgres @@ -41,6 +48,7 @@ jobs: API: ${{ matrix.api && 1 }} SYTEST_BRANCH: ${{ github.head_ref }} RACE_DETECTION: 1 + COVER: 1 steps: - uses: actions/checkout@v3 - uses: actions/cache@v3 @@ -73,7 +81,171 @@ jobs: path: | /logs/results.tap /logs/**/*.log* + + sytest-coverage: + timeout-minutes: 5 + name: "Sytest Coverage" + runs-on: ubuntu-latest + needs: sytest # only run once Sytest is done + if: ${{ always() }} + steps: + - uses: actions/checkout@v3 + - name: Install Go + uses: actions/setup-go@v3 + with: + go-version: '>=1.19.0' + cache: true + - name: Download all artifacts + uses: actions/download-artifact@v3 + - name: Install gocovmerge + run: go install github.com/wadey/gocovmerge@latest + - name: Run gocovmerge + run: | + find -name 'integrationcover.log' -printf '"%p"\n' | xargs gocovmerge | grep -Ev 'relayapi|setup/mscs|api_trace' > sytest.cov + go tool cover -func=sytest.cov + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v3 + with: + files: ./sytest.cov + flags: sytest + fail_ci_if_error: true + + # run Complement + complement: + name: "Complement (${{ matrix.label }})" + timeout-minutes: 60 + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - label: SQLite native + cgo: 0 + + - label: SQLite Cgo + cgo: 1 + + - label: SQLite native, full HTTP APIs + api: full-http + cgo: 0 + + - label: SQLite Cgo, full HTTP APIs + api: full-http + cgo: 1 + + - label: PostgreSQL + postgres: Postgres + cgo: 0 + + - label: PostgreSQL, full HTTP APIs + postgres: Postgres + api: full-http + cgo: 0 + steps: + # Env vars are set file a file given by $GITHUB_PATH. We need both Go 1.17 and GOPATH on env to run Complement. + # See https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-system-path + - name: "Set Go Version" + run: | + echo "$GOROOT_1_17_X64/bin" >> $GITHUB_PATH + echo "~/go/bin" >> $GITHUB_PATH + - name: "Install Complement Dependencies" + # We don't need to install Go because it is included on the Ubuntu 20.04 image: + # See https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-Readme.md specifically GOROOT_1_17_X64 + run: | + sudo apt-get update && sudo apt-get install -y libolm3 libolm-dev + go get -v github.com/gotesttools/gotestfmt/v2/cmd/gotestfmt@latest + - name: Run actions/checkout@v3 for dendrite + uses: actions/checkout@v3 + with: + path: dendrite + + # Attempt to check out the same branch of Complement as the PR. If it + # doesn't exist, fallback to main. + - name: Checkout complement + shell: bash + run: | + mkdir -p complement + # Attempt to use the version of complement which best matches the current + # build. Depending on whether this is a PR or release, etc. we need to + # use different fallbacks. + # + # 1. First check if there's a similarly named branch (GITHUB_HEAD_REF + # for pull requests, otherwise GITHUB_REF). + # 2. Attempt to use the base branch, e.g. when merging into release-vX.Y + # (GITHUB_BASE_REF for pull requests). + # 3. Use the default complement branch ("master"). + for BRANCH_NAME in "$GITHUB_HEAD_REF" "$GITHUB_BASE_REF" "${GITHUB_REF#refs/heads/}" "master"; do + # Skip empty branch names and merge commits. + if [[ -z "$BRANCH_NAME" || $BRANCH_NAME =~ ^refs/pull/.* ]]; then + continue + fi + (wget -O - "https://github.com/matrix-org/complement/archive/$BRANCH_NAME.tar.gz" | tar -xz --strip-components=1 -C complement) && break + done + # Build initial Dendrite image + - run: docker build --build-arg=CGO=${{ matrix.cgo }} -t complement-dendrite:${{ matrix.postgres }}${{ matrix.api }}${{ matrix.cgo }} -f build/scripts/Complement${{ matrix.postgres }}.Dockerfile . + working-directory: dendrite + env: + DOCKER_BUILDKIT: 1 + + - name: Create post test script + run: | + cat < /tmp/posttest.sh + #!/bin/bash + mkdir -p /tmp/Complement/logs/\$2/\$1/ + docker cp \$1:/dendrite/complementcover.log /tmp/Complement/logs/\$2/\$1/ + EOF + chmod +x /tmp/posttest.sh + # Run Complement + - run: | + set -o pipefail && + go test -v -json -tags dendrite_blacklist ./tests/... 2>&1 | gotestfmt + shell: bash + name: Run Complement Tests + env: + COMPLEMENT_BASE_IMAGE: complement-dendrite:${{ matrix.postgres }}${{ matrix.api }}${{ matrix.cgo }} + COMPLEMENT_DENDRITE_API: ${{ matrix.api && 1 }} + COMPLEMENT_SHARE_ENV_PREFIX: COMPLEMENT_DENDRITE_ + COMPLEMENT_DENDRITE_COVER: 1 + COMPLEMENT_POST_TEST_SCRIPT: /tmp/posttest.sh + working-directory: complement + + - name: Upload Complement logs + uses: actions/upload-artifact@v2 + if: ${{ always() }} + with: + name: Complement Logs - (Dendrite, ${{ join(matrix.*, ', ') }}) + path: | + /tmp/Complement/**/complementcover.log + + complement-coverage: + timeout-minutes: 5 + name: "Complement Coverage" + runs-on: ubuntu-latest + needs: complement # only run once Complement is done + if: ${{ always() }} + steps: + - uses: actions/checkout@v3 + - name: Install Go + uses: actions/setup-go@v3 + with: + go-version: '>=1.19.0' + cache: true + - name: Download all artifacts + uses: actions/download-artifact@v3 + - name: Install gocovmerge + run: go install github.com/wadey/gocovmerge@latest + - name: Run gocovmerge + run: | + find -name 'complementcover.log' -printf '"%p"\n' | xargs gocovmerge | grep -Ev 'relayapi|setup/mscs|api_trace' > complement.cov + go tool cover -func=complement.cov + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v3 + with: + files: ./complement.cov + flags: complement + fail_ci_if_error: true + element_web: timeout-minutes: 120 runs-on: ubuntu-latest diff --git a/build/scripts/complement-cmd.sh b/build/scripts/complement-cmd.sh index 061bd18e..def091e2 100755 --- a/build/scripts/complement-cmd.sh +++ b/build/scripts/complement-cmd.sh @@ -10,7 +10,7 @@ if [[ "${COVER}" -eq 1 ]]; then --tls-key server.key \ --config dendrite.yaml \ -api=${API:-0} \ - --test.coverprofile=integrationcover.log + --test.coverprofile=complementcover.log else echo "Not running with coverage" exec /dendrite/dendrite-monolith-server \ diff --git a/clientapi/routing/leaveroom.go b/clientapi/routing/leaveroom.go index a7166185..86414afc 100644 --- a/clientapi/routing/leaveroom.go +++ b/clientapi/routing/leaveroom.go @@ -18,6 +18,7 @@ import ( "net/http" "github.com/matrix-org/dendrite/clientapi/jsonerror" + "github.com/matrix-org/dendrite/internal/httputil" roomserverAPI "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/userapi/api" "github.com/matrix-org/util" @@ -44,6 +45,15 @@ func LeaveRoomByID( JSON: jsonerror.LeaveServerNoticeError(), } } + switch e := err.(type) { + case httputil.InternalAPIError: + if e.Message == jsonerror.LeaveServerNoticeError().Error() { + return util.JSONResponse{ + Code: http.StatusForbidden, + JSON: jsonerror.LeaveServerNoticeError(), + } + } + } return util.JSONResponse{ Code: http.StatusBadRequest, JSON: jsonerror.Unknown(err.Error()), diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index ff6a0900..be2b192b 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -31,6 +31,8 @@ import ( "time" "github.com/matrix-org/dendrite/internal" + internalHTTPUtil "github.com/matrix-org/dendrite/internal/httputil" + "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/tidwall/gjson" "github.com/matrix-org/dendrite/internal/eventutil" @@ -859,6 +861,16 @@ func completeRegistration( JSON: jsonerror.UserInUse("Desired user ID is already taken."), } } + switch e := err.(type) { + case internalHTTPUtil.InternalAPIError: + conflictErr := &userapi.ErrorConflict{Message: sqlutil.ErrUserExists.Error()} + if e.Message == conflictErr.Error() { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.UserInUse("Desired user ID is already taken."), + } + } + } return util.JSONResponse{ Code: http.StatusInternalServerError, JSON: jsonerror.Unknown("failed to create account: " + err.Error()), diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 93f6ea90..66610c0a 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -18,6 +18,7 @@ import ( "context" "net/http" "strings" + "sync" "github.com/gorilla/mux" "github.com/matrix-org/dendrite/setup/base" @@ -198,18 +199,24 @@ func Setup( // server notifications if cfg.Matrix.ServerNotices.Enabled { logrus.Info("Enabling server notices at /_synapse/admin/v1/send_server_notice") - serverNotificationSender, err := getSenderDevice(context.Background(), rsAPI, userAPI, cfg) - if err != nil { - logrus.WithError(err).Fatal("unable to get account for sending sending server notices") - } + var serverNotificationSender *userapi.Device + var err error + notificationSenderOnce := &sync.Once{} synapseAdminRouter.Handle("/admin/v1/send_server_notice/{txnID}", httputil.MakeAuthAPI("send_server_notice", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + notificationSenderOnce.Do(func() { + serverNotificationSender, err = getSenderDevice(context.Background(), rsAPI, userAPI, cfg) + if err != nil { + logrus.WithError(err).Fatal("unable to get account for sending sending server notices") + } + }) // not specced, but ensure we're rate limiting requests to this endpoint if r := rateLimits.Limit(req, device); r != nil { return *r } - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + var vars map[string]string + vars, err = httputil.URLDecodeMapValues(mux.Vars(req)) if err != nil { return util.ErrorResponse(err) } @@ -225,6 +232,12 @@ func Setup( synapseAdminRouter.Handle("/admin/v1/send_server_notice", httputil.MakeAuthAPI("send_server_notice", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + notificationSenderOnce.Do(func() { + serverNotificationSender, err = getSenderDevice(context.Background(), rsAPI, userAPI, cfg) + if err != nil { + logrus.WithError(err).Fatal("unable to get account for sending sending server notices") + } + }) // not specced, but ensure we're rate limiting requests to this endpoint if r := rateLimits.Limit(req, device); r != nil { return *r diff --git a/roomserver/internal/perform/perform_leave.go b/roomserver/internal/perform/perform_leave.go index fa998e3e..86f1dfae 100644 --- a/roomserver/internal/perform/perform_leave.go +++ b/roomserver/internal/perform/perform_leave.go @@ -20,6 +20,7 @@ import ( "fmt" "strings" + "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/gomatrix" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" @@ -109,7 +110,7 @@ func (r *Leaver) performLeaveRoomByID( // mimic the returned values from Synapse res.Message = "You cannot reject this invite" res.Code = 403 - return nil, fmt.Errorf("You cannot reject this invite") + return nil, jsonerror.LeaveServerNoticeError() } } } diff --git a/syncapi/internal/keychange.go b/syncapi/internal/keychange.go index 3d6b2a7f..4867f7d9 100644 --- a/syncapi/internal/keychange.go +++ b/syncapi/internal/keychange.go @@ -144,38 +144,42 @@ func TrackChangedUsers( // - Loop set of users and decrement by 1 for each user in newly left room. // - If count=0 then they share no more rooms so inform BOTH parties of this via 'left'=[...] in /sync. var queryRes roomserverAPI.QuerySharedUsersResponse - err = rsAPI.QuerySharedUsers(ctx, &roomserverAPI.QuerySharedUsersRequest{ - UserID: userID, - IncludeRoomIDs: newlyLeftRooms, - }, &queryRes) - if err != nil { - return nil, nil, err - } var stateRes roomserverAPI.QueryBulkStateContentResponse - err = rsAPI.QueryBulkStateContent(ctx, &roomserverAPI.QueryBulkStateContentRequest{ - RoomIDs: newlyLeftRooms, - StateTuples: []gomatrixserverlib.StateKeyTuple{ - { - EventType: gomatrixserverlib.MRoomMember, - StateKey: "*", - }, - }, - AllowWildcards: true, - }, &stateRes) - if err != nil { - return nil, nil, err - } - for _, state := range stateRes.Rooms { - for tuple, membership := range state { - if membership != gomatrixserverlib.Join { - continue - } - queryRes.UserIDsToCount[tuple.StateKey]-- + if len(newlyLeftRooms) > 0 { + err = rsAPI.QuerySharedUsers(ctx, &roomserverAPI.QuerySharedUsersRequest{ + UserID: userID, + IncludeRoomIDs: newlyLeftRooms, + }, &queryRes) + if err != nil { + return nil, nil, err } - } - for userID, count := range queryRes.UserIDsToCount { - if count <= 0 { - left = append(left, userID) // left is returned + + err = rsAPI.QueryBulkStateContent(ctx, &roomserverAPI.QueryBulkStateContentRequest{ + RoomIDs: newlyLeftRooms, + StateTuples: []gomatrixserverlib.StateKeyTuple{ + { + EventType: gomatrixserverlib.MRoomMember, + StateKey: "*", + }, + }, + AllowWildcards: true, + }, &stateRes) + if err != nil { + return nil, nil, err + } + for _, state := range stateRes.Rooms { + for tuple, membership := range state { + if membership != gomatrixserverlib.Join { + continue + } + queryRes.UserIDsToCount[tuple.StateKey]-- + } + } + + for userID, count := range queryRes.UserIDsToCount { + if count <= 0 { + left = append(left, userID) // left is returned + } } }