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

refactor(cli): keep alteration scripts folder writable by gid 0 #6328

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

bpow
Copy link
Contributor

@bpow bpow commented Jul 24, 2024

Summary

One possible approach to address #6327.

Ensures that /etc/logto/packages/cli/alteration-scripts is present and writable by gid 0.
Does not remove/replace this directory each time, just removes the contents. This ensures that
the writabiliy remains (and also allows this to be a separately-mounted directory.

The reasons for doing this are described in #6327, but briefly, the desire is to let logto be
run from within docker as a non-root user (although gid 0 would still be required unless
the alteration-scripts are on a separately-mounted directory with appropriate permissions).

I propose this approach rather than making all of /etc/logto/packages/cli as writable by gid 0 since keeping more things read-only just seems safer overall.

Testing

So far I have only tested by running with a custom docker-compose.yml that is modified to have user: 1001:0 for the app container, and it does apply seeding as desired. I have not tested
on openshift/k8s yet (will need to build a container image accessible to my openshift cluster
to do that), but wanted to put this out there for possible discussion.

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

Not sure which of these you would want for this type of change (I think this may qualify at most as a @logto/cli: minimal under .changeset, but accept input on that).

Copy link

github-actions bot commented Jul 24, 2024

COMPARE TO master

Total Size Diff 📈 +727 Bytes

Diff by File
Name Diff
Dockerfile 📈 +112 Bytes
packages/cli/src/commands/database/alteration/utils.ts 📈 +615 Bytes

@bpow bpow changed the title Keep packages/cli/alteration-scripts writable by gid 0 (WIP) Keep packages/cli/alteration-scripts writable by gid 0 (facilitate running in "rootless" containers) Jul 26, 2024
@gao-sun gao-sun requested a review from a team July 28, 2024 14:21
@bpow bpow force-pushed the no-rmdir-alteration-scripts branch from 77ad153 to a29e68e Compare July 28, 2024 23:33
@bpow
Copy link
Contributor Author

bpow commented Jul 28, 2024

just force-pushed with commits that have the same content but are now gpg-signed.

@wangsijie
Copy link
Contributor

Hi, @bpow the code look good, but would you mind change the commits to follow Conventional Commits in order to pass the CI tests?

Copy link

github-actions bot commented Sep 3, 2024

This PR is stale because it has been open 10 for days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Sep 3, 2024
@wangsijie wangsijie changed the title Keep packages/cli/alteration-scripts writable by gid 0 (facilitate running in "rootless" containers) refactor(cli): keep alteration scripts folder writable by gid 0 Sep 25, 2024
@github-actions github-actions bot added size/s enhancement Make it better and removed size/xs labels Sep 25, 2024
facilitates running in rootless container with r/w mounted alteration-scripts
directory (fixed logto-io#6327)
@wangsijie wangsijie merged commit fc6f94f into logto-io:master Sep 26, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants