E52A Re-fix #2767 by xfl12345 · Pull Request #7012 · coder/code-server · GitHub
[go: up one dir, main page]
Skip to content

Re-fix #2767#7012

Merged
code-asher merged 5 commits intocoder:mainfrom
xfl12345:patch-2
Oct 18, 2024
Merged

Re-fix #2767#7012
code-asher merged 5 commits intocoder:mainfrom
xfl12345:patch-2

Conversation

@xfl12345
Copy link
Contributor
@xfl12345 xfl12345 commented Sep 28, 2024

@xfl12345 xfl12345 requested a review from a team as a code owner September 28, 2024 09:33
@xfl12345
Copy link
Contributor Author
xfl12345 commented Sep 28, 2024

The example of docker-compose.yml can be view as flow link.
https://github.com/xfl12345/docker-setup/blob/a100a69e8245ec7a1abb0ec9073a8cd1bcfa8096/compose/standalone/code_server/docker-compose.example.yml

@xfl12345
Copy link
Contributor Author
xfl12345 commented Sep 28, 2024

Reproduce:

  1. docker compose up -d --remove-orphans
  2. Finish the WebUI setup
  3. docker restart code-server

The log will be like as follow:

[2024-09-25T09:10:10.098Z] info  code-server 4.90.3 26c763485b9101fcae2ce56fc0903f0e2d48abe3
[2024-09-25T09:10:10.126Z] info  Using user-data-dir /root/.local/share/code-server
[2024-09-25T09:10:10.274Z] info  Using config file /root/.config/code-server/config.yaml
[2024-09-25T09:10:10.275Z] info  HTTP server listening on http://0.0.0.0:8080/
[2024-09-25T09:10:10.276Z] info    - Authentication is enabled
[2024-09-25T09:10:10.276Z] info      - Using password from /root/.config/code-server/config.yaml
[2024-09-25T09:10:10.277Z] info    - Not serving HTTPS
[2024-09-25T09:10:10.278Z] info  Session server listening on /root/.local/share/code-server/code-server-ipc.sock
[23:20:31] 




[23:20:31] Extension host agent started.
[23:20:35] Deleted uninstalled extension from disk eamodio.gitlens /root/.local/share/code-server/extensions/eamodio.gitlens-15.4.0-universal
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist
usermod: user 'coder' does not exist

@code-asher
Copy link
Member
code-asher commented Sep 30, 2024

Thank you for the PR!

Do you know what exactly causes the bug? I tried to reproduce with docker compose and then docker restart but it seems to be working without errors for me. Here is what I ended up with:

# version: '3'
services:
  code_server:
    env_file:
      - path: ./default.env
        required: true
    container_name: code-server
    image: codercom/code-server:latest-xfl
    pull_policy: build
    build:
      context: ./
      dockerfile: code-server-xfl.Dockerfile
    user: "1000:1000"
    volumes:
      - /home/coder/.local:/home/coder/.local
      - /home/coder/.config:/home/coder/.config
    extra_hosts:
      - "host.docker.internal:host-gateway"
    restart: always

Maybe it is the user line? I used 1000 because I do not have a 2006 user. But I am not sure why it would matter.

It seems like the way for the bug to trigger is if whoami returned something other than code-server but it is not clear to me when or why this would happen.

@xfl12345
Copy link
Contributor Author
xfl12345 commented Oct 1, 2024 •
edited
Loading

Thank you for the PR!

Do you know what exactly causes the bug? I tried to reproduce with docker compose and then docker restart but it seems to be working without errors for me. Here is what I ended up with:

# version: '3'
services:
  code_server:
    env_file:
      - path: ./default.env
        required: true
    container_name: code-server
    image: codercom/code-server:latest-xfl
    pull_policy: build
    build:
      context: ./
      dockerfile: code-server-xfl.Dockerfile
    user: "1000:1000"
    volumes:
      - /home/coder/.local:/home/coder/.local
      - /home/coder/.config:/home/coder/.config
    extra_hosts:
      - "host.docker.internal:host-gateway"
    restart: always

