Skip to content

Add BASEPATH env var to enable use of --base-path option #19

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

Closed
wants to merge 4 commits into from
Closed

Add BASEPATH env var to enable use of --base-path option #19

wants to merge 4 commits into from

Conversation

tdack
Copy link

@tdack tdack commented Feb 14, 2020

Description:

Add support for BASEPATH environment variable to enable use of code-server --base-path option

Benefits of this PR and context:

When reverse proxied, allows code-server to be served from any path, not just the root of the domain/ip, eg: http://<your-ip>:8443/code

How Has This Been Tested?

  • built image with changes using instructions at end of README.md
  • created and launched image with:
docker run -it \
  --name=code-server \
  -e PUID=1000 \
  -e PGID=1000 \
  -e TZ=Europe/London \
  -e PASSWORD=password \
  -e SUDO_PASSWORD=password  \
  -e BASEPATH=coder \
  -p 8443:8443 \
  -v /path/to/appdata/config:/config \
  --restart unless-stopped \
  linuxserver/code-server:latest
  • configured nginx reverse proxy to serve code-server from http://<your-ip>:8443/code
	location /code {
		rewrite /code /code/ permanent;
	}

	location /code/ {
		proxy_pass http://localhost:8443/;
		proxy_set_header Host $host;
		proxy_set_header Upgrade $http_upgrade;
		proxy_set_header Connection upgrade;
		proxy_set_header Accept-Encoding gzip;
		proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
		proxy_set_header X-Forwarded-Proto $scheme;
		proxy_set_header X-Real-IP $remote_addr;
	}
  • navigate to http://<your-ip>:8443/code
  • code-server redirects to http://<your-ip>:8443/code/login
  • enter login credentials
  • code-server redirects back to http://<your-ip>:8443/code
  • UI is displayed and functions as expected

Source / References:

--base-path option: https://github.com/cdr/code-server/blob/c8fc54bfb1ba0fae592dee33a5f68843d9b41a4a/src/node/cli.ts#L59

Quoted substitution breaks if there is a space in var
1 similar comment
@aptalca aptalca self-requested a review February 14, 2020 16:23
@aptalca
Copy link
Member

aptalca commented Feb 14, 2020

Thanks for the PR @tdack but I can't get this to work.

Are you sure it worked for you with a local build? Because you had the basepath var wrapped in quotes in the original PR, which causes the space to be included as part of the variable and code server gives an error Option 'base-path coder' is unknown. Ignoring.

However, after fixing that, code server seems to run with the option accepted. But the option doesn't seem to work.
If no password is set, code server is accessible without a base url.
If a password is set, code server is published at the base path, but it is inaccessible as it doesn't redirect to login and instead displays Unathorized.

@aptalca
Copy link
Member

aptalca commented Feb 14, 2020

@tdack it seems like code-server devs had a completely different interpretation of how base path should be used, so they intended it to work only with a reverse proxy, which is super confusing to the end user. Also, the difference in base path handling behavior between setting or not setting a password may indicate a bug there.

In any case, it seems they will remove the base-path option soon and make all paths relative so it should work with reverse proxies natively: coder/code-server#1302

@aptalca aptalca closed this Feb 14, 2020
@tdack
Copy link
Author

tdack commented Feb 14, 2020

@aptalca It did work, however, my testing was done with code-server behind a reverse proxy, and accessed through the proxy. So it looks like you are right about their intention.

Hopefully the issue to remove the option and use relative paths gets sorted.

Thanks for fixing my mistakes and considering it.

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.

3 participants