Skip to content

Conversation

@peixian
Copy link
Member

@peixian peixian commented Feb 14, 2025

  • The stats port 8199 is open by default, and it should probably be closed.

  • Media ports are not open by default on the helm chart, fixes that

  • Changes the service type to an external load balancer type so that a default helm deployment creates an external IP.

  • Also fixes the docker build pipeline for PRs to only build, and not push

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 14, 2025
@peixian peixian force-pushed the peixian/add-external-loadbalancer branch 2 times, most recently from 549a74f to 4ce8417 Compare February 14, 2025 19:25
@peixian peixian force-pushed the peixian/add-external-loadbalancer branch from 4ce8417 to 515e365 Compare February 14, 2025 19:28
# http_proxy_port: 8080
# https_proxy_port: 8443
# jabber_proxy_port: 8222
# media_proxy_port: 7777
Copy link
Contributor

Choose a reason for hiding this comment

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

We need this one as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, these are left as examples for other users

{{- if .Values.service.media_proxy_port }}
- port: {{ .Values.service.media_proxy_port }}
targetPort: 7777
protocl: TCP
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm.. typo? How is YAML passing?

@peixian peixian force-pushed the peixian/add-external-loadbalancer branch from 25486ea to b4fe2a4 Compare February 19, 2025 22:59
{{- if .Values.service.media_port }}
- port: {{ .Values.service.media_port }}
targetPort: 587
protocl: TCP
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too.

Copy link
Contributor

@eozturk1 eozturk1 left a comment

Choose a reason for hiding this comment

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

LGTM assuming you have tested this version and confirmed proxy works!

@@ -0,0 +1,38 @@
name: Docker Image Build for PRs

# Run on every push to main + weekly, Sunday @ midnight
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the comment accurate?

@peixian peixian merged commit fc0bb96 into WhatsApp:main Feb 20, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants