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.

Sign up for GitHub

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

Jump to bottom

Set the default port for the dynamic proxy to 443 #831

Merged
merged 2 commits into from
Jan 28, 2025
Merged

Set the default port for the dynamic proxy to 443 #831

merged 2 commits into from
Jan 28, 2025

Conversation

njegosrailic
Copy link
Contributor

What

See title.

Why

The majority of workflows use 443 as the default. This doesn't conflict with anything because traffic has to be explicitly routed through it.

@njegosrailic njegosrailic marked this pull request as ready for review January 22, 2025 11:18
@njegosrailic njegosrailic requested a review from a team as a code owner January 22, 2025 11:18
chainchad
chainchad approved these changes Jan 23, 2025
View reviewed changes
Copy link
Contributor

@chainchad chainchad left a comment

Choose a reason for hiding this comment

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

I imagine we're launching the container as root if we need it to be available on the host at 443?

@njegosrailic
Copy link
Contributor Author

I imagine we're launching the container as root if we need it to be available on the host at 443?

I think so. Do you see any downside?

@njegosrailic njegosrailic merged commit bba3537 into main Jan 28, 2025
11 checks passed
@njegosrailic njegosrailic deleted the dynamic-proxy-port branch January 28, 2025 14:11
@app-token-issuer-releng-renovate app-token-issuer-releng-renovate bot mentioned this pull request Jan 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chainchad chainchad chainchad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@njegosrailic @chainchad