Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Federation support #776

Merged
merged 24 commits into from
Aug 8, 2024
Merged

Federation support #776

merged 24 commits into from
Aug 8, 2024

Conversation

fancycode
Copy link
Member

@coveralls
Copy link

coveralls commented Jul 18, 2024

Pull Request Test Coverage Report for Build 10263387860

Details

  • 659 of 882 (74.72%) changed or added relevant lines in 6 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+1.0%) to 58.364%

Changes Missing Coverage Covered Lines Changed/Added Lines %
clientsession.go 25 32 78.13%
api_signaling.go 62 79 78.48%
hub.go 67 125 53.6%
federation.go 500 641 78.0%
Files with Coverage Reduction New Missed Lines %
room.go 1 84.29%
grpc_remote_client.go 2 75.57%
Totals Coverage Status
Change from base Build 10262210080: 1.0%
Covered Lines: 10495
Relevant Lines: 17982

💛 - Coveralls

federation.go Outdated Show resolved Hide resolved
@fancycode fancycode force-pushed the federation branch 2 times, most recently from fef4ab6 to f064471 Compare July 24, 2024 13:36
@fancycode fancycode marked this pull request as ready for review July 24, 2024 14:35
@fancycode
Copy link
Member Author

@danxuliu this is ready for merging from my side, could you please do a test with your Talk changes and see if everything is working as expected? Thanks!

@SystemKeeper
Copy link
Contributor

I played around with it today, what I noticed:

  • When I first join the signaling controller without federated information
  • Then join the signaling controller with federated information (so still the same conversation)

The second join kills the first session on the Nextcloud server, so joining a call then fails, because I don't have an active session on the nextcloud server.
While I can see that it might be intended this way, it was totally unexpected to me. Is it intended or should the second join just "overwrite" the first one (without killing the session in Nextcloud?)?

The second thing I noticed, when I join the same room twice fast (this time both requests do have federated information attached), I sometimes got "no_such_room" back and only the second join worked.

@SystemKeeper
Copy link
Contributor

  • When I first join the signaling controller without federated information
  • Then join the signaling controller with federated information (so still the same conversation)

The second join kills the first session on the Nextcloud server, so joining a call then fails, because I don't have an active session on the nextcloud server.

It seems unrelated to the fact with/without federated information, when I try to join the same server again, a leave is sent to the nextcloud instance and then my join request fails and I get brute-force protected on top. While I can work around some cases, we still can end up in the case where a duplicated join happens (that's why we added already_joined back then). Is there a chance to make that more resilient?

@fancycode
Copy link
Member Author

@SystemKeeper duplicate federated join requests are now handled in 5d4efe8

@SystemKeeper
Copy link
Contributor

@SystemKeeper duplicate federated join requests are now handled in 5d4efe8

Tested and works, thanks!

@fancycode fancycode force-pushed the federation branch 2 times, most recently from 4387080 to 3582b05 Compare July 25, 2024 09:37
@SystemKeeper

This comment was marked as off-topic.

@fancycode
Copy link
Member Author

The expiration honors any caching headers set (since #780) instead of using a hardcoded cache duration of one hour. The default is one minute if no caching headers are returned.

Could you please check the response when you query the capabilities manually?

curl -ik -H "OCS-ApiRequest: true" -H "Accept: application/json" https://server.domain.tld/ocs/v2.php/cloud/capabilities

Anyway, if something is not working correctly, please open a new issue as this is unrelated to the federation changes.

@SystemKeeper

This comment was marked as off-topic.

@fancycode
Copy link
Member Author

@SystemKeeper FYI, rebased on latest master containing #783 in case you want to do some more testing.

@fancycode fancycode merged commit b4dbbaa into master Aug 8, 2024
22 checks passed
@fancycode fancycode deleted the federation branch August 8, 2024 07:01
mwalbeck pushed a commit to mwalbeck/docker-nextcloud-spreed-signaling that referenced this pull request Sep 13, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [strukturag/nextcloud-spreed-signaling](https://github.com/strukturag/nextcloud-spreed-signaling) | major | `v1.3.2` -> `v2.0.0` |

---

### Release Notes

<details>
<summary>strukturag/nextcloud-spreed-signaling (strukturag/nextcloud-spreed-signaling)</summary>

### [`v2.0.0`](https://github.com/strukturag/nextcloud-spreed-signaling/releases/tag/v2.0.0)

[Compare Source](strukturag/nextcloud-spreed-signaling@v1.3.2...v2.0.0)

##### Added

-   Federation support  [#&#8203;776](strukturag/nextcloud-spreed-signaling#776)
-   CI: Add job to update generated files.  [#&#8203;790](strukturag/nextcloud-spreed-signaling#790)
-   Expose backend session limits through prometheus stats.  [#&#8203;792](strukturag/nextcloud-spreed-signaling#792)
-   CI: Test with Golang 1.23  [#&#8203;805](strukturag/nextcloud-spreed-signaling#805)

##### Changed

-   Keep generated files in the repository.  [#&#8203;781](strukturag/nextcloud-spreed-signaling#781)
-   Improve caching when fetching capabilities.  [#&#8203;780](strukturag/nextcloud-spreed-signaling#780)
-   Enforce a minimum duration to cache capabilities.  [#&#8203;783](strukturag/nextcloud-spreed-signaling#783)
-   docs: Use the latest LTS of Ubuntu and Python 3.12.  [#&#8203;791](strukturag/nextcloud-spreed-signaling#791)
-   CI: Push generated code from service account.  [#&#8203;793](strukturag/nextcloud-spreed-signaling#793)
-   CI: Only build code if token exists (i.e. with Dependabot).  [#&#8203;794](strukturag/nextcloud-spreed-signaling#794)
-   CI: Always do a full build of generated files.  [#&#8203;795](strukturag/nextcloud-spreed-signaling#795)
-   Remove compatibility code for Go < 1.21.  [#&#8203;806](strukturag/nextcloud-spreed-signaling#806)
-   Send ping requests to local instance for federated sessions.  [#&#8203;808](strukturag/nextcloud-spreed-signaling#808)

##### Dependencies

-   Bump sphinx from 7.3.7 to 7.4.4 in /docs  [#&#8203;773](strukturag/nextcloud-spreed-signaling#773)
-   Bump google.golang.org/grpc from 1.64.0 to 1.65.0  [#&#8203;769](strukturag/nextcloud-spreed-signaling#769)
-   Bump sphinx from 7.4.4 to 7.4.5 in /docs  [#&#8203;774](strukturag/nextcloud-spreed-signaling#774)
-   Bump github.com/nats-io/nats-server/v2 from 2.10.17 to 2.10.18  [#&#8203;775](strukturag/nextcloud-spreed-signaling#775)
-   Bump sphinx from 7.4.5 to 7.4.6 in /docs  [#&#8203;777](strukturag/nextcloud-spreed-signaling#777)
-   Bump sphinx from 7.4.6 to 7.4.7 in /docs  [#&#8203;779](strukturag/nextcloud-spreed-signaling#779)
-   Bump the etcd group with 4 updates  [#&#8203;778](strukturag/nextcloud-spreed-signaling#778)
-   Bump golangci/golangci-lint-action from 6.0.1 to 6.1.0  [#&#8203;788](strukturag/nextcloud-spreed-signaling#788)
-   Bump google.golang.org/grpc/cmd/protoc-gen-go-grpc from 1.4.0 to 1.5.1  [#&#8203;784](strukturag/nextcloud-spreed-signaling#784)
-   Bump markdown from 3.6 to 3.7 in /docs  [#&#8203;801](strukturag/nextcloud-spreed-signaling#801)
-   Bump github.com/prometheus/client_golang from 1.19.1 to 1.20.2  [#&#8203;803](strukturag/nextcloud-spreed-signaling#803)
-   Bump golang from 1.22-alpine to 1.23-alpine in /docker/server  [#&#8203;798](strukturag/nextcloud-spreed-signaling#798)
-   Bump golang from 1.22-alpine to 1.23-alpine in /docker/proxy  [#&#8203;799](strukturag/nextcloud-spreed-signaling#799)
-   Bump google.golang.org/grpc from 1.65.0 to 1.66.0  [#&#8203;810](strukturag/nextcloud-spreed-signaling#810)
-   Bump github.com/nats-io/nats-server/v2 from 2.10.18 to 2.10.19  [#&#8203;809](strukturag/nextcloud-spreed-signaling#809)
-   Bump github.com/nats-io/nats.go from 1.36.0 to 1.37.0  [#&#8203;797](strukturag/nextcloud-spreed-signaling#797)
-   Bump mkdocs from 1.6.0 to 1.6.1 in /docs  [#&#8203;812](strukturag/nextcloud-spreed-signaling#812)
-   Bump github.com/nats-io/nats-server/v2 from 2.10.19 to 2.10.20  [#&#8203;813](strukturag/nextcloud-spreed-signaling#813)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4zIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMyIsInRhcmdldEJyYW5jaCI6Im1hc3RlciIsImxhYmVscyI6W119-->

Reviewed-on: https://git.walbeck.it/walbeck-it/docker-nextcloud-spreed-signaling/pulls/504
Co-authored-by: renovate-bot <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants