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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

streamer45
Copy link
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
@@ -30,7 +30,7 @@ spec:
{{- toYaml . | nindent 8 }}
{{- end }}
dnsPolicy: ClusterFirstWithHostNet
hostNetwork: true
hostNetwork: false
Copy link
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.

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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant