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

Remove default user #1090

Merged
merged 4 commits into from
Aug 12, 2024
Merged

Remove default user #1090

merged 4 commits into from
Aug 12, 2024

Conversation

gouttegd
Copy link
Contributor

The Ubuntu 24.04 base image that we now use contains a predefined ubuntu user account with a UID of 1000. This could cause some confusion with the ODK’s own user account (odkuser), which by default also has a UID of 1000.

There is no need for that ubuntu account, so this PR simply removes it.

While we are at it, this PR also refactors the last section of the ODKLite Dockerfile (where we add the instruction to remove the ubuntu user account) to group several operations together and reduce slightly the number of layers in the ODKLite image.

The Ubuntu 24.04 Docker image contains a default user account named
`ubuntu` with a UID of 1000. This is a change from the 22.04 image,
which did not contain such an account.

The presence of that account could cause some confusion as the ODK also
creates a `odkuser` user account with a default UID of 1000. To be on
the safe side, we explicitly remove the default `ubuntu` account.
This commit refactors some of the last steps of constructing the ODKLite
image to reduce the number of FS layers in the final image.

Specifically, two "condensations" are performed.

1. `COPY` and `RUN chmod` operations are lumped together:

  COPY src dst
  RUN chmod 755 dst

now becomes

  COPY --chmod=755 src dst

2. Several RUN operations are replaced by a single RUN with &&-separated
commands:

  RUN command1
  RUN command2

now becomes

  RUN command1 && command2

This reduces the number of layers in the ODKLite image from 21 to 15.
@gouttegd gouttegd self-assigned this Aug 11, 2024
@gouttegd
Copy link
Contributor Author

CI check explicitly cancelled since it is bound to fail until the debugpy issue is fixed (which should happen the next time the “Update Python constraints” workflow is run).

@matentzn
Copy link
Contributor

which should happen the next time the “Update Python constraints” workflow is run

FYI as admin of ODK you should feel free to simply run the action and merge the PR after an eyeball review (which only does one thing: checking that no tool decreased in version number, of if it does, it can be explained and dismissed).

@gouttegd
Copy link
Contributor Author

FYI as admin of ODK you should feel free to simply run the action

Sure, but in this case, with the next automatic run scheduled to happen tomorrow, there was no real need to trigger an out-of-cycle run.

One of the removed `RUN chmod` operations in a previous commit had not
been replaced by the proper `COPY --chmod`, resulting in the
/tools/check-rdfxml script being left with the execution bit unset. We
fix that here.
@gouttegd gouttegd marked this pull request as ready for review August 12, 2024 08:48
@gouttegd gouttegd requested a review from matentzn August 12, 2024 08:49
Copy link
Contributor

@matentzn matentzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it with one ontology and I cant see any difference, so I am assuming its all good.

I trust you have, as usual, weighed the risks if there are any; I can only do a LGTM review here as I dont understand the consequences of deleting the default user. :)

@gouttegd
Copy link
Contributor Author

Deleting the default user should have no consequences at all.

On the other hand, leaving the default user in place (that is, without this PR) could lead to confusion and possibly bugs (though I have not so far observed any), because we end up with two entries in /etc/passwd with the same UID but different home directories:

ubuntu:x:1000:1000:Ubuntu:/home/ubuntu:/bin/bash
odkuser:x:1000:1000:ODK User:/home/odkuser:/bin/bash

For standard workflows, this should not cause any issues because the standard workflows never refer to the user’s home directory anyway.

But if you have a custom workflow that somehow needs to refer to the home directory (for example, if you are use runoak, which may cache some data under $HOME/.data), then you may run into some troubles if two different programs in the workflow don’t use the same logic to pick up the home directory from the conflicting entries in /etc/passwd – that is, if one program believes the correct home directory is /home/ubuntu while another one believes that it is /home/odkuser.

Again, I have not observed any such trouble yet, but the mere fact that the possibility exists is reason enough to remove the (useless) default user.

@gouttegd gouttegd merged commit 7da6b03 into master Aug 12, 2024
1 check passed
@gouttegd gouttegd deleted the remove-default-user branch August 12, 2024 11:21
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.

2 participants