Feature/nextcloud garages3 primary storage#40
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83534dda69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <?php | ||
|
|
||
| $bucket = getenv('GARAGES3_BUCKET') ?: 'nextcloud'; | ||
| $key = getenv('GARAGES3_KEY'); |
There was a problem hiding this comment.
The bootstrap writes the usable S3 access key to GARAGES3_KEY_ID and stores only the human key name (nextcloud-app) in GARAGES3_KEY, while this compose file only exports GARAGES3_KEY_ID to the app/cron containers. With the documented make setup-garages3 flow, this line leaves $key empty and throws at startup; if a user manually exports GARAGES3_KEY, Nextcloud would authenticate with the key name rather than the Garage access key ID. Read GARAGES3_KEY_ID or map it into GARAGES3_KEY here.
Useful? React with 👍 / 👎.
| volumes: | ||
| - ./.docker/app/conf.d/php.ini:/usr/local/etc/php/conf.d/custom-php.ini | ||
| - ./volumes/nextcloud:/var/www/html | ||
| - ../volumes/nextcloud/config/s3.config.php:/var/www/html/config/s3.config.php:ro |
There was a problem hiding this comment.
The documented local flow (cp .env.dist .env && make reset-garages3) never creates ../volumes/nextcloud/config/s3.config.php; a repo-wide search only finds this path in this compose file and README. Docker short bind mounts create a missing source as a directory, so the first app run masks /var/www/html/config/s3.config.php with a read-only directory and prevents the Nextcloud image from using its generated OBJECTSTORE_S3_* config. Drop this bind or mount a real file inside the local stack.
Useful? React with 👍 / 👎.
| NEXTCLOUD_ADMIN_USER=admin | ||
| NEXTCLOUD_ADMIN_PASSWORD=admin | ||
| NEXTCLOUD_TRUSTED_DOMAINS=mydomain.coop | ||
| NEXTCLOUD_TRUSTED_DOMAINS=localhost,127.0.0.1,localhost:8080,127.0.0.1:8080 |
There was a problem hiding this comment.
The Nextcloud Docker image treats NEXTCLOUD_TRUSTED_DOMAINS as a space-separated list, so this comma-separated default is installed as one literal trusted domain. In the advertised local setup, requests to localhost:8080 or 127.0.0.1:8080 will still hit Nextcloud's untrusted-domain page after installation; use spaces here and in the compose default.
Useful? React with 👍 / 👎.
vitormattos
left a comment
vitormattos
left a comment
There was a problem hiding this comment.
We need to create a follow-up/fix PR to implement the comments.
| - ./garage/garage.toml:/etc/garage.toml | ||
| - ./garage/meta:/var/lib/garage/meta | ||
| - ./garage/data:/var/lib/garage/data |
There was a problem hiding this comment.
We normally mount volumes in separate volumes; this makes it easier to keep the environment clean and also helps with backups.
|
|
||
| rpc_bind_addr = "[::]:3901" | ||
| rpc_public_addr = "127.0.0.1:3901" | ||
| rpc_secret = "5f34a73fca71d4c1e8246781d619670f2f1b5e9a1abeceaf04b2d7b5c7cfe778" |
There was a problem hiding this comment.
I imagine it’s given data is fake; if it’s real, you’ll need to rotate the secret, and it’s good that here there’s something explicit saying that it’s a fake value, and maybe a comment on the previous line explaining how to generate the secret.
| volumes: | ||
| - ./.docker/app/conf.d/php.ini:/usr/local/etc/php/conf.d/custom-php.ini | ||
| - ./volumes/nextcloud:/var/www/html | ||
| - ../volumes/nextcloud/config/s3.config.php:/var/www/html/config/s3.config.php:ro |
There was a problem hiding this comment.
Something is wrong here, it’s mounting the volumes folder one level below the current folder, and the folder volumes/nextcloud is already mounted in the line above. The approach for configuring the s3.config.php file should be different.
| garage: | ||
| image: dxflrs/garage:v1.0.0 | ||
| restart: unless-stopped | ||
| network_mode: host | ||
| volumes: | ||
| - ./garage/garage.toml:/etc/garage.toml | ||
| - ./garage/meta:/var/lib/garage/meta | ||
| - ./garage/data:/var/lib/garage/data |
There was a problem hiding this comment.
I think that to avoid duplicating the docker-compose.yml file for every new service, the ideal is to have in some README the instructions for how to create the docker-compose.override.yml file and how to add the new service to it. That way, you can run docker compose up without issues, and the changes won’t impact anything when versioned files are updated—plus it reduces the number of files to maintain in the future.
There was a problem hiding this comment.
Having instructions in some README on how to add the service to a docker-compose.override.yml removes the need for a Makefile. You could even have a folder with examples of docker-compose.override.yml files already prepared for each scenario (e.g., one example for garages3) with a README in that examples folder explaining how to use them and adding the docker-compose.override.yml to the .gitignore.
| 2. Update `garage/garage.toml` and replace the placeholder `rpc_secret`. | ||
| 3. Run `make setup-garages3`. | ||
| 4. Open the Nextcloud URL and finish the initial admin setup if it is still pending. | ||
|
|
There was a problem hiding this comment.
This block in the project’s main README may be intimidating; it might be better to put it in a docs folder and also have more detailed project documentation. With a docs folder, you can even set up GitHub Pages later (a static site) with something like a “Read the Docs” style.
2 participants
Add example on how to setup with primary storage garages3