Skip to content

[CLD-7443] Allow rtcd service to be exposed on a privileged port#457

Open
streamer45 wants to merge 3 commits intomasterfrom
CLD-7443
Open

[CLD-7443] Allow rtcd service to be exposed on a privileged port#457
streamer45 wants to merge 3 commits intomasterfrom
CLD-7443

Conversation

@streamer45
Copy link
Copy Markdown
Contributor

Summary

PR proposes a possible solution to the issue we are facing when trying to have rtcd listen on a privileged port (lower than 1024).

So far we've tested a couple of workarounds (full context at https://hub.mattermost.com/private-core/pl/5ubornsmpfnkuyxnmyixgfpich):

  1. Setting the NET_BIND_SERVICE capability in the container security context.
  2. Setting the net.ipv4.ip_unprivileged_port_start sysctl in the pod security context.

Unfortunately, none of the above is working, possibly because the image was meant to be run using an unprivileged user at all times.

So I was thinking we could avoid the problem altogether by using the k8s service as designed and let it forward the traffic from privileged (node level) to unprivileged (container level). The only caveat is that to do so we'll need to disable host networking.

This is the bit I will need some guidance on because I don't have a full understanding of other potential side effects of doing so (e.g. DNS policy?).

Note
To make this work properly in our deployment we'll also need some changes coming in v0.15.1 (mattermost/rtcd#140).

Ticket Link

https://mattermost.atlassian.net/browse/CLD-7443

@streamer45 streamer45 added 1: Dev Review Requires review by a core commiter Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) labels May 17, 2024
@streamer45 streamer45 self-assigned this May 17, 2024
{{- end }}
dnsPolicy: ClusterFirstWithHostNet
hostNetwork: true
hostNetwork: false
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we decide to do this, I would make this a variable that can be controlled from the values file since there are still cases where it would be valuable to use host networking since it simplifies the generation of local candidates.

@stafot
Copy link
Copy Markdown
Contributor

stafot commented Jun 16, 2025

@streamer45 Is this still something we consider to merge?

@streamer45
Copy link
Copy Markdown
Contributor Author

@streamer45 Is this still something we consider to merge?

@stafot Ideally yes. We still have some clients failing to join calls on Community/Hub occasionally, likely because 8443 is blocked on their side. This could help.

@stafot
Copy link
Copy Markdown
Contributor

stafot commented Jun 16, 2025

@streamer45 If we want to maerge let's do variable hostNetwork and have true as default value.

@streamer45
Copy link
Copy Markdown
Contributor Author

@stafot Sounds good. I'd also like to try this in our test environment before merging since I haven't yet validated this in a real deployment.

@mattermost-build
Copy link
Copy Markdown
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1: Dev Review Requires review by a core commiter Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) Lifecycle/1:stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants