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

Companion standalone seems to ignore COMPANION_PATH #3514

Closed
cgansen opened this issue Feb 25, 2022 · 2 comments · Fixed by #3515
Closed

Companion standalone seems to ignore COMPANION_PATH #3514

cgansen opened this issue Feb 25, 2022 · 2 comments · Fixed by #3515
Labels

Comments

@cgansen
Copy link

cgansen commented Feb 25, 2022

When running the companion standalone server both as the Node application and the latest Docker image, I can't get it to respect the COMPANION_PATH environment variable.

When I set the value to something like /path/to/companion, requests to localhost:3020/path/to/companion/ fail with a 404, and only requests to the root localhost:3020/ and localhost:3020/metrics return 200s.

Node app:

$ COMPANION_PATH="/path/to/companion" COMPANION_DATADIR="/tmp"  COMPANION_DOMAIN="localhost"  companion
Welcome to Companion! v3.3.0
Listening on http://0.0.0.0:3020
::ffff:127.0.0.1 - - [25/Feb/2022:07:07:26 +0000] "GET /path/to/companion/metrics HTTP/1.1" 404 23 "-" "curl/7.77.0"
::ffff:127.0.0.1 - - [25/Feb/2022:07:07:32 +0000] "GET /metrics HTTP/1.1" 200 - "-" "curl/7.77.0"

Docker image:

$ docker run -it --rm -e COMPANION_PATH="/path/to/companion" -e COMPANION_DATADIR="/tmp" -e COMPANION_DOMAIN="localhost" -p 3020:3020 transloadit/companion:main
Welcome to Companion! v3.3.0
Listening on http://0.0.0.0:3020
::ffff:172.17.0.1 - - [25/Feb/2022:07:15:19 +0000] "GET /metrics HTTP/1.1" 200 - "-" "curl/7.77.0"
companion: 2022-02-25T07:15:26.058Z [info] companion.client.version uppy client version 1.0.0
::ffff:172.17.0.1 - - [25/Feb/2022:07:15:26 +0000] "GET /path/to/companion/metrics HTTP/1.1" 404 23 "-" "curl/7.77.0"

Am I misunderstanding the purpose of this configuration variable? I would expect in both cases that requests to localhost:3020/path/to/companion/ would return the homepage banner.

mifi added a commit that referenced this issue Feb 25, 2022
make sure it serves everything served behind that path
fixes #3514
@mifi
Copy link
Contributor

mifi commented Feb 25, 2022

Hi. It seems like not all paths are served (including those two that you were testing root / and /metrics)
Others paths should still be served under /path/to/companion though. Can you confirm that?

I've created a PR for this

@cgansen
Copy link
Author

cgansen commented Feb 25, 2022

I can confirm that requests to localhost:3020/path/to/companion/redirect/connect/drive/ are handled correctly when I set COMPANION_PATH. Thanks for clearing up the confusion!

@mifi mifi closed this as completed in #3515 Mar 2, 2022
mifi added a commit that referenced this issue Mar 2, 2022
* Fix COMPANION_PATH

make sure it serves everything served behind that path
fixes #3514

* add tests for subpath

* chmod +x companion.sh

* remove process.exit

as it's considered harmful in this case
an error thrown will also be printed to stderr

* add backward compatible redirect

* Apply suggestions from code review

Co-authored-by: Antoine du Hamel <[email protected]>

* fix oops

Co-authored-by: Antoine du Hamel <[email protected]>
HeavenFox pushed a commit to docsend/uppy that referenced this issue Jun 27, 2023
* Fix COMPANION_PATH

make sure it serves everything served behind that path
fixes transloadit#3514

* add tests for subpath

* chmod +x companion.sh

* remove process.exit

as it's considered harmful in this case
an error thrown will also be printed to stderr

* add backward compatible redirect

* Apply suggestions from code review

Co-authored-by: Antoine du Hamel <[email protected]>

* fix oops

Co-authored-by: Antoine du Hamel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants