Allows hostname instead of ip for streams #1036
Conversation
|
I also feel like my changes did not cause the CI to fail or am i reading the Jenkins Output wrong? |
|
FYI all PR's are on hold while Alpine ARM builds are being fixed. I will retrigger and assess this PR afterwards. |
|
Docker Image for build 2 is available on DockerHub as |
|
Not sure if you're still working on this? The PR is still in Draft status. |
|
Hi! Sorry if that was confusing. I made this PR a draft, because i was not sure if it is the right way. I'll change it now. |
|
I would love for this to be looked at and merged! We're currently in need of this exact feature. |
|
I need this! |
|
Functionally the PR works as expected, just some cleanup comments. As for your other (PP)PS's: Not going to bother with that in v2. Definitely more easily doable in v3 codebase instead. |
- Empty function removed - Placeholder and Maxlength restored - Validation improved - Typo fixed
|
This is an automated message from CI: Docker Image for build 3 is available on DockerHub as Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes. |
|
Feedback has been incorporated. Especially like your suggestion regarding validation! |
|
Awesome :) thanks for the contribution! |
|
Gladly! Albeit i had some issues navigating the code base, fixing this issue was pretty fast. |
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.