mirror of
https://github.com/NginxProxyManager/nginx-proxy-manager.git
synced 2026-04-25 09:25:55 +03:00
[PR #1036] [MERGED] Allows hostname instead of ip for streams #3341
Labels
No labels
awaiting feedback
bug
cannot reproduce
dns provider request
duplicate
enhancement
enhancement
enhancement
good first issue
help wanted
invalid
need more info
no certbot plugin available
product-support
pull-request
question
stale
troll
upstream issue
v2
v2
v2
v3
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/nginx-proxy-manager-NginxProxyManager#3341
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
📋 Pull Request Information
Original PR: https://github.com/NginxProxyManager/nginx-proxy-manager/pull/1036
Author: @BjoernAkAManf
Created: 4/23/2021
Status: ✅ Merged
Merged: 8/16/2021
Merged by: @jc21
Base:
develop← Head:master📝 Commits (3)
1a64d44Merge pull request #955 from jc21/develop389fd15allows hostname instead of ip for streamsba7bb57Incorporate feedback📊 Changes
10 files changed (+71 additions, -25 deletions)
View changed files
➕
backend/migrations/20210423103500_stream_domain.js(+40 -0)📝
backend/schema/endpoints/streams.json(+21 -10)📝
backend/templates/stream.conf(+2 -2)📝
docker/rootfs/etc/services.d/frontend/run(+1 -0)📝
docker/rootfs/etc/services.d/manager/run(+1 -0)📝
frontend/js/app/nginx/stream/form.ejs(+2 -2)📝
frontend/js/app/nginx/stream/form.js(+1 -8)📝
frontend/js/app/nginx/stream/list/item.ejs(+1 -1)📝
frontend/js/i18n/messages.json(+1 -1)📝
frontend/js/models/stream.js(+1 -1)📄 Description
I see no Benefit to reduce streams to ip addresses. In particular if running NPM inside docker containers.
This causes a duality in management between proxied http hosts and streamed protocols (e.g. ssh).
Once merged it fixes #44 .
Note: This implementation is a bit rough around the edges. In particular it currently allows input such as host name "example.com/meow" and Port "22". This is obviously not ideal.
As i've been struggeling with the source code a little (e.g. getting the dev environment to work). I wanted to get a bit feedback about this change first. As far as i can see it we should just allow any domain (example.com, test.example.com) and disallow usage of the path component for this to work correctly right? (example.com/meow:22 is obviously wrong, but even example.com:22/meow would not mean anything).
PS: Any reason for the files "fonts" and "images"? Right now theres a need to manually create a sym link to the location specified in that file. Would'nt it make sense to just replace the references to that file in the source code itself?
PPS: I noticed there is no real information about contributing to the Project. Would you mind adding some information to the readme or a dedicated file?
PPPS: Would you be interested in a PR decoupling the authentication of NPM? For example it would be a great feature if the Admin UI could be secured using OIDC itself. The only information to OIDC i could find in the repository right now is unfortunately only for protecting proxied services themselves.
Nethertheless thanks for the great product.
🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.