Maybe it is the user line? I used 1000 because I do not have a 2006 user. But I am not sure why it would matter.

It seems like the way for the bug to trigger is if whoami returned something other than code-server but it is not clear to me when or why this would happen.

You can create another normal user, one with a UID not equal to 1000.
Maybe code-server is not recommended(or not supported?) to use a UID that is different from 1000?
But it works fine with the patch applied.

@xfl12345
Copy link
Contributor Author
xfl12345 commented Oct 1, 2024

I noticed that code-server was running as root when I specified a UID different from 1000.
It seems that some function is broken after taking that action.

Perhaps code-server does not support using a custom UID.
So, I closed the PR.

@xfl12345 xfl12345 closed this Oct 1, 2024
@xfl12345
Copy link
Contributor Author
xfl12345 commented Oct 1, 2024 •
edited
Loading

I found the reason for "running as root" because I defined docker-compose.yml as below.

services:
  code_server:
    extends:
      file: ./docker-compose.example.yml
      service: code_server
    user: root
    pid: host
    ipc: host
    cap_add:
      - ALL
    security_opt:
      - apparmor=unconfined

Then I disabled some features by commenting out some fields.

services:
  code_server:
    extends:
      file: ./docker-compose.example.yml
      service: code_server
    # user: root
    # pid: host
    # ipc: host
    # cap_add:
    #  - ALL
    security_opt:
      - apparmor=unconfined

Yes, the official code works fine with a custom UID.
But I'm sure the patch has solved another issue, allowing code-server to run as root successfully every time it's restarted.

@xfl12345 xfl12345 reopened this Oct 1, 2024
@code-asher
Copy link
Member
code-asher commented Oct 1, 2024 •
edited
Loading

Ohh interesting! If it runs as root, is that a sign that something else is wrong?

Either way, it seems reasonable to use id to detect if the user already exists. I think we probably do not need the whoami call at all if we do it that way. Would this work?

# Rename coder user to the name in DOCKER_USER if DOCKER_USER does not already exist.
if ! id -u "$DOCKER_USER" 2>/dev/null ; then

@xfl12345
Copy link
Contributor Author
xfl12345 commented Oct 2, 2024

Ohh interesting! If it runs as root, is that a sign that something else is wrong?

Either way, it seems reasonable to use id to detect if the user already exists. I think we probably do not need the whoami call at all if we do it that way. Would this work?

# Rename coder user to the name in DOCKER_USER if DOCKER_USER does not already exist.
if ! id -u "$DOCKER_USER" 2>/dev/null ; then

Because the command will print the UID when it begins to check, I have replaced it with the code as below.

if [ -z "$(id -u "$DOCKER_USER" 2>/dev/null)" ]; then

Test case:

  1. UID[2006], name[code-server]
    • First run: OK
    • Restart: OK
  2. UID[2006], name[xfl12345]
    • First run: OK
    • Restart: OK
  3. UID[2006], name not set
    • First run: OK
    • Restart: OK
  4. UID[1000], name not set
    • First run: OK
    • Restart: OK
  5. UID[1000], name[coder]
    • First run: OK
    • Restart: OK
  6. UID[1000], name[xfl12345]
    • First run: OK
    • Restart: OK
  7. UID[0], name not set
    • First run: OK
    • Restart: OK
  8. UID[0], name[code-server]
    • First run: OK
    • Restart: OK
  9. UID[0], name[root]
    • First run: OK
    • Restart: OK

ALL PASSED! Cheers!

@code-asher
Copy link
Member
code-asher commented Oct 18, 2024 •
edited
Loading

Sorry for the delay, thank you for fixing this and for the thorough testing! This will be published soon, I think in the next few days after the release candidate has had time to sit.

@code-asher code-asher merged commit dd2e9fc into coder:main Oct 18, 2024
yiliang114 pushed a commit to yiliang114/code-server that referenced this pull request Jan 23, 2025
@xfl12345 @yiliang114
Fix user rename when restarting container (coder#7012)
65e83d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xfl12345 @code-asher
0