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

Feature: Add two new headers to proxy.conf #1614

Merged
merged 5 commits into from
Jan 2, 2022
Merged

Feature: Add two new headers to proxy.conf #1614

merged 5 commits into from
Jan 2, 2022

Conversation

the1ts
Copy link
Contributor

@the1ts the1ts commented Nov 29, 2021

Fixes #1609. Adding both X-Forwarded-Host and X-Forwarded-Port, this is vital for some services behind a proxy (used to allow creation of absolute links in html). I've had to include at least the Host version in the past for Jenkins and Nexus.
Been running locally for 24 hours, does not appear to break any of my 15+ services currently running behind NPM would allow people to host those problem services without the need for advanced configuration.

Fixes #1609. Adding both  X-Forwarded-Host  and X-Forwarded-Port, this is vital for some services behind a proxy (used to allow creation of absolute links in html). I've had to include at least the Host version in the past for jenkins and nexus.
Been running locally for 24 hours, does not appear to break any of my 15+ services currently running behind NPM would allow people to host those services without the need for advanced configuration
@the1ts
Copy link
Contributor Author

the1ts commented Dec 7, 2021

This appears to be failing due to lack of python during the sqlite3 build in the backed.

[2021-11-29T13:53:52.222Z] make: Entering directory '/app/node_modules/sqlite3/build'
[2021-11-29T13:53:52.222Z]   ACTION deps_sqlite3_gyp_action_before_build_target_unpack_sqlite_dep Release/obj/gen/sqlite-autoconf-3310100/sqlite3.c
[2021-11-29T13:53:52.222Z] /bin/sh: 1: python: not found

This appears to be looking for python2 (/usr/bin/python) rather than python3 (/usr/bin/python3), anything changed in that regard in the CI system lately?

@the1ts
Copy link
Contributor Author

the1ts commented Dec 15, 2021

I think this is because node:latest is now based on Debian bullseye (SHA1 hashes for latest and current-bullseye match). Bullseye by default no longer has python 2 installed. So either we apt-get update ; apt-get install -y python2.7 as part of that build docker run or use node:current-buster as we did before node:latest moved.
If we do option 2 we are waiting until node-gyp and dependencies move to python 3 and hoping node support buster until that happens.

@operinko
Copy link

operinko commented Dec 15, 2021

Wasn't the EOL for Python 2.7 in 2020 already? Would make absolutely no sense to use that going forwards.
Recent versions of node-gyp already require Python v3.6, v3.7, v3.8, or v3.9.

@the1ts
Copy link
Contributor Author

the1ts commented Dec 15, 2021

If node-gyp does support python 3 , then we need to fix the bullseye alternatives to allow python 3 to be used for /usr/bin/python so the build scripts will find python in their path.
From a new minimal bullseye install.

# python
-bash: python: command not found
# update-alternatives --install /usr/bin/python python /usr/bin/python3 2
update-alternatives: using /usr/bin/python3 to provide /usr/bin/python (python) in auto mode
# python --version
Python 3.9.2

@jc21
Copy link
Member

jc21 commented Dec 27, 2021

You might have to rebase on the develop branch as this python fix has been added

@jc21
Copy link
Member

jc21 commented Dec 28, 2021

This is an automated message from CI:

Docker Image for build 3 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1614

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

@jc21 jc21 merged commit b9ef11e into NginxProxyManager:develop Jan 2, 2022
@the1ts the1ts deleted the feature/proxy-header-additions branch January 2, 2022 08:36
@jc21
Copy link
Member

jc21 commented Jan 10, 2022

I have to revert this PR due to #1717. Perhaps there is another way around this.

Re-reading the PR comment above, I use jenkins behind NPM and haven't had a problem with it in the past.

jc21 added a commit that referenced this pull request Jan 10, 2022
as it breaks some existing services
Kurnihil pushed a commit to Kurnihil/nginx-proxy-manager that referenced this pull request Jan 11, 2023
as it breaks some existing services
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

proxy_set_header X-Forwarded-Host is missing in conf.d/proxy.conf
5 participants